Repository navigation
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
jacktengg
requested review from
924060929,
airborne12,
csun5285,
eldenmoon,
englefly,
gavinchou,
liaoxin01,
starocean999 and
yiguolei
as code owners
October 28, 2026 12:49
Contributor
Author
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Reviewed all 20 changed files at d6b4f79. No actionable newly introduced defects were verified. The review converged after 1 round: both complete-review scopes and the additional risk-focused scope returned NO_NEW_VALUABLE_FINDINGS, and every initial risk received an evidence-backed disposition. There are no existing inline findings to carry forward. No additional user focus was supplied.
Critical checkpoint conclusions:
- Goal and evidence: the changes implement stricter eligible single-key TopN bounds, preserve shared predicates during static key normalization, and refresh unread page ranges. Added FE, BE and regression tests cover the central behavior; they were inspected, not executed. The reported performance improvement was not independently measured.
- Scope and reuse: the implementation stays within that filtering path and reuses comparison predicates, descriptor transport, RowRanges and existing Zone Map APIs.
- Concurrency and locks: runtime-bound publication uses RuntimePredicate's lock; SharedPredicate pairs its version and immutable nested predicate under its own lock. Snapshots retain the comparison after unlocking, so page metadata reads do not hold the publication lock. No new reverse lock acquisition was found; cursor/progress state is iterator-local.
- Lifecycle and memory: shared clones retain the live predicate, snapshots own their old bound, and the old roaring cursor is destroyed before bitmap replacement. Heap and storage producers retain enough candidates before publishing; recursive target recreation was traced. No new static initialization dependency or ownership cycle was identified.
- Configuration: no new configuration item or dynamic-update obligation is introduced.
- Compatibility: no storage-format change is introduced. Absent strictness flags retain inclusive behavior on new BE; old BE ignores the optional field.
- Thrift IDs: the full touched TTopnFilterDesc was compared with current branch-3.1, 4.0, 4.1 and 4.2 definitions. Fields 1-4 retain their meanings, and field 5 is unused there and optional with a false default. The inspected branch refs match live GitHub refs.
- Parallel paths: reviewed heap and storage TopN producers, parallel heaps, expression/column/raw/dictionary evaluation, index paths, and forward/reverse scans. Unsupported Zone Map adapters conservatively retain full ranges; schema gating remains in place.
- Conditional checks: direct-scan/single-key/SlotRef eligibility preserves the retained-candidate argument; multi-key and intervening operators remain inclusive. Refresh intersects existing restrictions and excludes consumed rows before rebuilding; the first update and subsequent throttle have explicit behavior.
- Test coverage: inspected eligibility and missing-flag tests, strict scalar/binary/dictionary/null comparisons, key-range preservation, late bounds, both scan directions, partial-page restarts, holes, threshold, EOF and snapshot concurrency. The added fixtures are not exhaustive across all types and adapters; reachable implementation paths were reviewed statically.
- Expected results: all 28 new regression output sections were checked against the generated data and SQL ordering/offset through static calculation. Queries have explicit ordering. Whether output files were generated by the prescribed harness cannot be proved from the checkout.
- Observability: existing raw-row/page counters and Zone Map timers, together with runtime-range filtered-row accounting, expose the changed behavior. No additional logging requirement was identified.
- Transactions and persistence: no transaction, EditLog, replay or persistent metadata path changes; visible-version and delete-bitmap rules are unchanged. Range refresh preserves existing row restrictions.
- Writes and crash behavior: no data-write or commit path is modified. New metadata-read errors propagate through Status; no new write atomicity obligation arises.
- FE-to-BE propagation: both Coordinator and ThriftPlansBuilder use the updated TopnFilter serializer, and RuntimePredicate consumes the optional flag. Comparison function/opcode selection is consistent across the changed evaluators.
- Performance: the unchanged-version path avoids range work, later updates are throttled by raw rows, and immutable snapshots move metadata work outside the producer lock. A refresh copies the remaining bitmap and examines page metadata; no benchmark or latency conclusion is claimed.
- Other issues and remaining questions: no unresolved candidate or uncovered required static-review scope remains. Builds, tests, sanitizers, runtime behavior, mixed-version execution and binary compatibility artifacts were not exercised under this static-only contract.
Review converged after 1 round(s).
Contributor
Author
|
run buildall |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
TPC-H: Total hot run time: 28514 ms |
Contributor
TPC-DS: Total hot run time: 151297 ms |
Contributor
ClickBench: Total hot run time: 25.2 s |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
jacktengg
force-pushed
the
arm-perf
branch
3 times, most recently
from
October 30, 2026 08:58
ab9d4ce to
80e954f
Compare
Contributor
Author
|
/review |
Contributor
Author
|
run buildall |
Contributor
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
ClickBench: Total hot run time: 24.08 s |
Contributor
FE UT Coverage ReportIncrement line coverage `` 🎉 |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 3, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#69776 Problem Summary: A TopN filter on the null-generating side of an outer join can remove matching rows and create new NULL-extended rows. ORDER BY COALESCE(a.k,-1) after a right join can then return -1 when every original a.k is 10. Recursive fragment rebuilds also create empty TopN heaps but reuse the query-level bounds. Those bounds can reject new input before the new heap retains any candidates, which can terminate recursion prematurely. Reuse the Join runtime filter's null-propagation proof to check TopN pushdown through null-generating sides of hash and nested-loop joins. Reset only rebuilt TopN sources after old tasks drain and before new tasks are submitted. Clear scalar and storage bounds and reset predicate versions so the first new bound takes effect promptly. Preserve boundary sharing among parallel instances within one execution. ### Release note Fix incorrect TopN results for outer joins with NULL-converting sort expressions and recursive CTEs containing TopN runtime filters. ### Check List (For Author) - Test: ASAN BE/FE build; 61 BE unit tests; 61 FE unit tests; 6 regression suites; 5 filter-off/on result pairs; manual reproduction on the prior build; plan checks; clang-format v16; header hygiene; FE Checkstyle; clang-tidy - Behavior changed: Yes. Block unsafe outer-join filter pushdown and reset TopN bounds at recursive execution boundaries to preserve correct results - Does this need documentation: No
16 tasks
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 3, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. For Heap TopN with one actual sort key, these rows cannot improve the result after the heap has retained LIMIT + OFFSET candidates. Select LT or GT bounds in BE for a Heap TopN with one sort key. Keep LE or GE bounds for multiple sort keys. Apply the selected mode to storage predicates, column expressions, raw values, dictionary evaluation, and external predicate conversion. Preserve NULL ordering and support scan preparation before source initialization. Keep published bounds during parallel initialization within one execution. Clear the bounds and storage predicate versions when a recursive fragment is rebuilt, because the new heap has no candidates to support the old bound. Require null propagation when FE pushes a TopN filter through the left side of a right or full outer join. Otherwise, removing a matching boundary row can create a NULL-extended row whose expression value sorts before the boundary. For example, COALESCE(a.k, -1) can produce -1 after the filter removes a.k = 10. ### Release note Heap TopN runtime filters with one sort key can discard rows equal to the boundary. Multiple sort keys retain inclusive bounds. ### Check List (For Author) - Test: Manual checks of commit boundaries, dependencies, generated-result preservation, whitespace, clang-format v16, and header hygiene. Include BE and FE unit tests, strict-bound and safety regression suites, and strict-bound scan-key cases. Builds and tests were not rerun for this commit split. - Behavior changed: Yes. Single-key Heap TopN uses strict runtime bounds, and FE blocks unsafe filter pushdown through null-generating outer join inputs. - Does this need documentation: No
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 3, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. For Heap TopN with one actual sort key, these rows cannot improve the result after the heap has retained LIMIT + OFFSET candidates. Select LT or GT bounds in BE for a Heap TopN with one sort key. Keep LE or GE bounds for multiple sort keys. Apply the selected mode to storage predicates, column expressions, raw values, dictionary evaluation, and external predicate conversion. Preserve NULL ordering and support scan preparation before source initialization. Keep published bounds during parallel initialization within one execution. Clear the bounds and nested storage predicates when a recursive fragment is rebuilt, because the new heap has no candidates to support the old bound. Require null propagation when FE pushes a TopN filter through the left side of a right or full outer join. Otherwise, removing a matching boundary row can create a NULL-extended row whose expression value sorts before the boundary. For example, COALESCE(a.k, -1) can produce -1 after the filter removes a.k = 10. ### Release note Heap TopN runtime filters with one sort key can discard rows equal to the boundary. Multiple sort keys retain inclusive bounds. ### Check List (For Author) - Test: Manual verification of removed page-range changes, preserved strict-bound changes and generated regression results, dependency references, and whitespace excluding generated-result EOF separators. Conflict resolutions pass clang-format v16 and BE header hygiene. Includes BE and FE unit tests and strict-bound and safety regression suites. Builds and tests were not rerun after rebasing. - Behavior changed: Yes. Single-key Heap TopN uses strict runtime bounds, and FE blocks unsafe filter pushdown through null-generating outer join inputs. - Does this need documentation: No
jacktengg
changed the title
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 3, 2026
Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. Once a Heap TopN has retained LIMIT + OFFSET candidates, equal boundary values cannot improve a single-key result. Multiple sort keys still need inclusive bounds. Choose strict bounds in FE from the actual SortNode algorithm and final ordering expressions. Serialize this final decision in the optional TTopnFilterDesc.use_strict field. Both FE dispatch paths share toThrift(), after runtime-predicate algorithm selection where applicable. Plans omitting this decision or explicitly setting it to false retain inclusive bounds. BE constructs an immutable comparison mode from the descriptor and prepares one comparison function. Apply LT/GT or LE/GE consistently to storage predicates, column expressions, raw values, dictionary evaluation and external predicate conversion. Preserve NULL ordering and scan preparation before source initialization. Keep published bounds across parallel source initialization. Clear scalar and nested storage bounds when a recursive fragment is rebuilt, because the new heap has no candidates supporting the previous execution's boundary. Require null propagation when FE pushes a TopN filter through the left side of a right or full outer join. Otherwise, filtering a matching row can create a NULL-extended row with a better expression value: COALESCE(a.k, -1) becomes -1 after removing a.k = 10. Legacy FE plans remain inclusive on upgraded BEs. Thrift field 6 is optional and defaults to false. Its ID and the existing field meanings were checked against branch-3.1, branch-4.0, branch-4.1, branch-4.2 and master. Single-key Heap TopN runtime filters can discard rows equal to the boundary. Multiple keys and legacy plans without the FE's final decision keep inclusive bounds. Unsafe filter pushdown through null-generating outer joins is blocked. - Test: Unit tests: run-fe-ut.sh --run org.apache.doris.nereids.postprocess.TopNRuntimeFilterTest (12 passed), including forced heap/topn/full algorithms, one/multiple keys and redundant order-key removal. run-be-ut.sh --run with RuntimePredicateTest.*, SortOperatorTest.*, HeapSorterTest.* and FragmentMgrCrossClusterCancelTest.* (30 passed). FE Checkstyle, clang-format v16 on modified C++ files, BE header hygiene, modified-line clang-tidy and cross-branch Thrift audit passed. Missing and explicit-false planner decisions both retain boundary rows in ascending and descending scans. Mixed-version cluster regression was not run. - Behavior changed: Yes. FE decides strict bounds for safe single-key Heap TopN plans; other plans remain inclusive. Recursive executions clear stale bounds and unsafe outer-join filter targets are rejected. - Does this need documentation: No
jacktengg
force-pushed
the
arm-perf
branch
2 times, most recently
from
November 3, 2026 08:57
0a7ad95 to
60cc311
Compare
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 3, 2026
Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. Once a Heap TopN has retained LIMIT + OFFSET candidates, equal boundary values cannot improve a single-key result. Multiple sort keys still need inclusive bounds. Choose strict bounds in FE from the actual SortNode algorithm and final ordering expressions. Serialize this final decision in the optional TTopnFilterDesc.use_strict field. Both FE dispatch paths share toThrift(), after runtime-predicate algorithm selection where applicable. Plans omitting this decision or explicitly setting it to false retain inclusive bounds. BE constructs an immutable comparison mode from the descriptor and prepares one comparison function. Apply LT/GT or LE/GE consistently to storage predicates, column expressions, raw values, dictionary evaluation and external predicate conversion. Preserve NULL ordering and scan preparation before source initialization. Keep published bounds across parallel source initialization. Clear scalar and nested storage bounds when a recursive fragment is rebuilt, because the new heap has no candidates supporting the previous execution's boundary. Require null propagation when FE pushes a TopN filter through the left side of a right or full outer join. Otherwise, filtering a matching row can create a NULL-extended row with a better expression value: COALESCE(a.k, -1) becomes -1 after removing a.k = 10. Legacy FE plans remain inclusive on upgraded BEs. Thrift field 6 is optional and defaults to false. Its ID and the existing field meanings were checked against branch-3.1, branch-4.0, branch-4.1, branch-4.2 and master. Single-key Heap TopN runtime filters can discard rows equal to the boundary. Multiple keys and legacy plans without the FE's final decision keep inclusive bounds. Unsafe filter pushdown through null-generating outer joins is blocked. - Test: Unit tests: run-fe-ut.sh --run org.apache.doris.nereids.postprocess.TopNRuntimeFilterTest (12 passed), including forced heap/topn/full algorithms, one/multiple keys and redundant order-key removal. run-be-ut.sh --run with RuntimePredicateTest.*, SortOperatorTest.*, HeapSorterTest.* and FragmentMgrCrossClusterCancelTest.* (30 passed). FE Checkstyle, clang-format v16 on modified C++ files, BE header hygiene, modified-line clang-tidy and cross-branch Thrift audit passed. Missing and explicit-false planner decisions both retain boundary rows in ascending and descending scans. Mixed-version cluster regression was not run. - Behavior changed: Yes. FE decides strict bounds for safe single-key Heap TopN plans; other plans remain inclusive. Recursive executions clear stale bounds and unsafe outer-join filter targets are rejected. - Does this need documentation: No
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 4, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. Once a Heap TopN has retained LIMIT + OFFSET candidates, equal boundary values cannot improve a single-key result. Multiple sort keys still need inclusive bounds. Choose strict bounds in FE from the actual SortNode algorithm and final ordering expressions. Serialize this decision in optional field 5, TTopnFilterDesc.use_strict. Both FE dispatch paths share toThrift() after runtime-predicate algorithm selection. Missing or false decisions retain inclusive bounds, including legacy FE plans on upgraded BEs. BE constructs an immutable comparison mode and applies LT/GT or LE/GE consistently to storage predicates, column expressions, raw values, dictionary evaluation and external predicate conversion. Preserve NULL ordering and scan preparation before source initialization. Keep bounds across parallel source initialization. Clear scalar and nested storage bounds when a recursive fragment is rebuilt, because a new heap has no candidates supporting the previous execution's boundary. Require null propagation when pushing a TopN filter through the left side of a right or full outer join. Stop pushdown at AssertNumRows: an early UNION sibling bound can otherwise remove a scalar subquery's last input, causing AssertNumRows to synthesize a NULL that wins a NULLS FIRST TopN. Preserve filters on safe sibling branches and TopNs inside the assertion. Stop pushdown at hash aggregation as well. A bound from a UNION sibling can empty a scalar SUM input and synthesize NULL, or discard the remaining rows of an already-started group and emit a partial aggregate result. Such a row can survive a boundary tie in the source heap. Keep independent TopN filters inside the aggregate and on safe UNION siblings. Validate join output semantics using the original probe expression before any equality substitution. Reject probe-side filtering through RIGHT ANTI (hash and nested-loop, including constant probes), and marked RIGHT SEMI. Otherwise pruning a match can create an unmatched right row or change a retained mark. For RIGHT/FULL outer joins require an original left-side, NULL-propagating expression: replacing a right equality key with a left key would preserve rank while changing the output payload. Preserve separately safe inner, left-preserving and unmarked right-semi paths. Require at least one original left input slot: nonfolded slot-free expressions such as CAST(sleep(0) AS INT) do not become NULL on an unmatched right output. Reject fully substituted scan probes containing volatile expressions using the shared recursive containsVolatileExpression contract. Scan predicates and source projections have separate function states: strict filtering of IF(rand(3) < 0.5, 0, 1) can shift the projection sequence and lose its minimum. The final target-registration guard covers nested expressions, aliases, UNION mappings, joins and declared volatile UDFs, while retaining safe sibling filters and deterministic/query-stable expressions. ### Release note Single-key Heap TopN runtime filters can discard rows equal to the boundary. Multiple sort keys and legacy plans retain inclusive bounds. Prevent unsafe pushdown through null-generating outer joins, scalar row-count assertions, and aggregation; prevent probe filtering from creating or changing right-side join outputs; disable unsafe runtime filtering of volatile ordering expressions; clear stale boundaries between recursive executions. ### Check List (For Author) - Test: Unit Test: ./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.TopNRuntimeFilterTest (17 passed). Both aggregate barrier tests fail before this correction and pass afterward. Coverage includes scalar/grouped SUM, 16 ordering/NULL-placement/inner-TopN combinations, serialized safe UNION targets, COUNT/DISTINCT/HAVING/join plans, and the previously added AssertNumRows cases. Join coverage includes 144 join-type/algorithm/probe/order combinations (including nonfolded SLEEP(0)) and four commuted hash RIGHT_SEMI mark plans. Another 40 expression plans cover nested/slot-bearing RAND and UUID, standalone volatile sources, deterministic controls and safe serialized UNION targets. FE Checkstyle passed with zero violations. Added regression EXPLAIN checks for aggregate barriers; these were not run on a live cluster. No BE rebuild or live-cluster regression run was performed for this FE-only correction. - Behavior changed: Yes. Safe single-key Heap TopNs use strict bounds; unsafe outer/anti/marked-join, AssertNumRows, aggregate and volatile expression targets are rejected; recursive executions clear stale bounds. - Does this need documentation: No
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#69776
commit: 0312f2e60fc86d30bae2c5ddbe890538a4085087
base: a35707da74f8df0d6502df95feaf734dd9d20d9d
reviewed_at: 2026-11-04T21:46+08:00
reviewer: jacktengg
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
jacktengg
added a commit
to jacktengg/incubator-doris
that referenced
this pull request
Nov 4, 2026
### What problem does this PR solve? Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. Once a Heap TopN has retained LIMIT + OFFSET candidates, equal boundary values cannot improve a single-key result. Multiple sort keys still need inclusive bounds. Choose the comparison mode in BE from each source's actual TSortNode algorithm and final ordering expressions. Inspect the request's fragment plans while creating QueryContext, before publishing it to other fragments. Keep the mode immutable so a scan prepared before its source uses the same comparison as the storage predicate. An absent source plan or sort algorithm retains inclusive comparisons. Remove the unpublished use_strict Thrift field and the corresponding FE decision; reuse the existing sort metadata. Apply LT/GT or LE/GE consistently to storage predicates, column expressions, raw values, dictionary evaluation and external predicate conversion, while preserving NULL ordering. Keep bounds across parallel source initialization. Clear scalar and nested storage bounds when a recursive fragment is rebuilt, because the new heap has no candidates supporting the previous boundary. Keep FE pushdown safety checks. Stop at aggregation and AssertNumRows: filtering their input can synthesize NULL rows or change an aggregate result. Reject probe filtering through RIGHT ANTI and marked RIGHT SEMI joins, where removing a match can create or change a right-side output. Validate original probe slots before equality substitution; RIGHT/FULL outer joins require an original left-side, NULL-propagating expression and NULLS LAST. Slot-free expressions cannot satisfy this condition. Reject fully substituted probes containing volatile expressions, whose independent scan/projection function states can otherwise produce incorrect results. Preserve safe sibling and inner TopN filters. ### Release note Single-key Heap TopN runtime filters can discard rows equal to the boundary. Multiple sort keys retain inclusive bounds. Prevent unsafe pushdown through aggregation, scalar assertions, outer/anti/marked joins and volatile ordering expressions; clear stale boundaries between recursive executions. ### Check List (For Author) - Test: Unit Test and build: ./build.sh --be -j24 (ASAN) passed. ./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.TopNRuntimeFilterTest passed all 17 tests. ./run-be-ut.sh passed 36 distinct targeted tests across RuntimePredicateTest, SortOperatorTest, recursive fragment rebuild, Parquet TopN and Iceberg TopN; all 5 SortOperatorTest tests passed again after test-helper refactoring. Coverage includes actual algorithms, single/multiple ordering keys, absent source/algorithm, multiple source IDs, scan-before-source preparation, comparison paths and bound reset/preservation. FE Checkstyle, clang-format 16, build hygiene and clang-tidy passed for the changed code. No live-cluster regression run was performed. - Behavior changed: Yes. Safe single-key Heap TopNs use strict bounds; unsafe pushdown targets are rejected and recursive executions clear bounds. - Does this need documentation: No
Contributor
Author
|
/review |
Contributor
There was a problem hiding this comment.
Review completed at exact PR head cf7c0db (base 19dfb87). I inspected all 22 paths in the authoritative changed-file list and diff, including strict TopN comparison propagation, fragment metadata selection, recursive rebuild bound reset, scanner/index evaluation, FE pushdown through projects/unions/joins/aggregates/asserts/CTEs/windows, serialization, and the added BE/FE/regression coverage. The three assigned review scopes all returned NO_NEW_VALUABLE_FINDINGS, and the main risk items were independently dismissed with evidence. The existing inline P2 thread 4432112388 was revalidated as unchanged legacy context; it is not a new finding and no P0/P1 existing blocking comment applies. The user provided no additional focus points. This was a static-only review: no builds, tests, source edits, or runtime validation were performed.
Review converged after 1 round(s).
Contributor
Author
|
run buildall |
### What problem does this PR solve? Issue Number: None Related PR: apache#69776 Problem Summary: TopN runtime filters retain rows equal to the current boundary. Once a Heap TopN has retained LIMIT + OFFSET candidates, equal boundary values cannot improve a single-key result. Multiple sort keys still need inclusive bounds. Choose the comparison mode in BE from each source's actual TSortNode algorithm and final ordering expressions. Inspect the request's fragment plans while creating QueryContext, before publishing it to other fragments. Keep the mode immutable so a scan prepared before its source uses the same comparison as the storage predicate. An absent source plan or sort algorithm retains inclusive comparisons. Remove the unpublished use_strict Thrift field and the corresponding FE decision; reuse the existing sort metadata. Apply LT/GT or LE/GE consistently to storage predicates, column expressions, raw values, dictionary evaluation and external predicate conversion, while preserving NULL ordering. Source detection preserves any existing bound. Clear scalar and nested storage bounds when a recursive fragment is rebuilt, because the new heap has no candidates supporting the previous boundary. Keep FE pushdown safety checks. Stop at aggregation and AssertNumRows: filtering their input can synthesize NULL rows or change an aggregate result. Reject probe filtering through RIGHT ANTI and marked RIGHT SEMI joins, where removing a match can create or change a right-side output. Validate original probe slots before equality substitution; RIGHT/FULL outer joins require an original left-side, NULL-propagating expression and NULLS LAST. Slot-free expressions cannot satisfy this condition. Reject fully substituted probes containing volatile expressions, whose independent scan/projection function states can otherwise produce incorrect results. Preserve safe sibling and inner TopN filters. ### Release note Single-key Heap TopN runtime filters can discard rows equal to the boundary. Multiple sort keys retain inclusive bounds. Prevent unsafe pushdown through aggregation, scalar assertions, outer/anti/marked joins and volatile ordering expressions; clear stale boundaries between recursive executions. ### Check List (For Author) - Test: Unit Test and build: ./build.sh --be -j24 (ASAN) passed. ./run-fe-ut.sh --run org.apache.doris.nereids.postprocess.TopNRuntimeFilterTest passed all 17 tests. ./run-be-ut.sh passed 36 distinct targeted tests across RuntimePredicateTest, SortOperatorTest, recursive fragment rebuild, Parquet TopN and Iceberg TopN; all 5 SortOperatorTest tests passed again after test-helper refactoring. Coverage includes actual algorithms, single/multiple ordering keys, absent source/algorithm, multiple source IDs, scan-before-source preparation, comparison paths and bound reset/preservation. After the lifecycle-comment correction, all 27 selected RuntimePredicate, SortOperator and recursive-rebuild tests passed again. FE Checkstyle, clang-format 16, build hygiene and clang-tidy passed for the changed code. No live-cluster regression run was performed. - Behavior changed: Yes. Safe single-key Heap TopNs use strict bounds; unsafe pushdown targets are rejected and recursive executions clear bounds. - Does this need documentation: No
Contributor
TPC-H: Total hot run time: 28767 ms |
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#69776
commit: 2ba41429d5b5e9ea2f34c68ead3d6abfd2f72bbf
base: 19dfb8711ae3dbb8f56cc945f5dc7af20a844607
reviewed_at: 2026-11-05T01:36+08:00
reviewer: jacktengg
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
Author
|
run buildall |
Contributor
TPC-DS: Total hot run time: 153080 ms |
Contributor
ClickBench: Total hot run time: 24.15 s |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
TopN runtime filters retain rows equal to the current boundary. For Heap TopN
with one actual sort key, these rows cannot improve the result after the heap
has retained LIMIT + OFFSET candidates.
Select LT or GT bounds in BE for a Heap TopN with one sort key. Keep LE or GE
bounds for multiple sort keys. Apply the selected mode to storage predicates,
column expressions, raw values, dictionary evaluation, and external predicate
conversion. Preserve NULL ordering and support scan preparation before source
initialization.
Source detection preserves existing bounds. Recursive rebuild resets bounds before the next execution.
Clear the bounds and nested storage predicates when a recursive fragment is
rebuilt, because the new heap has no candidates to support the old bound.
Require null propagation when FE pushes a TopN filter through the left side of
a right or full outer join. Otherwise, removing a matching boundary row can
create a NULL-extended row whose expression value sorts before the boundary.
For example, COALESCE(a.k, -1) can produce -1 after the filter removes a.k = 10.
Release note
Heap TopN runtime filters with one sort key can discard rows equal to the
boundary. Multiple sort keys retain inclusive bounds.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)