Uh oh!
There was an error while loading. Please reload this page.
[fix](be) Stabilize conjunct cost ordering - #64637
Conversation
### What problem does this PR solve? Issue Number: None Related PR: None Problem Summary: When enable_adjust_conjunct_order_by_cost is enabled, conjuncts are sorted by execute_cost with std::ranges::sort. Equal-cost conjuncts may have their original relative order changed, which can introduce unnecessary evaluation-order side effects. This patch switches the cost ordering paths to std::ranges::stable_sort so lower-cost conjuncts are still prioritized while equal-cost conjuncts keep their original order. ### Release note None ### Check List (For Author) - Test: Manual test - Ran clang-format v16 on changed C++ files - Ran git diff --check - BE build was started but interrupted per request; not used as validation - Behavior changed: Yes. Equal-cost conjuncts now preserve their original order when enable_adjust_conjunct_order_by_cost is enabled. - Does this need documentation: No
hello-stephen
commented
Jun 18, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
BiteTheDDDDt
commented
Jun 18, 2026
run buildall |
BiteTheDDDDt
commented
Jun 18, 2026
/review |
There was a problem hiding this comment.
No blocking issues found in this review.
Code-review checkpoint conclusions:
- Goal: The PR switches all existing
enable_adjust_conjunct_order_by_cost()cost-ordering sites from unstable sort to stable sort, so equal-cost conjuncts preserve the current input order while lower-cost conjuncts are still prioritized. - Test coverage: No automated test specifically proves equal-cost tie preservation; the PR reports manual validation only. I consider this a residual test gap, not a blocker for this small mechanical fix.
- Scope: The modification is small and focused: three sort calls plus the required
<algorithm>includes. - Concurrency: The only changed concurrent path is late-arrival runtime filters in
ScanLocalStateBase::update_late_arrival_runtime_filter; it remains serialized by_conjuncts_lock, and scanners still clone under the same lock. No new lock-order issue found. - Lifecycle/static initialization: No new ownership, static initialization, or lifecycle coupling.
- Configuration/compatibility: No new config, protocol, persistence, storage format, or FE-BE compatibility changes.
- Parallel paths: I searched the changed option and
execute_cost()ordering paths; the three existing config-gated sort paths are all covered. - Error handling/data correctness: Status propagation and expression prepare/open/evaluation flow are unchanged; no silent failure path introduced.
- Observability: No new observability appears necessary for this local ordering behavior.
- Performance:
stable_sortmay do slightly more work thansort, but these conjunct vectors are expected to be small and the existing hot-path structure is unchanged. - User focus: No additional user-provided focus points were present.
CI note: the visible macOS BE UT failure fails before build/test execution due to the runner using JDK 25 while the script requires JDK 17, so I did not treat that as caused by this patch.
hello-stephen
commented
Jun 18, 2026
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Jun 18, 2026
TPC-H: Total hot run time: 29690 ms |
hello-stephen
commented
Jun 18, 2026
TPC-DS: Total hot run time: 176160 ms |
hello-stephen
commented
Jun 18, 2026
ClickBench: Total hot run time: 25.43 s |
PR approved by at least one committer and no changes requested. |
PR approved by anyone and no changes requested. |
Uh oh!
There was an error while loading. Please reload this page.
Problem Summary: When `enable_adjust_conjunct_order_by_cost` is enabled, conjuncts are sorted by `execute_cost` with `std::ranges::sort`. Equal-cost conjuncts may have their original relative order changed, which can introduce unnecessary evaluation-order side effects. This patch switches the cost ordering paths to `std::ranges::stable_sort` so lower-cost conjuncts are still prioritized while equal-cost conjuncts keep their original order.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: When
enable_adjust_conjunct_order_by_costis enabled, conjuncts are sorted byexecute_costwithstd::ranges::sort. Equal-cost conjuncts may have their original relative order changed, which can introduce unnecessary evaluation-order side effects. This patch switches the cost ordering paths tostd::ranges::stable_sortso lower-cost conjuncts are still prioritized while equal-cost conjuncts keep their original order.Release note
None
Check List (For Author)
clang-formatv16 on changed C++ filesgit diff --checkenable_adjust_conjunct_order_by_costis enabled.