You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by wz...@apache.org on 2021/07/21 19:38:20 UTC
[impala] branch master updated: IMPALA-9338 Fix impala crashing in
impala::RowDescriptor::TupleIsNullable(int)
This is an automated email from the ASF dual-hosted git repository.
wzhou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 90906ec IMPALA-9338 Fix impala crashing in impala::RowDescriptor::TupleIsNullable(int)
90906ec is described below
commit 90906ec68c93795b39a82f339c299a6fb363808d
Author: Yida Wu <wy...@gmail.com>
AuthorDate: Wed Jun 16 18:39:04 2021 -0700
IMPALA-9338 Fix impala crashing in impala::RowDescriptor::TupleIsNullable(int)
The patch fixes a bug in the function of orderConjunctsByCost, which
could remove the wrong object in the list when the first conjunct is
not the best and there is a same conjunct with different letter cases.
It could end up to have duplicate objects after reordering the list
because the conjunct, which has been added to the return list, is
still in the remaining list, and lead to a wrong plan later where
each side of the JOIN references columns from the other side due to
a double flip on a same conjunct (There are two conjuncts in the list,
and they are flipped as required by the analyzer, but unfortunately,
the two conjuncts are the same object).
The root cause of the issue is that some parts of the analyzer are
case-sensitive, but some parts are not. For example, the remove() of
the List considers the conjuncts with different letter cases are the
same because they refer the same columns, while the compareTo()
of the String considers the letter cases. This discrepancy creates
some unexpected bugs.
The fix uses the index instead of the Object to remove in the
remaining list to solve the bug. However, there may still be
somewhere else in our code that has similar issues regarding to
different letter cases, it could be better that we have a consistent
policy in SQL analyzing to avoid such bugs.
Regression testcases has been added to queries/tpch-outer-joins and
PlannerTest/join-order.
Tests:
Ran the Core FE_TEST and EE_TEST.
Passed the regression test in tpch-outer-joins and
PlannerTest/join-order.
Change-Id: I2ba031d7a6eda21a77b0e53bc41772ee9e00a528
Reviewed-on: http://gerrit.cloudera.org:8080/17610
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../java/org/apache/impala/planner/PlanNode.java | 11 ++++--
.../queries/PlannerTest/join-order.test | 31 ++++++++++++++++
.../workloads/tpch/queries/tpch-outer-joins.test | 41 ++++++++++++++++++++++
3 files changed, 80 insertions(+), 3 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/planner/PlanNode.java b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
index 7744aa6..e75722e 100644
--- a/fe/src/main/java/org/apache/impala/planner/PlanNode.java
+++ b/fe/src/main/java/org/apache/impala/planner/PlanNode.java
@@ -1008,8 +1008,12 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
while (!remaining.isEmpty()) {
double smallestCost = Float.MAX_VALUE;
T bestConjunct = null;
+ // IMPALA-9338: The bestConjunctIdx indicates the index of the bestConjunct in
+ // the remaining list.
+ int bestConjunctIdx = 0;
double backoffExp = 1.0 / (double) (sortedConjuncts.size() + 1);
- for (T e : remaining) {
+ for (int i = 0; i < remaining.size(); i++) {
+ T e = remaining.get(i);
double sel = Math.pow(e.hasSelectivity() ? e.getSelectivity() : defaultSel,
backoffExp);
@@ -1021,17 +1025,18 @@ abstract public class PlanNode extends TreeNode<PlanNode> {
if (cost < smallestCost) {
smallestCost = cost;
bestConjunct = e;
+ bestConjunctIdx = i;
} else if (cost == smallestCost) {
// Break ties based on toSql() to get a consistent display in explain plans.
if (e.toSql().compareTo(bestConjunct.toSql()) < 0) {
- smallestCost = cost;
bestConjunct = e;
+ bestConjunctIdx = i;
}
}
}
sortedConjuncts.add(bestConjunct);
- remaining.remove(bestConjunct);
+ remaining.remove(bestConjunctIdx);
totalCost -= bestConjunct.getCost();
}
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test b/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
index 7532e24..30ca561 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
@@ -1858,3 +1858,34 @@ PLAN-ROOT SINK
runtime filters: RF000 -> functional.alltypes.id
row-size=24B cardinality=10
====
+# Regression test for IMPALA-9338. The following query duplicates the same condition
+# with different letter cases. With the fix, the plan should flip the JOIN order and
+# reference the proper columns in the predicate.
+select * from tpch.customer c
+left outer join
+tpch.lineitem l
+ON c.c_custkey = l.l_orderkey
+and c.C_CUSTKEY = l.L_ORDERKEY
+---- DISTRIBUTEDPLAN
+PLAN-ROOT SINK
+|
+05:EXCHANGE [UNPARTITIONED]
+|
+02:HASH JOIN [RIGHT OUTER JOIN, PARTITIONED]
+| hash predicates: l.L_ORDERKEY = c.C_CUSTKEY, l.l_orderkey = c.c_custkey
+| runtime filters: RF000 <- c.C_CUSTKEY, RF001 <- c.c_custkey
+| row-size=448B cardinality=150.00K
+|
+|--04:EXCHANGE [HASH(c.C_CUSTKEY,c.c_custkey)]
+| |
+| 00:SCAN HDFS [tpch.customer c]
+| HDFS partitions=1/1 files=1 size=23.08MB
+| row-size=218B cardinality=150.00K
+|
+03:EXCHANGE [HASH(l.L_ORDERKEY,l.l_orderkey)]
+|
+01:SCAN HDFS [tpch.lineitem l]
+ HDFS partitions=1/1 files=1 size=718.94MB
+ runtime filters: RF000 -> l.L_ORDERKEY, RF001 -> l.l_orderkey
+ row-size=231B cardinality=6.00M
+====
diff --git a/testdata/workloads/tpch/queries/tpch-outer-joins.test b/testdata/workloads/tpch/queries/tpch-outer-joins.test
index d0c8162..ea51d71 100644
--- a/testdata/workloads/tpch/queries/tpch-outer-joins.test
+++ b/testdata/workloads/tpch/queries/tpch-outer-joins.test
@@ -47,3 +47,44 @@ ON cast(t1.o_comment as char(120)) = cast(t2.o_comment as char(120))
---- TYPES
BIGINT
====
+---- QUERY
+# Regression test for IMPALA-9338. The following query duplicates the same condition
+# with different letter cases. Without the fix of IMPALA-9338, the query should crash.
+SET mem_limit = 300m;
+SELECT c_custkey from customer c left outer join lineitem l
+ON c.c_custkey = l.l_orderkey and c.C_CUSTKEY = l.L_ORDERKEY
+ORDER BY c_custkey limit 1
+---- RESULTS
+1
+---- TYPES
+BIGINT
+====
+---- QUERY
+# Regression test for IMPALA-9338. The following query uses reserved key as columns,
+# and duplicates the same condition with different letter cases, should provide a
+# correct plan after the fix.
+drop table if exists default.t1;
+drop table if exists default.t2;
+create table default.t1(`insert` bigint);
+create table default.t2(`select` bigint);
+insert into default.t1(`insert`)
+select c_custkey
+from tpch_parquet.customer
+limit 100;
+insert into default.t2(`select`)
+select o_custkey
+from tpch_parquet.orders
+limit 100000;
+explain select * from default.t1 b
+left join default.t2 a
+ON b.`insert` = a.`select`
+AND a.`SELECT` = b.`INSERT`;
+---- RESULTS: VERIFY_IS_SUBSET
+'PLAN-ROOT SINK'
+'05:EXCHANGE [UNPARTITIONED]'
+'02:HASH JOIN [RIGHT OUTER JOIN, PARTITIONED]'
+'|--04:EXCHANGE [HASH(b.`INSERT`,b.`insert`)]'
+'| 00:SCAN HDFS [default.t1 b]'
+'03:EXCHANGE [HASH(a.`SELECT`,a.`select`)]'
+'01:SCAN HDFS [default.t2 a]'
+====