[fix](nereids) Require safe proofs for aggregate group key elimination - #67884
Open
morrySnow wants to merge 1 commit into
Open
[fix](nereids) Require safe proofs for aggregate group key elimination#67884morrySnow wants to merge 1 commit into
morrySnow wants to merge 1 commit into
Conversation
morrySnow
requested review from
924060929,
englefly and
starocean999
as code owners
September 11, 2026 17:49
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
morrySnow
force-pushed
the
fix/safe-aggregate-group-key-simplification
branch
from
September 11, 2026 20:59
5b56fe2 to
2f35bfb
Compare
Contributor
Author
|
run buildall |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 16753 ms |
Contributor
TPC-DS: Total hot run time: 81680 ms |
Contributor
ClickBench: Total hot run time: 14.68 s |
morrySnow
marked this pull request as draft
September 12, 2026 03:03
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Aggregate group-key rewrites treated expressions that referenced the same slot as interchangeable. One rule could synthesize a missing determinant, while another relied only on child functional dependencies. Non-injective or potentially failing expressions could therefore be removed, merging distinct groups or suppressing evaluation errors. Require an existing bare-slot determinant for local simplification, share fail-closed expression safety checks with functional-dependency elimination, and resolve Project/Alias lineage only when it is complete and unambiguous. Safe integral dependencies and ordinary schema functional dependencies remain optimized, and required output keys keep the existing ANY_VALUE ExprId rewrite.
### Release note
Aggregate group-key elimination now preserves non-injective or potentially failing expressions while retaining proven-safe integral and schema-FD optimizations.
### Check List (For Author)
- Test:
- Unit Test: EliminateGroupByKeyTest and SimplifyAggGroupByTest (39 tests).
- Regression test: simplify_agg_group_by force-generated output and normal run.
- Build: DISABLE_BUILD_UI=ON ./build.sh --fe, including FE checkstyle.
- Behavior changed: Yes. Unsafe group-key elimination is disabled when no existing bare-slot determinant is available, expression evaluation can fail, or lineage cannot be proven; proven-safe elimination remains enabled.
- Does this need documentation: No.
morrySnow
force-pushed
the
fix/safe-aggregate-group-key-simplification
branch
from
September 12, 2026 11:07
2f35bfb to
ce11ba3
Compare
Contributor
Author
|
run buildall |
morrySnow
marked this pull request as ready for review
September 12, 2026 11:08
Contributor
TPC-H: Total hot run time: 16875 ms |
Contributor
TPC-DS: Total hot run time: 82827 ms |
Contributor
ClickBench: Total hot run time: 14.81 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.
Problem
Aggregate group-key rewrites could remove expressions merely because they referenced the same input slot or were redundant according to child functional dependencies. That can merge distinct groups and can suppress observable errors. For example, a DECIMAL(38,0) key multiplied by 10 must raise arithmetic overflow for the maximum value, but dropping that key made the query succeed.
Root cause
The local simplification rule inferred a determinant from all group expressions instead of requiring one to exist. A second functional-dependency rule compared only input-slot sets, so
dandd * 10appeared equivalent. Neither path proved that a derived expression was total, nor did the latter safely resolve Project/Alias-produced slots. The generic injective-cast predicate is also too broad for character and complex types.Reproduction
Create a table containing the maximum DECIMAL(38,0) value and run
GROUP BY d, d * 10. The multiplication is evaluated by SQL and raises arithmetic overflow. Before this change, group-key elimination could discardd * 10and suppress the error. Related non-injective decimal expressions could also collapse groups.Fix
Tests