You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by za...@apache.org on 2022/10/19 11:53:07 UTC
[hive] 03/03: HIVE-26643: HiveUnionPullUpConstantsRule produces an invalid plan when pulling up constants for nullable fields (Alessandro Solimando reviewed by Stamatis Zampetakis)
This is an automated email from the ASF dual-hosted git repository.
zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hive.git
commit 6b05d64ce8c7161415d97a7896ea50025322e30a
Author: Alessandro Solimando <al...@gmail.com>
AuthorDate: Mon Oct 17 17:12:18 2022 +0200
HIVE-26643: HiveUnionPullUpConstantsRule produces an invalid plan when pulling up constants for nullable fields (Alessandro Solimando reviewed by Stamatis Zampetakis)
Closes #3680
---
.../hadoop/hive/ql/optimizer/calcite/Bug.java | 5 +
.../rules/HiveUnionPullUpConstantsRule.java | 27 ++--
.../rules/TestHiveUnionPullUpConstantsRule.java | 178 +++++++++++++++++++++
3 files changed, 199 insertions(+), 11 deletions(-)
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/Bug.java b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/Bug.java
index 32f8cff74b9..91877060ad0 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/Bug.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/Bug.java
@@ -78,4 +78,9 @@ public final class Bug {
* Whether <a href="https://issues.apache.org/jira/browse/CALCITE-5294">CALCITE-5294</a> is fixed.
*/
public static final boolean CALCITE_5294_FIXED = false;
+
+ /**
+ * Whether <a href="https://issues.apache.org/jira/browse/CALCITE-5337">CALCITE-5337</a> is fixed.
+ */
+ public static final boolean CALCITE_5337_FIXED = false;
}
diff --git a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveUnionPullUpConstantsRule.java b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveUnionPullUpConstantsRule.java
index 10d8c1362e7..dcb18072d06 100644
--- a/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveUnionPullUpConstantsRule.java
+++ b/ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveUnionPullUpConstantsRule.java
@@ -38,6 +38,7 @@ import org.apache.calcite.tools.RelBuilderFactory;
import org.apache.calcite.util.ImmutableBitSet;
import org.apache.calcite.util.Pair;
import org.apache.calcite.util.mapping.Mappings;
+import org.apache.hadoop.hive.ql.optimizer.calcite.Bug;
import org.apache.hadoop.hive.ql.optimizer.calcite.HiveRelFactories;
import org.apache.hadoop.hive.ql.optimizer.calcite.reloperators.HiveUnion;
import org.slf4j.Logger;
@@ -66,6 +67,10 @@ public class HiveUnionPullUpConstantsRule extends RelOptRule {
@Override
public void onMatch(RelOptRuleCall call) {
+ if (Bug.CALCITE_5337_FIXED) {
+ throw new IllegalStateException("Class redundant when the fix for CALCITE-5337 is merged into Calcite");
+ }
+
final Union union = call.rel(0);
final int count = union.getRowType().getFieldCount();
@@ -82,13 +87,11 @@ public class HiveUnionPullUpConstantsRule extends RelOptRule {
return;
}
- Map<RexNode, RexNode> conditionsExtracted =
- RexUtil.predicateConstants(RexNode.class, rexBuilder, predicates.pulledUpPredicates);
Map<RexNode, RexNode> constants = new HashMap<>();
for (int i = 0; i < count ; i++) {
RexNode expr = rexBuilder.makeInputRef(union, i);
- if (conditionsExtracted.containsKey(expr)) {
- constants.put(expr, conditionsExtracted.get(expr));
+ if (predicates.constantMap.containsKey(expr)) {
+ constants.put(expr, predicates.constantMap.get(expr));
}
}
@@ -107,7 +110,11 @@ public class HiveUnionPullUpConstantsRule extends RelOptRule {
RexNode expr = rexBuilder.makeInputRef(union, i);
RelDataTypeField field = fields.get(i);
if (constants.containsKey(expr)) {
- topChildExprs.add(constants.get(expr));
+ if (constants.get(expr).getType().equals(field.getType())) {
+ topChildExprs.add(constants.get(expr));
+ } else {
+ topChildExprs.add(rexBuilder.makeCast(field.getType(), constants.get(expr), true));
+ }
topChildExprsFields.add(field.getName());
} else {
topChildExprs.add(expr);
@@ -128,16 +135,14 @@ public class HiveUnionPullUpConstantsRule extends RelOptRule {
for (int i = 0; i < union.getInputs().size() ; i++) {
RelNode input = union.getInput(i);
List<Pair<RexNode, String>> newChildExprs = new ArrayList<>();
- for (int j = 0; j < refsIndex.cardinality(); j++ ) {
+ for (int j = 0; j < refsIndex.cardinality(); j++) {
int pos = refsIndex.nth(j);
- newChildExprs.add(
- Pair.<RexNode, String>of(rexBuilder.makeInputRef(input, pos),
- input.getRowType().getFieldList().get(pos).getName()));
+ newChildExprs.add(Pair.of(rexBuilder.makeInputRef(input, pos),
+ input.getRowType().getFieldList().get(pos).getName()));
}
if (newChildExprs.isEmpty()) {
// At least a single item in project is required.
- newChildExprs.add(Pair.<RexNode,String>of(
- topChildExprs.get(0), topChildExprsFields.get(0)));
+ newChildExprs.add(Pair.of(topChildExprs.get(0), topChildExprsFields.get(0)));
}
// Add the input with project on top
relBuilder.push(input);
diff --git a/ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/rules/TestHiveUnionPullUpConstantsRule.java b/ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/rules/TestHiveUnionPullUpConstantsRule.java
new file mode 100644
index 00000000000..6e455aa732d
--- /dev/null
+++ b/ql/src/test/org/apache/hadoop/hive/ql/optimizer/calcite/rules/TestHiveUnionPullUpConstantsRule.java
@@ -0,0 +1,178 @@
+/*
+ * 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.jdbc.JavaTypeFactoryImpl;
+import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptSchema;
+import org.apache.calcite.plan.RelOptUtil;
+import org.apache.calcite.plan.hep.HepPlanner;
+import org.apache.calcite.plan.hep.HepProgramBuilder;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.logical.LogicalTableScan;
+import org.apache.calcite.rel.type.RelDataType;
+import org.apache.calcite.rex.RexBuilder;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.tools.RelBuilder;
+import org.apache.hadoop.hive.ql.metadata.Table;
+import org.apache.hadoop.hive.ql.optimizer.calcite.HiveRelFactories;
+import org.apache.hadoop.hive.ql.optimizer.calcite.RelOptHiveTable;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Collections;
+
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.lenient;
+
+@RunWith(MockitoJUnitRunner.class)
+public class TestHiveUnionPullUpConstantsRule {
+
+ private final static JavaTypeFactoryImpl JAVA_TYPE_FACTORY = new JavaTypeFactoryImpl();
+
+ @Mock
+ private RelOptSchema schemaMock;
+ @Mock
+ RelOptHiveTable tableMock;
+ @Mock
+ Table hiveTableMDMock;
+
+ private HepPlanner planner;
+ private RelBuilder rexBuilder;
+
+ private static class MyRecordWithNullableField {
+ public Integer f1;
+ public int f2;
+ public double f3;
+ }
+
+ private static class MyRecord {
+ public int f1;
+ public int f2;
+ public double f3;
+ }
+
+ public void before(Class<?> clazz) {
+ HepProgramBuilder programBuilder = new HepProgramBuilder();
+ programBuilder.addRuleInstance(HiveUnionPullUpConstantsRule.INSTANCE);
+
+ planner = new HepPlanner(programBuilder.build());
+
+ RexBuilder rexBuilder = new RexBuilder(JAVA_TYPE_FACTORY);
+ final RelOptCluster optCluster = RelOptCluster.create(planner, rexBuilder);
+ RelDataType rowTypeMock = JAVA_TYPE_FACTORY.createStructType(clazz);
+ doReturn(rowTypeMock).when(tableMock).getRowType();
+ LogicalTableScan tableScan = LogicalTableScan.create(optCluster, tableMock, Collections.emptyList());
+ doReturn(tableScan).when(tableMock).toRel(ArgumentMatchers.any());
+ doReturn(tableMock).when(schemaMock).getTableForMember(any());
+ lenient().doReturn(hiveTableMDMock).when(tableMock).getHiveTableMD();
+
+ this.rexBuilder = HiveRelFactories.HIVE_BUILDER.create(optCluster, schemaMock);
+ }
+
+ public RexNode eq(String field, Number value) {
+ return rexBuilder.call(SqlStdOperatorTable.EQUALS,
+ rexBuilder.field(field), rexBuilder.literal(value));
+ }
+
+ private void test(RelNode plan, String expectedPrePlan, String expectedPostPlan) {
+ planner.setRoot(plan);
+ RelNode optimizedRelNode = planner.findBestExp();
+ assertEquals("Original plans do not match", expectedPrePlan, RelOptUtil.toString(plan));
+ assertEquals("Optimized plans do not match", expectedPostPlan, RelOptUtil.toString(optimizedRelNode));
+ }
+
+ @Test
+ public void testNonNullableFields() {
+ before(MyRecord.class);
+
+ final RelNode plan = rexBuilder
+ .scan("t")
+ .filter(eq("f1",1))
+ .project(rexBuilder.field("f1"), rexBuilder.field("f2"))
+ .scan("t")
+ .filter(eq("f1",1))
+ .project(rexBuilder.field("f1"), rexBuilder.field("f2"))
+ .union(true)
+ .build();
+
+ String prePlan = "HiveUnion(all=[true])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n";
+
+ String postPlan = "HiveProject(f1=[1], f2=[$0])\n"
+ + " HiveUnion(all=[true])\n"
+ + " HiveProject(f2=[$1])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n"
+ + " HiveProject(f2=[$1])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n";
+
+ test(plan, prePlan, postPlan);
+ }
+
+ @Test
+ public void testNullableFields() {
+ before(MyRecordWithNullableField.class);
+
+ final RelNode plan = rexBuilder
+ .scan("t")
+ .filter(eq("f1",1))
+ .project(rexBuilder.field("f1"), rexBuilder.field("f2"))
+ .scan("t")
+ .filter(eq("f1",1))
+ .project(rexBuilder.field("f1"), rexBuilder.field("f2"))
+ .union(false)
+ .build();
+
+ String prePlan = "HiveUnion(all=[true])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n";
+
+ String postPlan = "HiveProject(f1=[CAST(1):JavaType(class java.lang.Integer)], f2=[$0])\n"
+ + " HiveUnion(all=[true])\n"
+ + " HiveProject(f2=[$1])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n"
+ + " HiveProject(f2=[$1])\n"
+ + " HiveProject(f1=[$0], f2=[$1])\n"
+ + " HiveFilter(condition=[=($0, 1)])\n"
+ + " LogicalTableScan(table=[[]])\n";
+
+ test(plan, prePlan, postPlan);
+ }
+}