This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 4290eb7dc5451b12878566ba881c077fe19ad3b4 Author: Steve Carlin <[email protected]> AuthorDate: Fri Nov 8 15:33:16 2024 -0800 IMPALA-13576: Fix filter placement in the plan and related changes. This patch combines the following changes: - Filter above a Sort RelNode is now supported (IMPALA-13576) - A new rule is added to remove empty sort keys, as in the example SELECT '1' FROM alltypestiny ORDER BY 1 This also required bumping up the Calcite version to 1.37. (IMPALA-13578) - A parser fix to allow LIMIT clause in a subquery (IMPALA-13579) - Added optimization to push Filter past the Project RelNode (IMPALA-13577) This optimization needs to be added before "CoerceNodes" so that the filter does not get passed through a generated Project RelNode created for handling conversion of Calcite literal types to Impala literal types (see ImpalaProjectRel.isCoercedProjectForValues() method for details on that). Change-Id: Id075d8516f1fcff4e6402c2ab4b4992a174c8151 Reviewed-on: http://gerrit.cloudera.org:8080/22405 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Aman Sinha <[email protected]> --- java/calcite-planner/pom.xml | 2 +- java/calcite-planner/src/main/codegen/templates/Parser.jj | 4 +++- .../java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java | 7 ++++++- .../java/org/apache/impala/calcite/rel/node/ImpalaSortRel.java | 7 ++----- .../java/org/apache/impala/calcite/service/CalciteOptimizer.java | 8 ++++---- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/java/calcite-planner/pom.xml b/java/calcite-planner/pom.xml index e00925046..97d8ce64b 100644 --- a/java/calcite-planner/pom.xml +++ b/java/calcite-planner/pom.xml @@ -41,7 +41,7 @@ under the License. <dependency> <groupId>org.apache.calcite</groupId> <artifactId>calcite-core</artifactId> - <version>1.36.0</version> + <version>1.37.0</version> </dependency> <dependency> <groupId>org.apache.calcite.avatica</groupId> diff --git a/java/calcite-planner/src/main/codegen/templates/Parser.jj b/java/calcite-planner/src/main/codegen/templates/Parser.jj index e41d4eb2f..eb72f12f2 100644 --- a/java/calcite-planner/src/main/codegen/templates/Parser.jj +++ b/java/calcite-planner/src/main/codegen/templates/Parser.jj @@ -805,7 +805,9 @@ SqlNode LeafQuery(ExprContext exprContext) : // ensure a query is legal in this context checkQueryExpression(exprContext); } - e = SqlSelect() { return e; } + e = SqlSelect() + e = OrderByLimitOpt(e) + { return e; } | e = TableConstructor() { return e; } | diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java index 27f825dc0..8569bc633 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaProjectRel.java @@ -117,7 +117,12 @@ public class ImpalaProjectRel extends Project private NodeWithExprs getChildPlanNode(ParentPlanRelContext context, boolean isCoercedProjectForValues) throws ImpalaException { - Preconditions.checkState(context.filterCondition_ == null, + // The filter condition should have been pushed through the Project via a + // Calcite rule. The only exception to this is when a bogus "coerced project" + // (see comment in isCoercedProjectForValues method) is created after the + // rule has been applied. + Preconditions.checkState(context.filterCondition_ == null || + isCoercedProjectForValues, "Failure, Filter RelNode needs to be passed through the Project Rel Node."); ImpalaPlanRel relInput = (ImpalaPlanRel) getInput(0); ParentPlanRelContext.Builder builder = diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSortRel.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSortRel.java index 9808a521b..17eee414d 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSortRel.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/rel/node/ImpalaSortRel.java @@ -109,7 +109,8 @@ public class ImpalaSortRel extends Sort // all PlanNodes containing the limit so the PlanNode constructor can set the // limit, or leave the code here to mutate. inputNodeWithExprs.planNode_.setLimit(limit_); - return inputNodeWithExprs; + return NodeCreationUtils.wrapInSelectNodeIfNeeded(context, + inputNodeWithExprs, getCluster().getRexBuilder()); } List<Expr> inputExprs = inputNodeWithExprs.outputExprs_; @@ -164,10 +165,6 @@ public class ImpalaSortRel extends Sort } private void validateUnorderedLimit(RexNode filterCondition, long limit, long offset) { - // The filter should have been pushed through by the optimizer. This is verified - // here because there is no code to support pushing the filter condition if it - // hits this code. - Preconditions.checkArgument(filterCondition == null); Preconditions.checkArgument(limit_ > 0); Preconditions.checkArgument(offset_ == 0); } diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java index 62e96745a..e5ae54d98 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteOptimizer.java @@ -96,7 +96,8 @@ public class CalciteOptimizer implements CompilerStep { new ConvertToCNFRules.JoinConvertToCNFRule(), new ConvertToCNFRules.ProjectConvertToCNFRule(), ImpalaMinusToDistinctRule.Config.DEFAULT.toRule(), - new CombineValuesNodesRule() + new CombineValuesNodesRule(), + CoreRules.SORT_REMOVE_CONSTANT_KEYS )); builder.addMatchOrder(HepMatchOrder.BOTTOM_UP); @@ -145,13 +146,12 @@ public class CalciteOptimizer implements CompilerStep { HepProgramBuilder builder = new HepProgramBuilder(); - // Impala cannot handle the LITERAL_AGG method so we need to create - // an equivalent plan RelNode retRelNode = plan; builder.addMatchOrder(HepMatchOrder.BOTTOM_UP); builder.addRuleCollection(ImmutableList.of( RewriteRexOverRule.INSTANCE, - new ExtractLiteralAgg() + new ExtractLiteralAgg(), + CoreRules.FILTER_PROJECT_TRANSPOSE ));
