-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BugFix] fix agg in order by not being analyzed correctly #30108
Conversation
Signed-off-by: packy92 <[email protected]>
Signed-off-by: packy92 <[email protected]>
Signed-off-by: packy92 <[email protected]>
Signed-off-by: packy92 <[email protected]>
Signed-off-by: packy92 <[email protected]>
Signed-off-by: packy <[email protected]>
SonarCloud Quality Gate failed.
|
[BE Incremental Coverage Report]😍 pass : 0 / 0 (0%) |
[FE Incremental Coverage Report]😍 pass : 10 / 10 (100.00%) file detail
|
@Mergifyio backport branch-3.1 |
@Mergifyio backport branch-3.0 |
✅ Backports have been created
|
✅ Backports have been created
|
--------- Signed-off-by: packy92 <[email protected]> Signed-off-by: packy <[email protected]> (cherry picked from commit eea93ef)
--------- Signed-off-by: packy92 <[email protected]> Signed-off-by: packy <[email protected]> (cherry picked from commit eea93ef) # Conflicts: # fe/fe-core/src/main/java/com/starrocks/sql/analyzer/AggregationAnalyzer.java # fe/fe-core/src/main/java/com/starrocks/sql/analyzer/SelectAnalyzer.java # fe/fe-core/src/test/java/com/starrocks/sql/analyzer/AnalyzeAggregateTest.java
…tarRocks#30108) Signed-off-by: packy <[email protected]>
…tarRocks#30108) Signed-off-by: packy <[email protected]>
…tarRocks#30108) Signed-off-by: packy <[email protected]>
…30108) (#30298) Signed-off-by: packy <[email protected]>
…30108) (#30294) Signed-off-by: packy <[email protected]>
…30108) (#30427) Signed-off-by: packy <[email protected]>
…30108) --------- Signed-off-by: packy92 <[email protected]> Signed-off-by: packy <[email protected]>
--------- Signed-off-by: packy92 <[email protected]>
…30108) (#30237) Signed-off-by: packy92 <[email protected]> Co-authored-by: packy92 <[email protected]>
Fixes #issue
For sql like
The statement had been transformed to a wrong plan fragment like:
This reason is the scope for analyzing order by is combined querySource scope and output scope. For the order by elements in the sql, its scope is like
output scope: v1(tblName null), source scope: v1(tblName t0), v2(tblName t0), v3(tblName t0)
. When analyzemin(v1)
, the v1 matched withv1(tblName null)
firstly, while it should bev1(tblName t0)
. Even the old code would analyze the aggregations using source scope in the later step, the fisrt analyzed process has put theslotRef
in a map. The second analyzed process may change theslotRef
already in the map, leading to unexpect behavior when get obejct from the map.So, the better way is to distinguish the aggregation func from the normal func in the analyze orderby stage, and use the corresponding scope to process them respectively.
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check: