Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23316 )
Change subject: IMPALA-14106: Calcite planner: Register equivalent union expressions in value transfer graph ...................................................................... Patch Set 2: (3 comments) http://gerrit.cloudera.org:8080/#/c/23316/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeCreationUtils.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeCreationUtils.java: http://gerrit.cloudera.org:8080/#/c/23316/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeCreationUtils.java@85 PS2, Line 85: registerUnionValueTransfers(analyzer, outputExprs, childrenPlanNodes); Is there any additional check to be done for Union Distinct vs Union All ? I think only for the latter we should be creating the value transfer but I am not sure if we use the same Union plan node for both. If we only use a UnionAll plan node for both and expect that the Distinct part is done below by an Aggregate, then it should be ok. http://gerrit.cloudera.org:8080/#/c/23316/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeCreationUtils.java@119 PS2, Line 119: * Register the union value transfers which allows runtime filter generators Could you expand the comments to indicate that the value transfer is only between the output expr of the Union and each of the child inputs, and not a value transfer between the child inputs themselves. So if you have UNION ALL which has output exprs out_expr1, out_expr2 and it has 2 children: LHS: produces lhs_expr1, lhs_expr2 RHS: produces rhs_expr1, rhs_expr2 The value transfer is NOT between lhs_expr1 <-> rhs_expr1, but rather out_expr1 <-> lhs_expr1 and out_expr1 <-> rhs_expr1. This is different from an equality Join, because there we do create a value transfer between the join columns coming from each child. http://gerrit.cloudera.org:8080/#/c/23316/2/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/NodeCreationUtils.java@134 PS2, Line 134: SlotRef inputExpr = nodeWithExprs.outputExprs_.get(i).unwrapSlotRef(false); My understanding is that the slotRef of the left child of a UnionAll is the same as the slotRef of the output expr of the UnionAll. Do you see a difference ? in other words, if I have SELECT a, b UNION ALL c, d. then the output expr of the UnionAll is already a, b. -- To view, visit http://gerrit.cloudera.org:8080/23316 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4c858ae82a1cb7b89b0ae4e70205d8eeaeb28687 Gerrit-Change-Number: 23316 Gerrit-PatchSet: 2 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: Sat, 20 Sep 2025 19:12:48 +0000 Gerrit-HasComments: Yes
