github-actions[bot] commented on code in PR #65977:
URL: https://github.com/apache/doris/pull/65977#discussion_r3651844092
##########
be/src/storage/segment/historical_row_retriever.cpp:
##########
@@ -115,7 +115,7 @@ Status
PrimaryKeyModelRowRetriever::retrieve_historical_row(const Int8* delete_s
RowsetSharedPtr rowset;
auto st = tablet->lookup_row_key(key, tablet->tablet_schema().get(),
_seq_column != nullptr,
specified_rowsets, &loc,
_mow_context->max_version,
- segment_caches, &rowset);
+ segment_caches, &rowset,
/*with_rowid=*/false);
Review Comment:
[P1] Decode the cluster rowid before accepting a stale sequence
With this call now passing `with_rowid=false`, a `K+S` probe can correctly
reach a stored cluster-key entry encoded as `K+S+R`. However,
`Segment::lookup_row_key()` returns `KEY_ALREADY_EXISTS` for a lower incoming
sequence before it decodes `R`; `loc.row_id` is still the primary-index ordinal
set by the seek. This caller accepts that status and queues `loc` for the
historical/BEFORE read, so when cluster order differs from primary-key order it
reads another physical row (in the new `[11,10,1,0]` history, stale key `0`
uses ordinal `0` instead of rowid `3` and fetches key `11`). The sequence test
has no cluster rowid, while the combined test forces only `OK`. Please decode
`R` before the stale-sequence return and add a combined stale-sequence case
that asserts the exact BEFORE row.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
