github-actions[bot] commented on code in PR #67894:
URL: https://github.com/apache/doris/pull/67894#discussion_r3993417981
##########
be/src/exec/sort/partition_sorter.cpp:
##########
@@ -154,6 +154,7 @@ Status PartitionSorter::_read_row_num(Block* output_block,
bool* eos, int batch_
}
Status PartitionSorter::_read_row_rank(Block* output_block, bool* eos, int
batch_size) {
+ *eos = false;
Review Comment:
[P1] Poll cancellation during the newly unbounded sink drain
The cross-batch behavior can now keep a qualifying peer group alive for
arbitrarily many batches, but `PartitionBlocks::do_partition_topn_sort()`
checks cancellation in neither its retained-input loop nor its `get_next()`
drain. The sink observes cancellation only after `_split_block_by_partition()`
returns, so a cancelled query can still sort, permute, and copy the entire
remaining pass; final sink preparation has the same root issue. Please poll the
runtime state before each retained-block sort and each output batch and
propagate the cancellation status.
##########
be/src/exec/sort/partition_sorter.cpp:
##########
@@ -182,6 +177,7 @@ Status PartitionSorter::_read_row_rank(Block* output_block,
bool* eos, int batch
// rank() maybe need check when have get a distinct row
// so when the cmp_res is get a distinct row, need check have
output all rows num
if (_get_enough_data()) {
+ *eos = true;
Review Comment:
[P2] Clear stale DENSE_RANK state when resetting the sorter
`reset_sorter_state()` zeros `_output_distinct_rows`, but
`SortCursorCmp::reset()` rewinds the old `impl` instead of invalidating it. If
the next pass begins with the same first key, that group is treated as an
already-seen peer and is not counted, so dense-rank group `limit + 1` is
admitted; mixed retained runs can make this recur across passes. The exact
Window/global phase masks final rows, but not the extra local memory, sorting,
or exchange work. Please clear the comparator implementation on reset and add a
two-pass reuse test.
##########
be/src/exec/sort/partition_sorter.cpp:
##########
@@ -201,6 +197,9 @@ Status PartitionSorter::_read_row_rank(Block* output_block,
bool* eos, int batch
}
}
+ // A full batch can end inside the last qualifying peer group. Continue
reading
+ // until the next group exceeds the rank limit or the merge queue is
exhausted.
+ *eos = !queue.is_valid();
Review Comment:
[P1] Avoid quadratic reprocessing of retained peer groups
This now correctly keeps every qualifying RANK/DENSE_RANK peer, but
intermediate pruning still reruns `append_block()`/`partial_sort()` and copies
the complete retained prefix after each fixed 20,000 fresh rows. An all-equal
10-million-row peer group therefore processes roughly 2.505 billion retained
row instances before the final pass. Please preserve/reuse sorted runs or make
the next trigger grow when a pass cannot reduce the retained result, and cover
multiple thresholds in a focused test.
--
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]
