Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/23679 )
Change subject: IMPALA-14525: Calcite planner: Add support for RexSimplify ...................................................................... Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/23679/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java: http://gerrit.cloudera.org:8080/#/c/23679/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/coercenodes/CoerceOperandShuttle.java@366 PS7, Line 366: private static boolean isNullable(RexNode rexNode) { > So I wanted to use the code that they used within Calcite. That Strong.pol Done http://gerrit.cloudera.org:8080/#/c/23679/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaCoreRules.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaCoreRules.java: http://gerrit.cloudera.org:8080/#/c/23679/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaCoreRules.java@60 PS7, Line 60: public static final RelBuilderFactory LOGICAL_BUILDER_NO_SIMPLIFY = > I mean...you're prolly right. Very few rules actually need it. I would favor removing for the cases where it is not absolutely needed but would interested in seeing if there's any unexpected plan changes. The counter point to #2 is that we should force someone to think about when Impala's constant folding is needed and when it is not. Calcite's default RelBuilder should be used for the second case otherwise we might miss certain optimizations done by that RelBuilder. http://gerrit.cloudera.org:8080/#/c/23679/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinSimplifyRule.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinSimplifyRule.java: http://gerrit.cloudera.org:8080/#/c/23679/7/java/calcite-planner/src/main/java/org/apache/impala/calcite/rules/ImpalaJoinSimplifyRule.java@36 PS7, Line 36: public class ImpalaJoinSimplifyRule extends RelOptRule { > I did this more for safety reasons since I wasn't sure and there's no real I would say if there's not an example for this rule, it is better to take it out. It's not so much about performance of running a rule but the functional side effects of invoking simplify. If there's a complex predicate consisting of both 2 table join predicates anded with single table predicates, there are other rules that would first break it up and push the single table predicate down to below the join and the simplification of that would already be handled by the ImpalaFilterSimplifyRule. -- To view, visit http://gerrit.cloudera.org:8080/23679 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I44792688f361bf15affa565e5de5709f64dcf18c Gerrit-Change-Number: 23679 Gerrit-PatchSet: 7 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: Pranav Lodha <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Steve Carlin <[email protected]> Gerrit-Comment-Date: Mon, 19 Jan 2026 01:41:19 +0000 Gerrit-HasComments: Yes
