Steve Carlin has posted comments on this change. ( http://gerrit.cloudera.org:8080/21498 )
Change subject: IMPALA-12871: Added filtering capability for Calcite planner ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java: http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java@79 PS1, Line 79: // need to set the inputRefs. The HdfsScan RelNode needs to know which columns are : // needed from the table in order to implement the filter condition. The input ref : // used here may nor may not be projected out. So a union needs to be done with : // potential existing projected input refs from a parent RelNode. : // Note that if the parent RelNode hasn't set any input refs, it is assumed that all : // input refs are needed (the default case when inputRefs_ is null). > So, I'm assuming this would be something like: Correct http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaFilterRel.java@98 PS1, Line 98: List<RexNode> conditions = ImmutableList.of(previousCondition, newCondition); : return RexUtil.composeConjunction(getCluster().getRexBuilder(), conditions); > For my own understanding: I assume this is about having multiple layers of Unfortunately, this will not work right now. This works in a future commit. The reason it won't work is because the "and" and "or" clause cannot go through the FunctionCallExpr object. They need to use the special CompoundExpr which will be pushed in a later commit. http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ExprConjunctsConverter.java File java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ExprConjunctsConverter.java: http://gerrit.cloudera.org:8080/#/c/21498/1/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/util/ExprConjunctsConverter.java@87 PS1, Line 87: // If it's not a RexCall, there's no AND operator and we can : // just return the conjunct. : if (!(conjunct instanceof RexCall)) { : return ImmutableList.of(conjunct); : } > For my own curiosity, when would we hit this case? Would it be something li I'm not sure if it's necessary for this commit, but this would hold true for literals. -- To view, visit http://gerrit.cloudera.org:8080/21498 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If104bf1cd801d5ee92dd7e43d398a21a18be5d97 Gerrit-Change-Number: 21498 Gerrit-PatchSet: 1 Gerrit-Owner: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Comment-Date: Mon, 17 Jun 2024 17:09:14 +0000 Gerrit-HasComments: Yes
