Joe McDonnell 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: select int_col from functional.alltypes where tinyint_col = 100; 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 filters, so we need to combine them along the way down to the scan node. Separately, are we expecting multiple predicates to work right now? i.e. select int_col from functional.alltypes where int_col = 1000 and bool_col; Right now, this gets me: I0612 12:56:57.740746 1343191 CalciteJniFrontend.java:143] 2045d49fadce3d7c:a4a17e8a00000000] Stack Trace:java.lang.IllegalStateException: eq(functional.alltypes.int_col, 1000) at com.google.common.base.Preconditions.checkState(Preconditions.java:512) at org.apache.impala.planner.PlanNode.orderConjunctsByCost(PlanNode.java:1203) at org.apache.impala.planner.HdfsScanNode.init(HdfsScanNode.java:449) at org.apache.impala.calcite.rel.node.ImpalaHdfsScanRel.getPlanNode(ImpalaHdfsScanRel.java:84) at org.apache.impala.calcite.rel.node.ImpalaFilterRel.getChildPlanNode(ImpalaFilterRel.java:90) 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 like: select int_col from functional.alltypes where bool_col -- 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 <[email protected]> Gerrit-Reviewer: Aman Sinha <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Comment-Date: Wed, 12 Jun 2024 20:03:13 +0000 Gerrit-HasComments: Yes
