You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/09/29 21:49:11 UTC
[GitHub] [hive] zabetak commented on a diff in pull request #3588: HIVE-26524: Use Calcite to remove sections of a query plan known never produces rows
zabetak commented on code in PR #3588:
URL: https://github.com/apache/hive/pull/3588#discussion_r983479717
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/Bug.java:
##########
@@ -74,4 +74,14 @@ public final class Bug {
* Whether <a href="https://issues.apache.org/jira/browse/CALCITE-4704">CALCITE-4704</a> is fixed.
*/
public static final boolean CALCITE_4704_FIXED = false;
+
+ /**
+ * Whether <a href="https://issues.apache.org/jira/browse/CALCITE-4704">CALCITE-5293</a> is fixed.
Review Comment:
nit: Link points to the wrong JIRA.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/Bug.java:
##########
@@ -74,4 +74,14 @@ public final class Bug {
* Whether <a href="https://issues.apache.org/jira/browse/CALCITE-4704">CALCITE-4704</a> is fixed.
*/
public static final boolean CALCITE_4704_FIXED = false;
+
+ /**
+ * Whether <a href="https://issues.apache.org/jira/browse/CALCITE-4704">CALCITE-5293</a> is fixed.
+ */
+ public static final boolean CALCITE_5293_FIXED = false;
+
+ /**
+ * Whether <a href="https://issues.apache.org/jira/browse/CALCITE-4704">CALCITE-5294</a> is fixed.
Review Comment:
nit: Link points to the wrong JIRA.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java:
##########
@@ -121,7 +127,87 @@ public static ASTNode convert(final RelNode relNode, List<FieldSchema> resultSch
return c.convert();
}
+ // TOK_QUERY
+ // TOK_INSERT
+ // TOK_DESTINATION
+ // TOK_DIR
+ // TOK_TMP_FILE
+ // TOK_SELECT
+ // TOK_SELEXPR
+ // TOK_FUNCTION
+ // TOK_<type>
+ // TOK_NULL
+ // alias0
+ // ...
+ // TOK_SELEXPR
+ // TOK_FUNCTION
+ // TOK_<type>
+ // TOK_NULL
+ // aliasn
+ // TOK_LIMIT
+ // 0
+ // 0
+ public static ASTNode emptyPlan(RelDataType dataType) {
+ if (dataType.getFieldCount() == 0) {
+ throw new IllegalArgumentException("Schema is empty.");
+ }
+
+ ASTBuilder select = ASTBuilder.construct(HiveParser.TOK_SELECT, "TOK_SELECT");
+ for (int i = 0; i < dataType.getFieldCount(); ++i) {
+ RelDataTypeField fieldType = dataType.getFieldList().get(i);
+ if (fieldType.getValue().getSqlTypeName() == SqlTypeName.NULL) {
+ select.add(ASTBuilder.selectExpr(
+ ASTBuilder.construct(HiveParser.TOK_NULL, "TOK_NULL").node(),
+ fieldType.getName()));
+ } else {
+ ASTNode typeNode = createCast(fieldType);
+ select.add(ASTBuilder.selectExpr(
+ ASTBuilder.construct(HiveParser.TOK_FUNCTION, "TOK_FUNCTION")
+ .add(typeNode)
+ .add(ASTBuilder.construct(HiveParser.TOK_NULL, "TOK_NULL").node()).node(),
+ fieldType.getName()));
+ }
+ }
+
+ ASTNode insert = ASTBuilder.
+ construct(HiveParser.TOK_INSERT, "TOK_INSERT").
+ add(ASTBuilder.destNode()).
+ add(select).
+ add(ASTBuilder.limit(0, 0)).
+ node();
+
+ return ASTBuilder.
+ construct(HiveParser.TOK_QUERY, "TOK_QUERY").
+ add(insert).
+ node();
+ }
+
+ private static ASTNode createCast(RelDataTypeField fieldType) {
+ HiveToken ht = TypeConverter.hiveToken(fieldType.getType());
+ ASTNode typeNode;
+ if (ht == null) {
+ typeNode = ASTBuilder.construct(
+ HiveParser.Identifier, fieldType.getType().getSqlTypeName().getName().toLowerCase()).node();
+ } else {
+ ASTBuilder typeNodeBuilder = ASTBuilder.construct(ht.type, ht.text);
+ if (ht.args != null) {
+ for (String castArg : ht.args) {
+ typeNodeBuilder.add(HiveParser.Identifier, castArg);
+ }
+ }
+ typeNode = typeNodeBuilder.node();
+ }
+ return typeNode;
+ }
+
private ASTNode convert() throws CalciteSemanticException {
+ if (root instanceof HiveValues) {
+ HiveValues values = (HiveValues) root;
+ if (isEmpty(values)) {
+ select = values;
+ return emptyPlan(values.getRowType());
+ }
Review Comment:
If we have a `HiveValues` that it is not empty can the code below handle it? If not, I would put an assertion with a proper comment.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveValues.java:
##########
@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.optimizer.calcite.reloperators;
+
+import com.google.common.collect.ImmutableList;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelTraitSet;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Values;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexLiteral;
+
+import java.util.List;
+
+/**
+ * Subclass of {@link org.apache.calcite.rel.core.Values}.
+ * Targeting Hive engine.
+ */
+public class HiveValues extends Values {
+
+ public HiveValues(
+ RelOptCluster cluster,
+ RelDataType rowType,
+ ImmutableList<ImmutableList<RexLiteral>> tuples,
+ RelTraitSet traits) {
+ super(cluster, rowType, tuples, traits);
+ }
+
+ @Override
+ public RelNode copy(RelTraitSet traitSet, List<RelNode> inputs) {
+ if (getInputs().equals(inputs) && traitSet.equals(getTraitSet())) {
+ return this;
+ }
+
Review Comment:
I think the Javadoc of `copy` implies that the method creates a new instance; better avoid returning `this`.
If we want to be extra cautious we can introduce assertions similar to those in `LogicalValues`.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java:
##########
@@ -121,7 +127,87 @@ public static ASTNode convert(final RelNode relNode, List<FieldSchema> resultSch
return c.convert();
}
+ // TOK_QUERY
+ // TOK_INSERT
+ // TOK_DESTINATION
+ // TOK_DIR
+ // TOK_TMP_FILE
+ // TOK_SELECT
+ // TOK_SELEXPR
+ // TOK_FUNCTION
+ // TOK_<type>
+ // TOK_NULL
+ // alias0
+ // ...
+ // TOK_SELEXPR
+ // TOK_FUNCTION
+ // TOK_<type>
+ // TOK_NULL
+ // aliasn
+ // TOK_LIMIT
+ // 0
+ // 0
+ public static ASTNode emptyPlan(RelDataType dataType) {
+ if (dataType.getFieldCount() == 0) {
+ throw new IllegalArgumentException("Schema is empty.");
+ }
+
+ ASTBuilder select = ASTBuilder.construct(HiveParser.TOK_SELECT, "TOK_SELECT");
+ for (int i = 0; i < dataType.getFieldCount(); ++i) {
+ RelDataTypeField fieldType = dataType.getFieldList().get(i);
+ if (fieldType.getValue().getSqlTypeName() == SqlTypeName.NULL) {
+ select.add(ASTBuilder.selectExpr(
+ ASTBuilder.construct(HiveParser.TOK_NULL, "TOK_NULL").node(),
+ fieldType.getName()));
+ } else {
+ ASTNode typeNode = createCast(fieldType);
+ select.add(ASTBuilder.selectExpr(
+ ASTBuilder.construct(HiveParser.TOK_FUNCTION, "TOK_FUNCTION")
+ .add(typeNode)
+ .add(ASTBuilder.construct(HiveParser.TOK_NULL, "TOK_NULL").node()).node(),
+ fieldType.getName()));
+ }
+ }
+
+ ASTNode insert = ASTBuilder.
+ construct(HiveParser.TOK_INSERT, "TOK_INSERT").
+ add(ASTBuilder.destNode()).
+ add(select).
+ add(ASTBuilder.limit(0, 0)).
+ node();
+
+ return ASTBuilder.
+ construct(HiveParser.TOK_QUERY, "TOK_QUERY").
+ add(insert).
+ node();
+ }
+
+ private static ASTNode createCast(RelDataTypeField fieldType) {
Review Comment:
Do we handle STRUCT types? Should we?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/opconventer/HiveValuesVisitor.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer;
+
+import org.apache.calcite.rel.core.Values;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.exec.Operator;
+import org.apache.hadoop.hive.ql.exec.OperatorFactory;
+import org.apache.hadoop.hive.ql.exec.RowSchema;
+import org.apache.hadoop.hive.ql.exec.SelectOperator;
+import org.apache.hadoop.hive.ql.exec.TableScanOperator;
+import org.apache.hadoop.hive.ql.metadata.Table;
+import org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveValues;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.TypeConverter;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer.HiveOpConverter.OpAttr;
+import org.apache.hadoop.hive.ql.parse.SemanticAnalyzer;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.ql.plan.ExprNodeConstantDesc;
+import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
+import org.apache.hadoop.hive.ql.plan.LimitDesc;
+import org.apache.hadoop.hive.ql.plan.SelectDesc;
+import org.apache.hadoop.hive.ql.plan.TableScanDesc;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+class HiveValuesVisitor extends HiveRelNodeVisitor<HiveValues> {
+ HiveValuesVisitor(HiveOpConverter hiveOpConverter) {
+ super(hiveOpConverter);
+ }
+
+ @Override
+ OpAttr visit(HiveValues valuesRel) throws SemanticException {
+
+ LOG.debug("Translating operator rel#{}:{} with row type: [{}]",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.getRowType());
+ LOG.debug("Operator rel#{}:{} has {} tuples.",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.tuples.size());
+
+ if (!Values.isEmpty(valuesRel)) {
+ LOG.error("Empty {} operator translation not supported yet in return path.",
+ valuesRel.getClass().getCanonicalName());
+ return null;
+ }
+
+ // 1. collect columns for project row schema
Review Comment:
nit: I tend to avoid enumerations in comments cause as code evolves and steps are added/removed people will probably update the numbers leading to more changes than necessary.
Moreover the comment would be completely redundant (and possibly the code more readable) if you create a function such as `List<ExprNodeDesc> createColumnsForProject`.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/ASTConverter.java:
##########
@@ -121,7 +127,87 @@ public static ASTNode convert(final RelNode relNode, List<FieldSchema> resultSch
return c.convert();
}
+ // TOK_QUERY
+ // TOK_INSERT
+ // TOK_DESTINATION
+ // TOK_DIR
+ // TOK_TMP_FILE
+ // TOK_SELECT
+ // TOK_SELEXPR
+ // TOK_FUNCTION
+ // TOK_<type>
+ // TOK_NULL
+ // alias0
+ // ...
+ // TOK_SELEXPR
+ // TOK_FUNCTION
+ // TOK_<type>
+ // TOK_NULL
+ // aliasn
+ // TOK_LIMIT
+ // 0
+ // 0
+ public static ASTNode emptyPlan(RelDataType dataType) {
Review Comment:
Since this is public I would put the plan in javadoc and also add an explanation about why this particular AST representation was chosen. Not everybody knows that this is not gonna be executed by Hive due to subsequent optimizations.
##########
ql/src/test/queries/clientpositive/antijoin.q:
##########
@@ -44,7 +44,7 @@ explain cbo select a.key, a.value from t1_n55 a left join t2_n33 b on a.key=b.ke
select a.key, a.value from t1_n55 a left join t2_n33 b on a.key=b.key join t3_n12 c on a.key=c.key where b.key is null sort by a.key, a.value;
-- single extra simple filter on right side.
-explain select a.key from t1_n55 a left join t2_n33 b on a.key = b.key where b.key is null and b.value > 'val_1';
+explain select a.key from t1_n55 a left join t2_n33 b on a.key = b.key where b.key is null and b.value is null;
Review Comment:
Regarding changes in query (.q) files such as this one it would be helpful to document the reasoning somewhere (either in the final commit message or under the JIRA). Why do we change the initial query?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/opconventer/HiveValuesVisitor.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer;
+
+import org.apache.calcite.rel.core.Values;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.exec.Operator;
+import org.apache.hadoop.hive.ql.exec.OperatorFactory;
+import org.apache.hadoop.hive.ql.exec.RowSchema;
+import org.apache.hadoop.hive.ql.exec.SelectOperator;
+import org.apache.hadoop.hive.ql.exec.TableScanOperator;
+import org.apache.hadoop.hive.ql.metadata.Table;
+import org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveValues;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.TypeConverter;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer.HiveOpConverter.OpAttr;
+import org.apache.hadoop.hive.ql.parse.SemanticAnalyzer;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.ql.plan.ExprNodeConstantDesc;
+import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
+import org.apache.hadoop.hive.ql.plan.LimitDesc;
+import org.apache.hadoop.hive.ql.plan.SelectDesc;
+import org.apache.hadoop.hive.ql.plan.TableScanDesc;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+class HiveValuesVisitor extends HiveRelNodeVisitor<HiveValues> {
+ HiveValuesVisitor(HiveOpConverter hiveOpConverter) {
+ super(hiveOpConverter);
+ }
+
+ @Override
+ OpAttr visit(HiveValues valuesRel) throws SemanticException {
+
+ LOG.debug("Translating operator rel#{}:{} with row type: [{}]",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.getRowType());
+ LOG.debug("Operator rel#{}:{} has {} tuples.",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.tuples.size());
+
+ if (!Values.isEmpty(valuesRel)) {
+ LOG.error("Empty {} operator translation not supported yet in return path.",
+ valuesRel.getClass().getCanonicalName());
+ return null;
+ }
+
+ // 1. collect columns for project row schema
Review Comment:
It could be helpful to have a comment with the overall shape of the plan; similar to what you did for AST. Something like:
```
// TS [DUMMY] - SEL [null, ..., null] - LIM [0]
```
##########
ql/src/test/results/clientpositive/llap/subquery_ALL.q.out:
##########
@@ -413,8 +413,7 @@ POSTHOOK: Input: default@part
POSTHOOK: Input: default@part_null_n0
#### A masked pattern was here ####
26
-Warning: Shuffle Join MERGEJOIN[37][tables = [$hdt$_1, $hdt$_2]] in Stage 'Reducer 3' is a cross product
-Warning: Shuffle Join MERGEJOIN[38][tables = [$hdt$_1, $hdt$_2, $hdt$_0]] in Stage 'Reducer 4' is a cross product
+Warning: Shuffle Join MERGEJOIN[22][tables = [$hdt$_0, $hdt$_1]] in Stage 'Reducer 2' is a cross product
Review Comment:
Is there a branch here that is simplified to empty? Why?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveRemoveEmptySingleRules.java:
##########
@@ -0,0 +1,280 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hive.ql.optimizer.calcite.rules;
+
+import org.apache.calcite.plan.RelOptRule;
+import org.apache.calcite.plan.RelOptRuleCall;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.RelRule;
+import org.apache.calcite.plan.hep.HepRelVertex;
+import org.apache.calcite.plan.volcano.RelSubset;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.core.Aggregate;
+import org.apache.calcite.rel.core.Filter;
+import org.apache.calcite.rel.core.Join;
+import org.apache.calcite.rel.core.JoinRelType;
+import org.apache.calcite.rel.core.Project;
+import org.apache.calcite.rel.core.Sort;
+import org.apache.calcite.rel.core.Union;
+import org.apache.calcite.rel.core.Values;
+import org.apache.calcite.rel.rules.PruneEmptyRules;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.hadoop.hive.ql.optimizer.calcite.Bug;
+import org.apache.hadoop.hive.ql.optimizer.calcite.HiveRelFactories;
+
+import java.util.Collections;
+import java.util.List;
+
+import static com.google.common.collect.Iterables.concat;
+
+/**
+ * This class provides access to Calcite's {@link PruneEmptyRules}.
+ * The instances of the rules use {@link org.apache.hadoop.hive.ql.optimizer.calcite.HiveRelBuilder}.
+ */
+public class HiveRemoveEmptySingleRules extends PruneEmptyRules {
+
+ public static final RelOptRule PROJECT_INSTANCE =
+ RelRule.Config.EMPTY
+ .withDescription("HivePruneEmptyProject")
+ .as(PruneEmptyRules.RemoveEmptySingleRule.Config.class)
+ .withOperandFor(Project.class, project -> true)
Review Comment:
Do we want the rules to match JdbcProject and other non-Hive operators?
##########
ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/translator/TestASTConverter.java:
##########
@@ -0,0 +1,104 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.optimizer.calcite.translator;
+
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.calcite.rel.type.RelDataTypeFieldImpl;
+import org.apache.calcite.rel.type.RelRecordType;
+import org.apache.calcite.sql.type.ArraySqlType;
+import org.apache.calcite.sql.type.BasicSqlType;
+import org.apache.calcite.sql.type.SqlTypeName;
+import org.apache.hadoop.hive.ql.optimizer.calcite.HiveTypeSystemImpl;
+import org.apache.hadoop.hive.ql.parse.ASTNode;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.util.Collections;
+import java.util.List;
+
+import static java.util.Arrays.asList;
+import static org.apache.hadoop.hive.ql.optimizer.calcite.translator.ASTConverter.emptyPlan;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+
+class TestASTConverter {
+ @Test
+ void testEmptyPlanWhenInputSchemaIsEmpty() {
+ RelRecordType dataType = new RelRecordType(Collections.emptyList());
+ IllegalArgumentException thrown = Assertions.assertThrows(IllegalArgumentException.class, () -> {
+ emptyPlan(dataType);
+ });
+
+ Assertions.assertTrue(thrown.getMessage().contains("Schema is empty"));
+ }
+
+ @Test
+ void testEmptyPlan() {
+ List<RelDataTypeField> fields = asList(
+ new RelDataTypeFieldImpl("a", 0, new BasicSqlType(new HiveTypeSystemImpl(), SqlTypeName.INTEGER)),
+ new RelDataTypeFieldImpl("b", 1, new BasicSqlType(new HiveTypeSystemImpl(), SqlTypeName.CHAR, 30)),
+ new RelDataTypeFieldImpl("c", 2, new BasicSqlType(new HiveTypeSystemImpl(), SqlTypeName.NULL)),
+ new RelDataTypeFieldImpl("d", 3, new ArraySqlType(new BasicSqlType(new HiveTypeSystemImpl(), SqlTypeName.INTEGER), false)),
+ new RelDataTypeFieldImpl("e", 4, new ArraySqlType(new BasicSqlType(new HiveTypeSystemImpl(), SqlTypeName.INTEGER), true)));
+ RelDataType dataType = new RelRecordType(fields);
+
+ ASTNode tree = emptyPlan(dataType);
+
+ // TOK_QUERY -> TOK_INSERT -> TOK_SELECT
+ assertThat(tree.getChild(0).getChild(1).getChildCount(), is(fields.size()));
Review Comment:
nit: The assertion below covers also this one; no need to have both.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/opconventer/HiveValuesVisitor.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer;
+
+import org.apache.calcite.rel.core.Values;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.exec.Operator;
+import org.apache.hadoop.hive.ql.exec.OperatorFactory;
+import org.apache.hadoop.hive.ql.exec.RowSchema;
+import org.apache.hadoop.hive.ql.exec.SelectOperator;
+import org.apache.hadoop.hive.ql.exec.TableScanOperator;
+import org.apache.hadoop.hive.ql.metadata.Table;
+import org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveValues;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.TypeConverter;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer.HiveOpConverter.OpAttr;
+import org.apache.hadoop.hive.ql.parse.SemanticAnalyzer;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.ql.plan.ExprNodeConstantDesc;
+import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
+import org.apache.hadoop.hive.ql.plan.LimitDesc;
+import org.apache.hadoop.hive.ql.plan.SelectDesc;
+import org.apache.hadoop.hive.ql.plan.TableScanDesc;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+class HiveValuesVisitor extends HiveRelNodeVisitor<HiveValues> {
+ HiveValuesVisitor(HiveOpConverter hiveOpConverter) {
+ super(hiveOpConverter);
+ }
+
+ @Override
+ OpAttr visit(HiveValues valuesRel) throws SemanticException {
+
+ LOG.debug("Translating operator rel#{}:{} with row type: [{}]",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.getRowType());
+ LOG.debug("Operator rel#{}:{} has {} tuples.",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.tuples.size());
Review Comment:
This debug message is a bit redundant given that there is the error message below.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/opconventer/HiveValuesVisitor.java:
##########
@@ -0,0 +1,109 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer;
+
+import org.apache.calcite.rel.core.Values;
+import org.apache.calcite.rel.type.RelDataTypeField;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.exec.Operator;
+import org.apache.hadoop.hive.ql.exec.OperatorFactory;
+import org.apache.hadoop.hive.ql.exec.RowSchema;
+import org.apache.hadoop.hive.ql.exec.SelectOperator;
+import org.apache.hadoop.hive.ql.exec.TableScanOperator;
+import org.apache.hadoop.hive.ql.metadata.Table;
+import org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveValues;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.TypeConverter;
+import org.apache.hadoop.hive.ql.optimizer.calcite.translator.opconventer.HiveOpConverter.OpAttr;
+import org.apache.hadoop.hive.ql.parse.SemanticAnalyzer;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.ql.plan.ExprNodeConstantDesc;
+import org.apache.hadoop.hive.ql.plan.ExprNodeDesc;
+import org.apache.hadoop.hive.ql.plan.LimitDesc;
+import org.apache.hadoop.hive.ql.plan.SelectDesc;
+import org.apache.hadoop.hive.ql.plan.TableScanDesc;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+class HiveValuesVisitor extends HiveRelNodeVisitor<HiveValues> {
+ HiveValuesVisitor(HiveOpConverter hiveOpConverter) {
+ super(hiveOpConverter);
+ }
+
+ @Override
+ OpAttr visit(HiveValues valuesRel) throws SemanticException {
+
+ LOG.debug("Translating operator rel#{}:{} with row type: [{}]",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.getRowType());
+ LOG.debug("Operator rel#{}:{} has {} tuples.",
+ valuesRel.getId(), valuesRel.getRelTypeName(), valuesRel.tuples.size());
+
+ if (!Values.isEmpty(valuesRel)) {
+ LOG.error("Empty {} operator translation not supported yet in return path.",
+ valuesRel.getClass().getCanonicalName());
+ return null;
+ }
+
+ // 1. collect columns for project row schema
+ List<String> columnNames = new ArrayList<>();
+ List<ExprNodeDesc> exprNodeDescList = new ArrayList<>();
+ Map<String, ExprNodeDesc> colExprMap = new HashMap<>();
+
+ List<ColumnInfo> colInfoList = new ArrayList<>();
+ for (int i = 0; i < valuesRel.getRowType().getFieldList().size(); i++) {
+ RelDataTypeField typeField = valuesRel.getRowType().getFieldList().get(i);
+
+ ColumnInfo ci = new ColumnInfo(
+ typeField.getName(), TypeConverter.convert(typeField.getType()), SemanticAnalyzer.DUMMY_TABLE, false);
+ colInfoList.add(ci);
+ columnNames.add(typeField.getName());
+
+ ExprNodeDesc exprNodeDesc = new ExprNodeConstantDesc(TypeConverter.convert(typeField.getType()), null);
+ colExprMap.put(typeField.getName(), exprNodeDesc);
+ exprNodeDescList.add(exprNodeDesc);
+ }
+
+ // 2. Create TS on dummy table
+ Table metadata = hiveOpConverter.getSemanticAnalyzer().getDummyTable();
+ TableScanDesc tsd = new TableScanDesc(SemanticAnalyzer.DUMMY_TABLE, Collections.emptyList(), metadata);
+
+ TableScanOperator ts = (TableScanOperator) OperatorFactory.get(
+ hiveOpConverter.getSemanticAnalyzer().getOpContext(), tsd, new RowSchema(Collections.emptyList()));
+
+ hiveOpConverter.getTopOps().put(SemanticAnalyzer.DUMMY_TABLE, ts);
+
+ // 3. Create Select operator
+ SelectDesc sd = new SelectDesc(exprNodeDescList, columnNames);
+ SelectOperator selOp = (SelectOperator) OperatorFactory.getAndMakeChild(sd, new RowSchema(colInfoList), ts);
+ selOp.setColumnExprMap(colExprMap);
+
+ // 4. Create Limit 0 operator
Review Comment:
nit: The comments are somewhat redundant; here for instance it is straightforward what we create a Limit 0 operator.
##########
ql/src/test/results/clientpositive/llap/ppd_udf_col.q.out:
##########
@@ -80,22 +80,9 @@ STAGE DEPENDENCIES:
STAGE PLANS:
Stage: Stage-0
Fetch Operator
- limit: -1
+ limit: 0
Processor Tree:
- TableScan
- alias: src
- filterExpr: (UDFToDouble(key) = 100.0D) (type: boolean)
- Filter Operator
- predicate: (UDFToDouble(key) = 100.0D) (type: boolean)
- Limit
- Number of rows: 0
- Select Operator
- expressions: key (type: string)
- outputColumnNames: _col0
- Select Operator
- expressions: _col0 (type: string), rand() (type: double), '4' (type: string)
- outputColumnNames: _col0, _col1, _col2
- ListSink
+ ListSink
Review Comment:
Is this the empty plan? I though it was a bit different including a dummy table; what am I missing?
##########
ql/src/test/results/clientpositive/llap/masking_10.q.out:
##########
@@ -137,9 +136,7 @@ STAGE PLANS:
Tez
#### A masked pattern was here ####
Edges:
- Reducer 2 <- Map 1 (CUSTOM_SIMPLE_EDGE), Reducer 4 (CUSTOM_SIMPLE_EDGE)
- Reducer 3 <- Map 1 (SIMPLE_EDGE), Reducer 2 (SIMPLE_EDGE)
- Reducer 4 <- Map 1 (SIMPLE_EDGE)
+ Reducer 2 <- Map 1 (SIMPLE_EDGE), Map 3 (SIMPLE_EDGE)
Review Comment:
Why it was simplified?
##########
ql/src/test/results/clientpositive/llap/fold_case.q.out:
##########
@@ -177,7 +180,7 @@ STAGE PLANS:
sort order:
Statistics: Num rows: 1 Data size: 8 Basic stats: COMPLETE Column stats: COMPLETE
value expressions: _col0 (type: bigint)
- Execution mode: vectorized, llap
+ Execution mode: llap
Review Comment:
Does it have any effect in the rest of the plan the fact that vectorization was lost? I guess not but just to be sure.
##########
ql/src/test/results/clientpositive/llap/float_equality.q.out:
##########
@@ -9,9 +9,7 @@ POSTHOOK: Input: _dummy_database@_dummy_table
1
PREHOOK: query: select 1 where -0.0<0.0
PREHOOK: type: QUERY
-PREHOOK: Input: _dummy_database@_dummy_table
Review Comment:
Why it was removed?
##########
ql/src/test/results/clientpositive/llap/auto_join21.q.out:
##########
@@ -43,37 +45,38 @@ STAGE PLANS:
outputColumnNames: _col0, _col1, _col2, _col3
input vertices:
1 Map 2
- Statistics: Num rows: 1 Data size: 356 Basic stats: COMPLETE Column stats: COMPLETE
+ Statistics: Num rows: 55 Data size: 19580 Basic stats: COMPLETE Column stats: COMPLETE
Reduce Output Operator
key expressions: _col2 (type: string)
null sort order: z
sort order: +
Map-reduce partition columns: _col2 (type: string)
- Statistics: Num rows: 1 Data size: 356 Basic stats: COMPLETE Column stats: COMPLETE
+ Statistics: Num rows: 55 Data size: 19580 Basic stats: COMPLETE Column stats: COMPLETE
value expressions: _col0 (type: string), _col1 (type: string), _col3 (type: string)
Execution mode: vectorized, llap
LLAP IO: all inputs
Map 2
Map Operator Tree:
TableScan
alias: src2
+ filterExpr: ((UDFToDouble(key) > 9.0D) and (UDFToDouble(key) < 11.0D)) (type: boolean)
Statistics: Num rows: 500 Data size: 89000 Basic stats: COMPLETE Column stats: COMPLETE
- Limit
- Number of rows: 0
- Statistics: Num rows: 1 Data size: 178 Basic stats: COMPLETE Column stats: COMPLETE
+ Filter Operator
+ predicate: ((UDFToDouble(key) > 9.0D) and (UDFToDouble(key) < 11.0D)) (type: boolean)
+ Statistics: Num rows: 55 Data size: 9790 Basic stats: COMPLETE Column stats: COMPLETE
Review Comment:
I suppose you changed the constants in the query to prevent the new optimization to kick in but why?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org