Return-Path: X-Original-To: apmail-drill-commits-archive@www.apache.org Delivered-To: apmail-drill-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 4BCDC17ECE for ; Wed, 22 Apr 2015 06:10:05 +0000 (UTC) Received: (qmail 89980 invoked by uid 500); 22 Apr 2015 06:10:05 -0000 Delivered-To: apmail-drill-commits-archive@drill.apache.org Received: (qmail 89935 invoked by uid 500); 22 Apr 2015 06:10:05 -0000 Mailing-List: contact commits-help@drill.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: commits@drill.apache.org Delivered-To: mailing list commits@drill.apache.org Received: (qmail 89910 invoked by uid 99); 22 Apr 2015 06:10:05 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 22 Apr 2015 06:10:05 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 0BE53E0280; Wed, 22 Apr 2015 06:10:05 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: jni@apache.org To: commits@drill.apache.org Date: Wed, 22 Apr 2015 06:10:04 -0000 Message-Id: <42d6d9e5e83c46c29880e372b8f31e19@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [01/13] drill git commit: 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 * Repository: drill Updated Branches: refs/heads/master 2a484251b -> e99f27032 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. Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/b413ea73 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/b413ea73 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/b413ea73 Branch: refs/heads/master Commit: b413ea737e27bb6bde72690c6bd9268bedd8640c Parents: 4a3ad5f Author: Jinfeng Ni Authored: Thu Apr 2 13:09:57 2015 -0700 Committer: Jinfeng Ni Committed: Tue Apr 21 16:29:37 2015 -0700 ---------------------------------------------------------------------- .../codegen/includes/compoundIdentifier.ftl | 14 +++-- .../exec/planner/common/DrillRelOptUtil.java | 16 ++--- .../exec/planner/logical/DrillProjectRel.java | 13 +++++ .../planner/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 + .../planner/sql/handlers/DefaultSqlHandler.java | 61 +++++++++++++++++--- .../planner/sql/handlers/ExplainHandler.java | 9 ++- .../planner/sql/handlers/SqlHandlerUtil.java | 3 +- 11 files changed, 97 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl b/exec/java-exec/src/main/codegen/includes/compoundIdentifier.ftl index 50d8c20..f613770 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()); + } + ) ) | ( http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillRelOptUtil.java ---------------------------------------------------------------------- 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 cea8fec..1dc9349 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 abstract class DrillRelOptUtil { * 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 abstract class DrillRelOptUtil { 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); } } http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillProjectRel.java ---------------------------------------------------------------------- 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 26f366e..6e132aa 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 class DrillProjectRel extends DrillProjectRelBase implements DrillRel { 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); + } } http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillPushProjIntoScan.java ---------------------------------------------------------------------- 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 379f6e9..2981de8 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 class DrillPushProjIntoScan extends RelOptRule { newProjects, proj.getRowType()); - if (ProjectRemoveRule.isTrivial(newProj)) { + if (ProjectRemoveRule.isTrivial(newProj, true)) { call.transformTo(newScan); } else { call.transformTo(newProj); http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillReduceExpressionsRule.java ---------------------------------------------------------------------- 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 3527601..2b65831 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 @@ public class DrillReduceExpressionsRule { } 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))); } http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/logical/DrillRuleSets.java ---------------------------------------------------------------------- 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 d035def..532fd43 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 class DrillRuleSets { // SwapJoinRule.INSTANCE, AggregateRemoveRule.INSTANCE, // RemoveDistinctRule // UnionToDistinctRule.INSTANCE, - ProjectRemoveRule.INSTANCE, // RemoveTrivialProjectRule + ProjectRemoveRule.NAME_CALC_INSTANCE, // RemoveTrivialProjectRule // RemoveTrivialCalcRule.INSTANCE, SortRemoveRule.INSTANCE, //RemoveSortRule.INSTANCE, http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/DrillSqlOperator.java ---------------------------------------------------------------------- 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 776da3f..7b5a99d 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 @@ public class DrillSqlOperator extends SqlFunction { 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 class DrillSqlOperator extends SqlFunction { @Override public RelDataType inferReturnType(SqlOperatorBinding opBinding) { - if (NONE.equals(returnType)) { - return super.inferReturnType(opBinding); - } - return getNullableReturnDataType(opBinding.getTypeFactory()); } } http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/CreateTableHandler.java ---------------------------------------------------------------------- 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 f2097be..e9ac1e1 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 @@ package org.apache.drill.exec.planner.sql.handlers; 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; http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/DefaultSqlHandler.java ---------------------------------------------------------------------- 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 22f9803..eda1b5f 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.ArrayList; 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.PhysicalPlan; 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.DrillDistributionTrait; 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 @@ public class DefaultSqlHandler extends AbstractSqlHandler { @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 class DefaultSqlHandler extends AbstractSqlHandler { 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 @@ public class DefaultSqlHandler extends AbstractSqlHandler { throw ex; } - return sqlNodeValidated; + return typedSqlNode; } protected RelNode convertToRel(SqlNode node) throws RelConversionException { @@ -191,14 +236,16 @@ public class DefaultSqlHandler extends AbstractSqlHandler { 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()); http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/ExplainHandler.java ---------------------------------------------------------------------- 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 d1420df..1636a25 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 @@ package org.apache.drill.exec.planner.sql.handlers; 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 class ExplainHandler extends DefaultSqlHandler { @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) { http://git-wip-us.apache.org/repos/asf/drill/blob/b413ea73/exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/handlers/SqlHandlerUtil.java ---------------------------------------------------------------------- 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 2572ace..50af972 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 class SqlHandlerUtil { // 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;