[CALCITE-7439] Qualify GROUP BY keys for DISTINCT over joins#4829
[CALCITE-7439] Qualify GROUP BY keys for DISTINCT over joins#4829bvolpato wants to merge 4 commits intoapache:mainfrom
Conversation
ae686b6 to
be816c1
Compare
8805604 to
0558ef1
Compare
67bc8f8 to
b6af6a8
Compare
decd96d to
1903b00
Compare
17204e8 to
939c906
Compare
|
|
I will conduct the review over the weekend. |
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions. |
939c906 to
d3361f5
Compare
|
|
[review-prs-verification]
|
| } | ||
|
|
||
| private static boolean isSimpleIdentifier(SqlNode node) { | ||
| return node instanceof SqlIdentifier |
There was a problem hiding this comment.
SqlIdentifier has a method isSimple
| + " FROM \"foodmart\".\"product\" p4\n" | ||
| + ")"; | ||
|
|
||
| final RuleSet rules = |
There was a problem hiding this comment.
can you pull this into a helper function to avoid repeating it?
| } | ||
| } | ||
|
|
||
| private static boolean isMergedJoinKey(SqlJoin fromJoin, |
There was a problem hiding this comment.
although these methods are private having some JavaDoc would help maintain them in the future and review them now.
| } | ||
| } | ||
|
|
||
| private static boolean isQualifiedJoinField(@Nullable SqlNode node, |
There was a problem hiding this comment.
I wonder what happens if you have a ROW field and in the join you use a qualified field deeper than 2. Maybe you can add a test case for that?



Summary
RelToSqlConverter emitted ambiguous
GROUP BYkeys after semi-join rewrites forDISTINCTqueries over joins with merged key columns.This PR qualifies merged join keys when building
GROUP BYand correspondingSELECTkeys from a join-shaped aggregate input:LEFT/INNER-like joins qualify against the left aliasRIGHTjoins qualify against the right aliasFULLjoins useCOALESCE(left.key, right.key)=predicates, not justUSINGReproduction query
Error before this PR
Tests
testPostgresqlRoundTripDistinctLeftJoinInSubqueryWithSemiJoinRulestestPostgresqlRoundTripDistinctLeftJoinUsingTwoKeysWithSemiJoinRulesRIGHTandFULLjoin qualification behaviorRelToSqlConverterTestregression slice locally on Java 21Disclaimer
The changes were done with the assistance of Codex in response to an actual bug when using Calcite in production. The changes were manually verified prior to sending this to review.