From b413ea737e27bb6bde72690c6bd9268bedd8640c Mon Sep 17 00:00:00 2001 From: Jinfeng Ni Date: Thu, 2 Apr 2015 13:09:57 -0700 Subject: [PATCH] DRILL-1384: Part 5 - Make sure ProjectRemove will honor the output fieldName and use validated rowtype from SqlValidator to honor the final output field. ProjectRemove should honor parent's output field name. Fix Parser, allow * in compound identifier in DrillParserImpl. Make sure ProjectRemove will honor the output fieldName and use validated rowtype from SqlValidator to honor the final output field. This is required, since Drill's execution framework is name-based, different from Calcite's ordinal-based execution engine. --- .../codegen/includes/compoundIdentifier.ftl | 14 +++-- .../exec/planner/common/DrillRelOptUtil.java | 16 ++--- .../exec/planner/logical/DrillProjectRel.java | 13 ++++ .../logical/DrillPushProjIntoScan.java | 2 +- .../logical/DrillReduceExpressionsRule.java | 3 +- .../exec/planner/logical/DrillRuleSets.java | 2 +- .../exec/planner/sql/DrillSqlOperator.java | 6 +- .../sql/handlers/CreateTableHandler.java | 1 + .../sql/handlers/DefaultSqlHandler.java | 61 ++++++++++++++++--- .../planner/sql/handlers/ExplainHandler.java | 9 ++- .../planner/sql/handlers/SqlHandlerUtil.java | 3 +- 11 files changed, 97 insertions(+), 33 deletions(-) diff --git a/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl b/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl index 50d8c2038ef..f613770ec37 100644 --- a/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl +++ b/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl @@ -24,10 +24,16 @@ SqlIdentifier CompoundIdentifier() : } ( ( - p = Identifier() - { - builder.addString(p, getPos()); - } + + ( + p = Identifier() { + builder.addString(p, getPos()); + } + | + { + builder.addString("*", getPos()); + } + ) ) | ( diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java index cea8fec2255..1dc93498a06 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java @@ -92,8 +92,6 @@ public static boolean areRowTypesCompatible( * Returns a relational expression which has the same fields as the * underlying expression, but the fields have different names. * - * Note: This method is copied from {@link org.eigenbase.rel.CalcRel#createRename(RelNode, List)} because it has a bug - * which doesn't rename the exprs. This bug is fixed in latest version of Apache Calcite (1.2). * * @param rel Relational expression * @param fieldNames Field names @@ -104,19 +102,17 @@ public static RelNode createRename( final List fieldNames) { final List fields = rel.getRowType().getFieldList(); assert fieldNames.size() == fields.size(); - final List> refs = - new AbstractList>() { + final List refs = + new AbstractList() { public int size() { return fields.size(); } - public Pair get(int index) { - return Pair.of( - (RexNode) new RexInputRef(index, fields.get(index).getType()), - fieldNames.get(index)); + public RexNode get(int index) { + return RexInputRef.of(index, fields); } }; - return RelOptUtil.createRename(rel, fieldNames); - // return Calc.createProject(rel, refs, true); + + return RelOptUtil.createProject(rel, refs, fieldNames, false); } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectRel.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectRel.java index 26f366e92eb..6e132aa9316 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectRel.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectRel.java @@ -75,4 +75,17 @@ public static DrillProjectRel convert(Project project, ConversionContext context return new DrillProjectRel(context.getCluster(), context.getLogicalTraits(), input, exps, new RelRecordType(fields)); } + /** provide a public method to create an instance of DrillProjectRel. + * + * @param cluster + * @param traits + * @param child + * @param exps + * @param rowType + * @return new instance of DrillProjectRel + */ + public static DrillProjectRel create(RelOptCluster cluster, RelTraitSet traits, RelNode child, List exps, + RelDataType rowType) { + return new DrillProjectRel(cluster, traits, child, exps, rowType); + } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java index 379f6e97be1..2981de88eca 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java @@ -76,7 +76,7 @@ public void onMatch(RelOptRuleCall call) { newProjects, proj.getRowType()); - if (ProjectRemoveRule.isTrivial(newProj)) { + if (ProjectRemoveRule.isTrivial(newProj, true)) { call.transformTo(newScan); } else { call.transformTo(newProj); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java index 3527601165a..2b658316cb1 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java @@ -17,6 +17,7 @@ ******************************************************************************/ package org.apache.drill.exec.planner.logical; +import org.apache.calcite.rel.RelCollations; import org.apache.calcite.rel.core.Calc; import org.apache.calcite.rel.logical.LogicalCalc; import org.apache.calcite.rel.core.Filter; @@ -76,7 +77,7 @@ protected RelNode createEmptyRelOrEquivalent(Calc calc) { } private static RelNode createEmptyEmptyRelHelper(SingleRel input) { - return LogicalSort.create(input.getInput(), RelCollationImpl.EMPTY, + return LogicalSort.create(input.getInput(), RelCollations.EMPTY, input.getCluster().getRexBuilder().makeExactLiteral(BigDecimal.valueOf(0)), input.getCluster().getRexBuilder().makeExactLiteral(BigDecimal.valueOf(0))); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java index d035defc1f2..532fd43877c 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java @@ -131,7 +131,7 @@ public static RuleSet getDrillBasicRules(QueryContext context) { // SwapJoinRule.INSTANCE, AggregateRemoveRule.INSTANCE, // RemoveDistinctRule // UnionToDistinctRule.INSTANCE, - ProjectRemoveRule.INSTANCE, // RemoveTrivialProjectRule + ProjectRemoveRule.NAME_CALC_INSTANCE, // RemoveTrivialProjectRule // RemoveTrivialCalcRule.INSTANCE, SortRemoveRule.INSTANCE, //RemoveSortRule.INSTANCE, diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java index 776da3fc45f..7b5a99dbc2e 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java @@ -58,7 +58,7 @@ protected RelDataType getReturnDataType(final RelDataTypeFactory factory) { if (MinorType.BIT.equals(returnType.getMinorType())) { return factory.createSqlType(SqlTypeName.BOOLEAN); } - return factory.createSqlType(SqlTypeName.ANY); + return factory.createTypeWithNullability(factory.createSqlType(SqlTypeName.ANY), true); } private RelDataType getNullableReturnDataType(final RelDataTypeFactory factory) { @@ -81,10 +81,6 @@ public RelDataType deriveType(SqlValidator validator, SqlValidatorScope scope, S @Override public RelDataType inferReturnType(SqlOperatorBinding opBinding) { - if (NONE.equals(returnType)) { - return super.inferReturnType(opBinding); - } - return getNullableReturnDataType(opBinding.getTypeFactory()); } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java index f2097bef15d..e9ac1e14905 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java @@ -20,6 +20,7 @@ import java.io.IOException; import org.apache.calcite.schema.SchemaPlus; +import org.apache.calcite.sql.TypedSqlNode; import org.apache.calcite.tools.Planner; import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java index 22f9803bbd4..eda1b5fd6c1 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java @@ -22,6 +22,15 @@ import java.util.Collection; import java.util.List; +import org.apache.calcite.rel.core.Project; +import org.apache.calcite.rel.logical.LogicalProject; +import org.apache.calcite.rel.rules.ProjectRemoveRule; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.rex.RexBuilder; +import org.apache.calcite.rex.RexNode; +import org.apache.calcite.rex.RexUtil; +import org.apache.calcite.sql.TypedSqlNode; +import org.apache.calcite.sql.validate.SqlValidatorUtil; import org.apache.calcite.tools.Planner; import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; @@ -37,6 +46,7 @@ import org.apache.drill.exec.physical.base.AbstractPhysicalVisitor; import org.apache.drill.exec.physical.base.PhysicalOperator; import org.apache.drill.exec.physical.impl.join.JoinUtils; +import org.apache.drill.exec.planner.logical.DrillProjectRel; import org.apache.drill.exec.planner.logical.DrillRel; import org.apache.drill.exec.planner.logical.DrillScreenRel; import org.apache.drill.exec.planner.logical.DrillStoreRel; @@ -45,6 +55,7 @@ import org.apache.drill.exec.planner.physical.PhysicalPlanCreator; import org.apache.drill.exec.planner.physical.PlannerSettings; import org.apache.drill.exec.planner.physical.Prel; +import org.apache.drill.exec.planner.physical.ProjectPrel; import org.apache.drill.exec.planner.physical.explain.PrelSequencer; import org.apache.drill.exec.planner.physical.visitor.ComplexToJsonPrelVisitor; import org.apache.drill.exec.planner.physical.visitor.ExcessiveExchangeIdentifier; @@ -129,12 +140,17 @@ protected void log(String name, PhysicalPlan plan) throws JsonProcessingExceptio @Override public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConversionException, IOException, ForemanSetupException { SqlNode rewrittenSqlNode = rewrite(sqlNode); - SqlNode validated = validateNode(rewrittenSqlNode); + TypedSqlNode validatedTypedSqlNode = validateNode(rewrittenSqlNode); + SqlNode validated = validatedTypedSqlNode.getSqlNode(); + RelDataType validatedRowType = validatedTypedSqlNode.getType(); + RelNode rel = convertToRel(validated); rel = preprocessNode(rel); + log("Optiq Logical", rel); - DrillRel drel = convertToDrel(rel); + DrillRel drel = convertToDrel(rel, validatedRowType); + log("Drill Logical", drel); Prel prel = convertToPrel(drel); log("Drill Physical", prel); @@ -144,8 +160,37 @@ public PhysicalPlan getPlan(SqlNode sqlNode) throws ValidationException, RelConv return plan; } - protected SqlNode validateNode(SqlNode sqlNode) throws ValidationException, RelConversionException, ForemanSetupException { - SqlNode sqlNodeValidated = planner.validate(sqlNode); + protected DrillRel addRenamedProject(DrillRel rel, RelDataType validatedRowType) { + RelDataType t = rel.getRowType(); + + RexBuilder b = rel.getCluster().getRexBuilder(); + List projections = Lists.newArrayList(); + int projectCount = t.getFieldList().size(); + + for (int i =0; i < projectCount; i++) { + projections.add(b.makeInputRef(rel, i)); + } + + final List fieldNames2 = SqlValidatorUtil.uniquify(validatedRowType.getFieldNames(), SqlValidatorUtil.F_SUGGESTER2); + + RelDataType newRowType = RexUtil.createStructType(rel.getCluster().getTypeFactory(), projections, fieldNames2); + + DrillProjectRel topProj = DrillProjectRel.create(rel.getCluster(), rel.getTraitSet(), rel, projections, newRowType); + + if (ProjectRemoveRule.isTrivial(topProj, true)) { + return rel; + } else{ + return topProj; + } + //return RelOptUtil.createProject(rel, projections, fieldNames2); + + } + + + protected TypedSqlNode validateNode(SqlNode sqlNode) throws ValidationException, RelConversionException, ForemanSetupException { + TypedSqlNode typedSqlNode = planner.validateAndGetType(sqlNode); + + SqlNode sqlNodeValidated = typedSqlNode.getSqlNode(); // Check if the unsupported functionality is used UnsupportedOperatorsVisitor visitor = UnsupportedOperatorsVisitor.createVisitor(context); @@ -159,7 +204,7 @@ protected SqlNode validateNode(SqlNode sqlNode) throws ValidationException, RelC throw ex; } - return sqlNodeValidated; + return typedSqlNode; } protected RelNode convertToRel(SqlNode node) throws RelConversionException { @@ -191,14 +236,16 @@ protected RelNode preprocessNode(RelNode rel) throws SqlUnsupportedException{ return rel; } - protected DrillRel convertToDrel(RelNode relNode) throws RelConversionException, SqlUnsupportedException { + protected DrillRel convertToDrel(RelNode relNode, RelDataType validatedRowType) throws RelConversionException, SqlUnsupportedException { try { RelNode convertedRelNode = planner.transform(DrillSqlWorker.LOGICAL_RULES, relNode.getTraitSet().plus(DrillRel.DRILL_LOGICAL), relNode); if (convertedRelNode instanceof DrillStoreRel) { throw new UnsupportedOperationException(); } else { - return new DrillScreenRel(convertedRelNode.getCluster(), convertedRelNode.getTraitSet(), convertedRelNode); + // Put a non-trivial topProject to ensure the final output field name is preserved, when necessary. + DrillRel topPreservedNameProj = addRenamedProject((DrillRel)convertedRelNode, validatedRowType); + return new DrillScreenRel(topPreservedNameProj.getCluster(), topPreservedNameProj.getTraitSet(), topPreservedNameProj); } } catch (RelOptPlanner.CannotPlanException ex) { logger.error(ex.getMessage()); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java index d1420dff9b7..1636a2528c2 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java @@ -19,6 +19,8 @@ import java.io.IOException; +import org.apache.calcite.rel.type.RelDataType; +import org.apache.calcite.sql.TypedSqlNode; import org.apache.calcite.tools.RelConversionException; import org.apache.calcite.tools.ValidationException; @@ -53,12 +55,15 @@ public ExplainHandler(SqlHandlerConfig config) { @Override public PhysicalPlan getPlan(SqlNode node) throws ValidationException, RelConversionException, IOException, ForemanSetupException { SqlNode sqlNode = rewrite(node); - SqlNode validated = validateNode(sqlNode); + TypedSqlNode validatedTypedSqlNode = validateNode(sqlNode); + SqlNode validated = validatedTypedSqlNode.getSqlNode(); + RelDataType validatedRowType = validatedTypedSqlNode.getType(); + RelNode rel = convertToRel(validated); rel = preprocessNode(rel); log("Optiq Logical", rel); - DrillRel drel = convertToDrel(rel); + DrillRel drel = convertToDrel(rel, validatedRowType); log("Drill Logical", drel); if (mode == ResultMode.LOGICAL) { diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java index 2572ace2a95..50af9721e77 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java @@ -83,8 +83,7 @@ public static RelNode resolveNewTableRel(boolean isNewTableView, Planner planner // SELECT col1, median(col2), avg(col3) FROM sourcetbl GROUP BY col1 ; // Similary for CREATE VIEW. - return RelOptUtil.createRename(validatedQueryRelNode, tableFieldNames); - // return DrillRelOptUtil.createRename(validatedQueryRelNode, tableFieldNames); + return DrillRelOptUtil.createRename(validatedQueryRelNode, tableFieldNames); } return validatedQueryRelNode;