HappenLee commented on code in PR #67817:
URL: https://github.com/apache/doris/pull/67817#discussion_r4080986400
##########
be/src/storage/segment/column_reader.cpp:
##########
@@ -2432,11 +2440,196 @@ Status ArrayFileColumnIterator::read_by_rowids(const
rowid_t* rowids, const size
_recovery_from_place_holder_column(dst);
+ if (count == 0) {
+ return Status::OK();
+ }
+
+ if (!std::is_sorted(rowids, rowids + count)) {
Review Comment:
Please replace this fallback with a precondition assertion, after the
existing `count == 0` return:
```cpp
DCHECK(std::is_sorted(rowids, rowids + count));
```
I traced the production callers at `6654658` and did not find a reachable
path supplying decreasing row IDs to this ARRAY reader:
- Segment scans produce ascending row IDs within each batch, including
[backward bitmap
scans](https://github.com/apache/doris/blob/665465826784b5c3115fcca0c7243c3a4c2fba4c/be/src/storage/segment/segment_iterator.cpp#L309).
Predicate/lazy filtering preserves that order; descending output is applied
after materialization.
- [TopN row
fetching](https://github.com/apache/doris/blob/665465826784b5c3115fcca0c7243c3a4c2fba4c/be/src/exec/rowid_fetcher.cpp#L375)
sorts and deduplicates each segment's row IDs before reading. Point-query
column reads pass a single row ID.
- Fixed/flexible partial-update plans are populated from [primary-key-sorted
MemTable
output](https://github.com/apache/doris/blob/665465826784b5c3115fcca0c7243c3a4c2fba4c/be/src/load/memtable/memtable.cpp#L817).
These paths reject cluster keys, so grouping the ordered key lookups by
historical segment preserves row-ID order.
- [Publish-time conflict
rewriting](https://github.com/apache/doris/blob/665465826784b5c3115fcca0c7243c3a4c2fba4c/be/src/storage/tablet/base_tablet.cpp#L675)
traverses the primary-key index in order, and sorts the reconstructed block
before flushing. Row-binlog BEFORE reads follow the source block's key order;
[MoW tables with cluster keys cannot enable row
binlog](https://github.com/apache/doris/blob/665465826784b5c3115fcca0c7243c3a4c2fba4c/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java#L3004).
`FixedReadPlan::prepare_to_read()` preserving insertion order does not
itself establish a production counterexample: its producers already supply
ordered keys. `ReadColumnsReturnsThePlannedRows` directly constructs the `[2,
0]` plan, bypassing those producers.
Please also document the ordered-input precondition and adjust the new
unordered ARRAY test / accompanying comment to match it. This removes the
release-build O(count) ordering scan per ARRAY column and the per-row fallback.
Validation here is static call-chain inspection; no tests were run.
--
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]
