Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23313 )

Change subject: IMPALA-14102: [part 2] Fixed the JoinTranspose rule.
......................................................................


Patch Set 6:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/23313/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/23313/6//COMMIT_MSG@20
PS6, Line 20: tranpose
nit: typo 'transpose'


http://gerrit.cloudera.org:8080/#/c/23313/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinProjectTransposeRule.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinProjectTransposeRule.java:

http://gerrit.cloudera.org:8080/#/c/23313/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinProjectTransposeRule.java@60
PS6, Line 60: pushed
nit: this rule is 'pulling' above the join.  'Push' would imply pushing below 
the join.


http://gerrit.cloudera.org:8080/#/c/23313/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinProjectTransposeRule.java@61
PS6, Line 61:  * While the JoinProjectTranspose rule handles this, it has one 
flaw. If there is a
To clarify, suppose I have a plan:
   Join_1
  /      \
       Project_1
            \
         Join_2
        /      \
            Project_2
This rule will try to pull up Project_1 above Join_1 but leave Project_2 as is. 
I am curious why the row width argument does not apply to Project_1.


http://gerrit.cloudera.org:8080/#/c/23313/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinProjectTransposeRule.java@93
PS6, Line 93:     if (join.getJoinType() == JoinRelType.SEMI) {
For semi-joins, there's a different transpose rule in Calcite:  
core/src/main/java/org/apache/calcite/rel/rules/SemiJoinProjectTransposeRule.java

Will that be included in the set of rules to trigger ?


http://gerrit.cloudera.org:8080/#/c/23313/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java
File 
java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java:

http://gerrit.cloudera.org:8080/#/c/23313/6/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java@231
PS6, Line 231:     // XXX: add comment about project
nit: remove the XXX annotation.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7f62ec030fc8fbe36e6150bf96c9673c44b7da1b
Gerrit-Change-Number: 23313
Gerrit-PatchSet: 6
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-Comment-Date: Sun, 07 Sep 2025 00:32:24 +0000
Gerrit-HasComments: Yes

Reply via email to