Steve Carlin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/22138 )

Change subject: IMPALA-13587: Calcite planner: Outer join not aggregating nulls 
properly
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/22138/3//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/22138/3//COMMIT_MSG@11
PS3, Line 11: select t2.int_col y from alltypessmall t1 left outer join
            : alltypestiny t2 on t1.int_col = t2.int_col group by 1
> How is this patch tested?
The Calcite planner has a special testing framework.  We haven't incorporated 
it into the main testing yet since it's still sorta experimental.

Having said that, you can see the run here (note the url with the 22138 in it):
https://jenkins.impala.io/view/all/job/calcite-report-prototype/135/

And the results of the run here:

https://jenkins.impala.io/view/all/job/calcite-report-prototype/135/artifact/Impala/calcite_report/html/index.html


http://gerrit.cloudera.org:8080/#/c/22138/3//COMMIT_MSG@20
PS3, Line 20: sent to the same node. 
> nit: whitespace
Done


http://gerrit.cloudera.org:8080/#/c/22138/3//COMMIT_MSG@24
PS3, Line 24: found on the outer join side, where it becomse null.
> typo: becomes
Done


http://gerrit.cloudera.org:8080/#/c/22138/3/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaJoinRel.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaJoinRel.java:

http://gerrit.cloudera.org:8080/#/c/22138/3/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaJoinRel.java@150
PS3, Line 150:     // join side could be NULL when the left side is not NULL.
> Should we do anything to check whether otherJoinConjuncts could be NULL?
Not sure I understand why.

We ran into trouble when registering too many conjuncts.  This limits what we 
now register, only putting in equals conjuncts and only if they are a true 
equivalency (not an outer join)



--
To view, visit http://gerrit.cloudera.org:8080/22138
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I57e9d4ad4c4af5a4c268e43ac2937064dab6ffd7
Gerrit-Change-Number: 22138
Gerrit-PatchSet: 3
Gerrit-Owner: Steve Carlin <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Fang-Yu Rao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Michael Smith <[email protected]>
Gerrit-Reviewer: Riza Suminto <[email protected]>
Gerrit-Reviewer: Steve Carlin <[email protected]>
Gerrit-Comment-Date: Tue, 25 Feb 2025 03:14:23 +0000
Gerrit-HasComments: Yes

Reply via email to