disable filter pushdown for scan command#238
Conversation
WalkthroughAdds per-record object type metadata across the scan pipeline: new obj_type fields and accessors on scan tuple types, updated method signatures to thread object_type through cache/map/scanner/remote paths, protobuf extension for ScanTuple_msg, a minor ScanOpen signature change, a removed timeout branch, and a timing call adjustment. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Scan Operation
participant Map as TemplateCcMap
participant Cache as ScanCache
participant Tuple as ScanTuple
Note over Map,Cache: New object_type flow added (int32)
Client->>Map: ScanKey/ScanGap(..., object_type)
activate Map
Map->>Map: GetObjectType(cce) ⟶ obj_type (default -1 possible)
Map->>Cache: AddScanTuple(..., object_type, ...)
activate Cache
Cache->>Tuple: construct ScanTuple(..., obj_type, ...)
activate Tuple
Tuple->>Tuple: obj_type_ = object_type
Tuple-->>Cache: ScanTuple created
deactivate Tuple
Cache-->>Map: tuple stored
deactivate Cache
Map-->>Client: results include obj_type metadata
deactivate Map
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
include/tx_operation_result.h (1)
844-856: Move assignment operator missing bucket_number_ handling.The move assignment operator doesn't handle the newly added
bucket_number_member, while the move constructor does. This inconsistency can lead to incorrect state after a move assignment.Apply this diff to fix the move assignment operator:
BucketScanPlan &operator=(BucketScanPlan &&other) noexcept { if (this != &other) { plan_index_ = other.plan_index_; + bucket_number_ = other.bucket_number_; buckets_ = std::move(other.buckets_); other.buckets_ = nullptr; + other.bucket_number_ = 0; current_position_ = std::move(other.current_position_); node_group_terms_ = std::move(other.node_group_terms_); } return *this; }src/tx_execution.cpp (1)
1089-1092: Fix incorrect TX_TRACE context argument in ReloadCacheTxRequest handlerIn
ProcessTxRequest(ReloadCacheTxRequest &req),TX_TRACE_ACTION_WITH_CONTEXTis called with&fi_req, but there is nofi_reqin scope here. This will not compile and looks like a copy‑paste from the fault‑inject handler; it should be&req.- TX_TRACE_ACTION_WITH_CONTEXT( - this, - &fi_req, + TX_TRACE_ACTION_WITH_CONTEXT( + this, + &req, [this]() -> std::stringinclude/cc/template_cc_map.h (3)
2265-2333: AddScanTuple now requiresobject_type, butScanSliceCccall sites are not updated (compile-time break + inconsistent metadata).
AddScanTuple’s signature was extended with anint32_t object_typeparameter (Line 2274) and call sites inScanNextBatchCcare correctly passingGetObjectType(...)(Lines 2550–2565, 2706–2721). However, bothScanSliceCcpaths still callAddScanTuplewithout this new argument (Lines 3757–3775 and 3961–3975), which will not compile and would omitobj_type_even if it did.You should pass
GetObjectType(cce)(or a locally cached value) at allAddScanTuplecall sites inScanSliceCc. One minimal fix is:@@ lambda scan_tuple_func(...) - bool is_locked = false; + bool is_locked = false; + int32_t object_type = GetObjectType(cce); @@ if (req.IsLocal()) - AddScanTuple(cce_key, - cce, - scan_cache, - scan_type, - ng_id, - ng_term, - req.Txn(), - req.ReadTimestamp(), - is_read_snapshot, - need_fetch_snapshot, - is_locked, - false, - is_require_keys, - is_require_recs); + AddScanTuple(cce_key, + cce, + scan_cache, + scan_type, + ng_id, + ng_term, + req.Txn(), + req.ReadTimestamp(), + object_type, + is_read_snapshot, + need_fetch_snapshot, + is_locked, + false, + is_require_keys, + is_require_recs);and in the resumed-blocking branch of
ScanSliceCc:@@ resumed ScanSliceCc path (local branch) - bool need_fetch_snapshot = false; - if (req.IsLocal()) - { - AddScanTuple(cce_key, - cce, - scan_cache, - scan_type, - ng_id, - ng_term, - req.Txn(), - req.ReadTimestamp(), - is_read_snapshot, - need_fetch_snapshot, - is_locked, - false, - is_require_keys, - is_require_recs); + bool need_fetch_snapshot = false; + if (req.IsLocal()) + { + int32_t object_type = GetObjectType(cce); + AddScanTuple(cce_key, + cce, + scan_cache, + scan_type, + ng_id, + ng_term, + req.Txn(), + req.ReadTimestamp(), + object_type, + is_read_snapshot, + need_fetch_snapshot, + is_locked, + false, + is_require_keys, + is_require_recs);This keeps range-partition scans compiling and makes object_type handling consistent with hash-partition scans.
Also applies to: 2550-2565, 2706-2721, 3757-3775, 3961-3975
3137-3155: Useend_cce’s page when reconstructingend_itinRemoteScanNextBatchresumed path.In the remote
ScanNextBatchCcpath, when resuming from a blocked scan and releasing the read-intent on the end marker,end_itis built usingccp(the page ofprior_cce) instead ofend_cce->GetCcPage()(Line 3149). The localScanNextBatchCcpath right above correctly usesend_ccp = end_cce->GetCcPage()and passes that intoIterator(end_cce, end_ccp, …).If
end_cceresides on a different page thanprior_cce, usingccphere can makeIterator::FindEntry(cce)search the wrong page, leading to incorrectend_itand potentially broken blocking/resume behavior.Align the remote path with the local fix:
if (end_lock_addr != 0) { KeyGapLockAndExtraData *end_lock = reinterpret_cast<KeyGapLockAndExtraData *>(end_lock_addr); assert(end_lock != nullptr && end_lock->GetCcEntry() != nullptr); auto end_cce = static_cast< CcEntry<KeyT, ValueT, VersionedRecord, RangePartitioned> *>( end_lock->GetCcEntry()); - assert(end_cce->GetCcPage() != nullptr); - - // Release read intent lock - end_it = Iterator(end_cce, ccp, &neg_inf_); + assert(end_cce->GetCcPage() != nullptr); + auto *end_ccp = static_cast< + CcPage<KeyT, ValueT, VersionedRecord, RangePartitioned> *>( + end_cce->GetCcPage()); + + // Release read intent lock + end_it = Iterator(end_cce, end_ccp, &neg_inf_); ReleaseCceLock(end_cce->GetKeyLock(), end_cce, req.Txn(), ng_id, LockType::ReadIntent);
11762-11818: Guard onis_deleted_comes too late—rec_str_.data()is dereferenced before checking if item is deleted.At lines 11810 and 11900, both local and remote branches dereference
item.rec_str_.data()unconditionally for all items in the loop, but theif (!item.is_deleted_)guard does not start until lines 11812 and 11904. This means deleted items—which may have emptyrec_str_—will have their first byte read as object type, resulting in undefined/garbage values (or'\0'for empty strings).Additionally, the
VersionedRecordconstexpr guard at line 11785 protects only the pattern/obj_type filter logic, not the dereference itself.Recommended fix: Guard the dereference with
if (!item.rec_str_.empty())before line 11810/11900, or document and enforce the invariant thatrec_str_is always non-empty (even for deleted items) by adding an assert at the producer ofbucket_data_items_.
🧹 Nitpick comments (8)
include/cc/cc_handler.h (1)
320-320: Consider restoring const qualifier for consistency.The
constqualifier was removed from theschema_versionparameter (a pass-by-value scalar). While this has no functional impact, other methods in this file still useconst uint64_t schema_version(e.g., lines 90, 228, 376, 506), creating an inconsistency.Apply this diff to restore consistency:
- uint64_t schema_version, + const uint64_t schema_version,include/cc/cc_request.h (1)
3297-3313: Confirm intentional use of-1sentinel for remote scan tuple object typeThe new
-1argument is in the correct position for the addedobject_typeparameter onAddScanTuple, so the call-site signature is consistent. What’s not obvious is whether remote scan responses already carry per-tuple object type (via the new proto field); if they do, hard‑coding-1here will silently drop that metadata for all remote scan results.If the intent is “object type unknown / not applicable” for this path, consider:
- Adding a named constant (e.g.,
kUnknownObjectType) instead of a naked-1for clarity, and/or- Leaving a brief comment or TODO indicating why remote object type is intentionally ignored.
If the intent is to propagate object types, this should be wired through from the response instead of
-1.include/proto/cc_request.proto (1)
955-963: Addingobj_typetoScanTuple_msglooks correct and backward‑compatibleField
obj_type = 7is appended with a new tag, so existing deployments remain wire‑compatible. Just ensure all producers either set this explicitly (including-1for “unknown”) or consumers treat0conservatively if unset by older peers.include/cc/object_cc_map.h (1)
2789-2795:GetObjectTypeimplementation is consistent with existing payload accessUsing
cur_payload_and falling back to-1when null matches howFilterRecordalready inspectspayload_.cur_payload_(e.g., forIsMatchTypeand TTL). This should surface the committed object type to scan callers without exposing uncommitted dirty state.Consider a brief comment noting that this returns the committed payload’s type (ignoring dirty payloads) so future callers understand the semantics.
include/cc/scan.h (1)
53-76: Scan tupleobj_type_threading is coherent and non‑breaking
obj_type_is initialized to-1by default and carried through the new constructor and TemplateScanTuple move-ctor, so the metadata remains aligned withkey_ts_,gap_ts_, andrec_status_.- Adding
ObjectType()with a default implementation avoids changes to existing subclasses while exposing the new field via the common interface.Looks good as an additive extension of the scan tuple API.
A brief comment on the meaning of
obj_type_ == -1(e.g., “unknown / not set”) would make the convention clearer to future readers.Also applies to: 85-89, 90-102, 117-128
include/tx_request.h (1)
595-609: ScanBatchTuple extended withobject_type_without changing move semantics
- New constructors take
object_typealongsideversion, wiring them intoversion_ts_andobject_type_.- The move constructor now also moves
object_type_, keeping tuples self‑contained for batch scan flows.- Default
object_type_{-1}is consistent with other scan request types.This is a straightforward extension of the scan batch tuple payload.
If some paths conceptually distinguish “unset” vs “any type”, documenting that
-1means “no object type constraint / unknown” would help prevent misuse.Also applies to: 612-625, 629-637, 639-645
include/cc/ccm_scanner.h (1)
298-306: Object type assignment is correctly implemented.The
object_typeparameter is consistently assigned toscan_tuple->obj_type_in both the ScanGap path (line 305) and the regular scan path (line 323), ensuring all scan tuples receive the object type metadata regardless of the code path taken.Consider adding a comment explaining what
object_typerepresents and its valid value range to improve code maintainability:ScanTuple *AddScanTuple(const std::string &key_str, size_t &key_offset, uint64_t key_ts, const std::string &record_str, size_t &rec_offset, RecordStatus rec_status, + // Object type identifier for this scan tuple int32_t object_type, uint64_t gap_ts,Also applies to: 307-333
include/cc/template_cc_map.h (1)
2623-2628: Filter and object-type API changes require derived maps to update overrides.
FilterRecordnow takes anint32_t obj_typeargument (Line 11290) and is called from both local and remoteScanNextBatchpaths as:FilterRecord(key, cce, req.GetRedisObjectType(), req.GetRedisScanPattern());(Lines 2623–2628 and 3266–3271), and a new virtual
GetObjectType(...)returning-1by default was added (Lines 11299–11303).For correctness:
- Any derived
TemplateCcMapsubclasses that customize scan behavior (e.g., Redis/object maps) must update theirFilterRecordimplementation to match the new signature and useoverride. Otherwise, the old overload will silently stop being called and in-memory filtering will degrade to the base implementation (no filtering).- Those same subclasses should override
GetObjectType(...)to ensure theobj_typeset onTemplateScanTuple/ScanTuple_msgis consistent with the semantics used byFilterRecordand by clients consuming scan results.Please double-check downstream overrides and add
overridewhere applicable to catch mismatches at compile time.Also applies to: 3266-3271, 11290-11303
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
include/cc/cc_handler.h(1 hunks)include/cc/cc_request.h(1 hunks)include/cc/ccm_scanner.h(4 hunks)include/cc/object_cc_map.h(1 hunks)include/cc/scan.h(4 hunks)include/cc/template_cc_map.h(34 hunks)include/proto/cc_request.proto(1 hunks)include/tx_operation_result.h(2 hunks)include/tx_request.h(3 hunks)src/remote/cc_stream_receiver.cpp(2 hunks)src/tx_execution.cpp(10 hunks)src/tx_operation.cpp(0 hunks)
💤 Files with no reviewable changes (1)
- src/tx_operation.cpp
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-20T04:30:07.884Z
Learnt from: liunyl
Repo: eloqdata/tx_service PR: 149
File: include/cc/cc_request.h:1876-1927
Timestamp: 2025-10-20T04:30:07.884Z
Learning: ScanNextBatchCc in include/cc/cc_request.h is used only for hash-partition scans; range-partition scans are handled by ScanSliceCc.
Applied to files:
include/cc/cc_request.hsrc/remote/cc_stream_receiver.cppinclude/cc/template_cc_map.hsrc/tx_execution.cpp
📚 Learning: 2025-10-21T06:46:53.700Z
Learnt from: lokax
Repo: eloqdata/tx_service PR: 149
File: src/remote/cc_stream_receiver.cpp:1066-1075
Timestamp: 2025-10-21T06:46:53.700Z
Learning: In src/remote/cc_stream_receiver.cpp, for ScanNextRequest handling, BucketIds() on RemoteScanNextBatch should never be empty—this is an expected invariant of the scan protocol.
Applied to files:
include/cc/cc_request.hsrc/remote/cc_stream_receiver.cppsrc/tx_execution.cpp
🧬 Code graph analysis (1)
include/cc/scan.h (3)
include/cc/cc_request.h (1)
obj_type_(1811-1830)include/tx_request.h (2)
obj_type_(585-806)obj_type_(678-738)include/remote/remote_cc_request.h (1)
obj_type_(452-462)
🔇 Additional comments (8)
include/tx_operation_result.h (3)
822-825: LGTM! Bucket count computation is correct.The loop correctly accumulates the total number of buckets across all node groups.
833-841: LGTM! Move constructor correctly handles bucket_number_.The move constructor properly copies
bucket_number_and resets it in the source object, maintaining correct move semantics.
914-917: LGTM! Accessor is correctly implemented.The
BucketNumber()getter properly returns the cached bucket count.src/remote/cc_stream_receiver.cpp (1)
1144-1167:obj_typepropagation intoScanCache::AddScanTupleis wired correctlyBoth memory and KV scan caches now pass
tuple_msg.obj_type()in the correct slot betweenrec_statusandgap_ts, matching the updatedAddScanTuple/ScanTuplesignature. This keeps remote and local scan tuple layout consistent.If mixed-version rolling upgrades are expected, please confirm that all scan producers are updated to fill
obj_type, or that downstream consumers treat proto‑default0as “unknown” where appropriate.Also applies to: 1195-1208
include/tx_request.h (1)
464-507: ScanOpenTxRequest: clean threading ofobj_typeandscan_patternThe constructor and
Resetnow consistently initializeobj_type_andscan_pattern_, with sensible defaults (-1and empty pattern) that preserve prior behavior when filters are not used. This matches how scan filters are represented elsewhere.Because
scan_pattern_is astd::string_view, just ensure the underlying string storage outlives theScanOpenTxRequestinstance to avoid dangling references.Also applies to: 510-555, 585-587
include/cc/ccm_scanner.h (1)
150-162: All call sites properly updated with object_type parameter.Verification confirms that the breaking API change has been consistently implemented:
- cc_stream_receiver.cpp (lines 1154, 1195): Both call sites pass
tuple_msg.obj_type()as the object_type parameter- cc_request.h (line 3301): Call site passes
-1(default value) for object_type- Virtual declaration and override (ccm_scanner.h lines 150–162, 271–283): Signatures match with object_type as the 7th parameter
The template_cc_map.h calls use a different overloaded AddScanTuple method with a distinct signature and are not affected by this virtual method change.
src/tx_execution.cpp (2)
2909-2909: Unconditional StartTiming in ScanNext is reasonableCalling
StartTiming()at the end ofProcess(ScanNextOperation &scan_next)ensures the timeout/forwarding clock is reset after each scan‑next invocation, regardless of whether the batch was fully local or involved remote work. This is consistent with other async paths and doesn’t introduce correctness issues.
3058-3505: ScanBatchTuple emplace_back updates correctly thread new object_type fieldAll
scan_batch.emplace_backsites now pass arguments in the order(key, record, status, ts, object_type, cce_addr), usingcc_scan_tuple->obj_type_for CCM‑derived tuples and-1for write‑set‑only / non‑object entries. This matches the updatedScanBatchTuplelayout and preserves existing semantics while enriching hash‑partition scans with object type metadata.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
include/cc/template_cc_map.h (2)
3137-3155: Potential wrong page used when rebuildingend_itinRemoteScanNextBatchresumeIn the
end_lock_addr != 0branch ofExecute(remote::RemoteScanNextBatch &req),end_itis rebuilt as:end_it = Iterator(end_cce, ccp, &neg_inf_);where
ccpis derived fromprior_cce. Ifend_cceresides on a differentCcPagethanprior_cce,Iteratorwill be constructed with a mismatched page, which can causeFindEntry(cce)/UpdateCurrent()to fail or misbehave.The local
ScanNextBatchCcpath above correctly derives anend_ccpfromend_cce->GetCcPage()and uses that when buildingend_it. The remote path should mirror that pattern, for example:- auto end_cce = static_cast< - CcEntry<KeyT, ValueT, VersionedRecord, RangePartitioned> *>( - end_lock->GetCcEntry()); - assert(end_cce->GetCcPage() != nullptr); - - // Release read intent lock - end_it = Iterator(end_cce, ccp, &neg_inf_); + auto end_cce = static_cast< + CcEntry<KeyT, ValueT, VersionedRecord, RangePartitioned> *>( + end_lock->GetCcEntry()); + auto *end_ccp = static_cast< + CcPage<KeyT, ValueT, VersionedRecord, RangePartitioned> *>( + end_cce->GetCcPage()); + assert(end_ccp != nullptr); + + // Release read intent lock + end_it = Iterator(end_cce, end_ccp, &neg_inf_);
11773-11840: Be explicit aboutrec_str_invariants when deriving obj_type, and gate it to non‑versioned recordsTwo related concerns in
BackfillForScanNextBatch:
item.rec_str_is dereferenced without checks to derive the object type:scan_tuple->obj_type_ = static_cast<int8_t>(*item.rec_str_.data()); ... int8_t store_rec_obj_type = static_cast<int8_t>(*item.rec_str_.data());and similarly in the remote path. If
rec_str_can ever be empty for tombstones / deleted entries, this becomes undefined behavior. If the invariant is “rec_str_always has at least one byte even for deletes”, it would be safer to encode that with anassert(!item.rec_str_.empty())or an earlycontinuewhen empty, so future changes don’t silently violate it.The same
static_cast<int8_t>(*item.rec_str_.data())is used unconditionally, even forVersionedRecord == true, where the first byte of the serialized record is not an object type. Downstream code appears to treat-1as the sentinel for “no object type”; to avoid leaking meaningless values for non‑object tables, consider restricting this to the non‑versioned instantiation:
TemplateScanTuple<KeyT, ValueT> *scan_tuple =scan_cache->AddScanTuple();
TemplateScanTuple<KeyT, ValueT> *scan_tuple =scan_cache->AddScanTuple(); ...
scan_tuple->KeyObj().SetPackedKey(item.key_str_.data(),item.key_str_.size());scan_tuple->obj_type_ = static_cast<int8_t>(*item.rec_str_.data());
scan_tuple->KeyObj().SetPackedKey(item.key_str_.data(),item.key_str_.size());if constexpr (!VersionedRecord){// rec_str_ is expected to be non‑empty for Redis objectsassert(!item.rec_str_.empty());scan_tuple->obj_type_ =static_cast<int8_t>(item.rec_str_[0]);}else{scan_tuple->obj_type_ = -1;}and the same pattern for the remote `object_type` / `scan_tuple->set_obj_type(...)` path.This keeps Redis SCAN behavior while making the invariants around
rec_str_explicit and preventing confusing obj_type values for non‑Redis tables.Also applies to: 11872-11932
🧹 Nitpick comments (2)
src/tx_execution.cpp (2)
2909-2909: UnconditionalStartTiming()inScanNextmay change timeout semanticsPreviously
StartTiming()was only invoked in branches with remote work; now it runs wheneverProcess(ScanNextOperation&)falls through, including purely local/hash-range cases. This will resetstate_clock_and can enqueue the tx as “waiting” for external processing even when no remote I/O is in flight. That’s probably fine, but it’s a behavioral change: local-only scans will now participate in timeout tracking and external-txm waiting.Consider either:
- Documenting that
StartTiming()is intentionally broader now, or- Restoring a guard (e.g., only call if some remote or deferred work is outstanding), to keep behavior closer to the previous version.
3048-3063:ScanBatchTupleobj_type_propagation and-1sentinel handlingYou’ve plumbed
obj_type_intoScanBatchTuplecreation:
- Hash-partition CCM tuples carry
cc_scan_tuple->obj_type_(line 3061), which looks correct.- All tuples synthesized from the write set as well as CCM tuples in the range-partition path use a hardcoded
-1(variousemplace_backsites in forward/backward merge and tail-write handling).A few things to double‑check / consider:
- If
ScanTuplefor range-partition scans now also has a meaningfulobj_type_, you may want to pass it through instead of-1so downstream consumers see consistent type info across both hash and range scans.- For write-set tuples with a non-null
TxRecord*, confirm that downstream code either:
- Does not rely on
obj_type_for them, or- Treats
-1as “derive from record / no filter pushdown” and never as a real type.- To avoid repeating magic constants, it might be clearer to introduce a named constant (e.g.,
constexpr int kObjTypeUnknown = -1;) and use that everywhere you mean “no object type”.Functionally this looks consistent, but the semantics of
-1should be clearly defined and matched on the consuming side.Also applies to: 3124-3126, 3132-3134, 3204-3210, 3220-3227, 3231-3235, 3276-3278, 3284-3286, 3342-3347, 3350-3355, 3424-3430, 3442-3447, 3450-3455, 3491-3496, 3500-3505
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
include/cc/template_cc_map.h(36 hunks)src/cc/local_cc_handler.cpp(0 hunks)src/tx_execution.cpp(10 hunks)
💤 Files with no reviewable changes (1)
- src/cc/local_cc_handler.cpp
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-20T04:30:07.884Z
Learnt from: liunyl
Repo: eloqdata/tx_service PR: 149
File: include/cc/cc_request.h:1876-1927
Timestamp: 2025-10-20T04:30:07.884Z
Learning: ScanNextBatchCc in include/cc/cc_request.h is used only for hash-partition scans; range-partition scans are handled by ScanSliceCc.
Applied to files:
src/tx_execution.cppinclude/cc/template_cc_map.h
📚 Learning: 2025-10-21T06:46:53.700Z
Learnt from: lokax
Repo: eloqdata/tx_service PR: 149
File: src/remote/cc_stream_receiver.cpp:1066-1075
Timestamp: 2025-10-21T06:46:53.700Z
Learning: In src/remote/cc_stream_receiver.cpp, for ScanNextRequest handling, BucketIds() on RemoteScanNextBatch should never be empty—this is an expected invariant of the scan protocol.
Applied to files:
src/tx_execution.cpp
🔇 Additional comments (2)
include/cc/template_cc_map.h (2)
2265-2333: obj_type threading through scan tuple builders looks consistentThe extra
int32_t object_typeparameter is propagated cleanly throughAddScanTuple, bothAddScanTupleMsgoverloads that need it, and down intoScanKeyand theScanGaphelpers for local and remote scan tuples. The newGetObjectType(...)virtual with a default-1keeps non‑object‑aware tables behaviorally unchanged while lettingObjectCcMapoverride as needed. No correctness issues spotted here.Also applies to: 2769-2839, 11205-11305
3765-3777: Passing-1as obj_type forScanSliceCcis a reasonable defaultBoth the normal and resumed paths in
ScanSliceCcnow callAddScanTuple/AddScanTupleMsgwithobject_typehard‑coded to-1, which matches the new defaultGetObjectType(...)behavior and keeps range‑partition scans object‑type agnostic while still benefiting from the extended signatures. This looks fine.Also applies to: 3969-4017
2 participants
Summary by CodeRabbit
New Features
Improvements
Behavioral Changes
✏️ Tip: You can customize this high-level summary in your review settings.