You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2019/02/26 23:54:24 UTC
[impala] 02/04: IMPALA-8069: Crash in impala::Sorter::Run::Run
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit e4f46dad92f92c9a0e7730d7b393d42cf7ee8c7f
Author: Paul Rogers <pr...@cloudera.com>
AuthorDate: Fri Feb 22 19:05:29 2019 -0800
IMPALA-8069: Crash in impala::Sorter::Run::Run
Modifies the planner to omit a sorter node in the obscure case we have
an analytic query with a constant sort. The analytic node requires a
buffered tuple. When the sort is omitted, creates a buffered tuple on
top of the node that would have been input to the sort.
Testing:
* Added a PlannerTest case.
* Added an end-to-end test.
* Reran the query that previously crashed: it now produces proper results.
Change-Id: I19a57f3791ce9e41fe0ba79dc5834e762341c74e
Reviewed-on: http://gerrit.cloudera.org:8080/12562
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../org/apache/impala/planner/AnalyticPlanner.java | 13 +++++--
.../queries/PlannerTest/analytic-fns.test | 42 ++++++++++++++--------
.../queries/PlannerTest/order.test | 17 +++++++++
.../queries/QueryTest/analytic-fns.test | 16 +++++++++
4 files changed, 71 insertions(+), 17 deletions(-)
diff --git a/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java b/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
index 92c44bb..3e58eae 100644
--- a/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
+++ b/fe/src/main/java/org/apache/impala/planner/AnalyticPlanner.java
@@ -324,8 +324,13 @@ public class AnalyticPlanner {
ExprSubstitutionMap bufferedSmap = new ExprSubstitutionMap();
boolean activePartition = activeExprs(partitionByExprs);
+ // IMPALA-8069: Ignore something like ORDER BY 0
+ boolean isConstSort = true;
+ for (OrderByElement elmt : orderByElements) {
+ isConstSort = isConstSort && elmt.getExpr().isConstant();
+ }
// sort on partition by (pb) + order by (ob) exprs and create pb/ob predicates
- if (activePartition || !orderByElements.isEmpty()) {
+ if (activePartition || !isConstSort) {
// first sort on partitionExprs (direction doesn't matter)
List<Expr> sortExprs = Lists.newArrayList(partitionByExprs);
List<Boolean> isAsc =
@@ -365,10 +370,12 @@ public class AnalyticPlanner {
root = sortNode;
root.init(analyzer_);
- sortSmap = sortNode.getOutputSmap();
+ }
+ if (activePartition || !orderByElements.isEmpty()) {
+ sortSmap = root.getOutputSmap();
// create bufferedTupleDesc and bufferedSmap
- sortTupleId = sortNode.tupleIds_.get(0);
+ sortTupleId = root.tupleIds_.get(0);
bufferedTupleDesc =
analyzer_.getDescTbl().copyTupleDescriptor(sortTupleId, "buffered-tuple");
if (LOG.isTraceEnabled()) {
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test b/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
index fef2844..a2038e4 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/analytic-fns.test
@@ -2888,22 +2888,36 @@ PLAN-ROOT SINK
constant-operands=3
row-size=1B cardinality=3
====
-# IMPALA-6323 Order by a constant is equivalent to no ordering.
-select x, count() over(order by 1) from (VALUES((1 x), (2), (3))) T;
----- DISTRIBUTEDPLAN
+# Regression test for IMPALA-8069
+SELECT FIRST_VALUE(0) OVER (ORDER BY 0 ASC)
+FROM functional.alltypestiny
+---- PLAN
PLAN-ROOT SINK
|
-02:ANALYTIC
-| functions: count()
-| order by: 1 ASC
-| window: RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
-| row-size=9B cardinality=3
+01:ANALYTIC
+| functions: first_value(0)
+| order by: 0 ASC
+| window: ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
+| row-size=1B cardinality=8
|
-01:SORT
-| order by: 1 ASC
-| row-size=1B cardinality=3
+00:SCAN HDFS [functional.alltypestiny]
+ partitions=4/4 files=4 size=460B
+ row-size=0B cardinality=8
+====
+# Regression test for IMPALA-8069
+# Same case as in end-to-end test analytic-fns.test
+SELECT FIRST_VALUE(0) OVER (ORDER BY 0 ASC), row_number() over (order by 0 ASC)
+FROM functional.alltypestiny
+---- PLAN
+PLAN-ROOT SINK
|
-00:UNION
- constant-operands=3
- row-size=1B cardinality=3
+01:ANALYTIC
+| functions: first_value(0), row_number()
+| order by: 0 ASC
+| window: ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
+| row-size=9B cardinality=8
+|
+00:SCAN HDFS [functional.alltypestiny]
+ partitions=4/4 files=4 size=460B
+ row-size=0B cardinality=8
====
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/order.test b/testdata/workloads/functional-planner/queries/PlannerTest/order.test
index fe02596..113b28c 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/order.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/order.test
@@ -1598,3 +1598,20 @@ PLAN-ROOT SINK
limit: 1
row-size=0B cardinality=1
====
+# Regression test for IMPALA-8069
+SELECT
+FIRST_VALUE(0) OVER (ORDER BY 0 ASC)
+FROM functional.alltypestiny
+---- PLAN
+PLAN-ROOT SINK
+|
+01:ANALYTIC
+| functions: first_value(0)
+| order by: 0 ASC
+| window: ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW
+| row-size=1B cardinality=8
+|
+00:SCAN HDFS [functional.alltypestiny]
+ partitions=4/4 files=4 size=460B
+ row-size=0B cardinality=8
+====
diff --git a/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test b/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
index fbbd939..4188205 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/analytic-fns.test
@@ -2140,3 +2140,19 @@ TINYINT, BIGINT
2,3
3,3
====
+---- QUERY
+# IMPALA-6323 Order by a constant is equivalent to no ordering.
+SELECT FIRST_VALUE(0) OVER (ORDER BY 0 ASC)
+FROM functional.alltypestiny;
+---- TYPES
+TINYINT
+---- RESULTS
+0
+0
+0
+0
+0
+0
+0
+0
+====