IMPALA-10096: Use the original ordinal if the group by ordinal reference is a constant int
The SelectStmt's groupingExprs_ uses the analyzed version and the
ordinal reference will be substituted.
It will throw an exception if the ordinal reference is still a numeric
literal, because we will re-analyze after the expression has been
rewritten. For example, here "count(1)" is rewritten to "count(*)" so
we need to re-analyze the new query.
select 13, id, count(1) from dimtbl group by 1, 2;
The rewritten sql should be
select 13, id, count(*) from dimtbl group by 1, id;
If the original query uses "count(*)", it won't hit the bug since
no rewriting happens.
Testing:
- Added new unit tests with ordinal in GROUP BY whereas SELECT has
the INT literal.
- Ran 'mvn test' for the FE
Change-Id: I34f659d15073d69aa0a4685f56ad94557df86560
Reviewed-on: http://gerrit.cloudera.org:8080/16353
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
diff --git a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
index 3d520f3..4ad4652 100644
--- a/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
+++ b/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
@@ -756,7 +756,17 @@
}
}
// initialize groupingExprs_ with the analyzed version
- groupingExprs_ = groupingExprsCopy_;
+ // use the original ordinal if the analyzed expr is a INT literal
+ List<Expr> groupingExprs = new ArrayList<>();
+ for (int i = 0; i < groupingExprsCopy_.size(); ++i) {
+ Expr expr = groupingExprsCopy_.get(i);
+ if (expr instanceof NumericLiteral && Expr.IS_INT_LITERAL.apply(expr)) {
+ groupingExprs.add(groupingExprs_.get(i).clone());
+ } else {
+ groupingExprs.add(expr);
+ }
+ }
+ groupingExprs_ = groupingExprs;
if (groupByClause_ != null && groupByClause_.hasGroupingSets()) {
groupByClause_.analyzeGroupingSets(groupingExprsCopy_);
diff --git a/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test b/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
index d36a2af..2ac3f53 100644
--- a/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
+++ b/testdata/workloads/functional-planner/queries/PlannerTest/aggregation.test
@@ -1612,3 +1612,63 @@
partitions=24/24 files=24 size=478.45KB
row-size=4B cardinality=7.30K
====
+# IMPALA-10096: use the original ordinal if the group by ordinal reference is a
+# constant int
+select 13, id, count(1) from functional.dimtbl group by 1, 2
+---- PLAN
+PLAN-ROOT SINK
+|
+01:AGGREGATE [FINALIZE]
+| output: count(*)
+| group by: 13, id
+| row-size=17B cardinality=10
+|
+00:SCAN HDFS [functional.dimtbl]
+ HDFS partitions=1/1 files=1 size=171B
+ row-size=8B cardinality=10
+====
+# IMPALA-10096: use the original ordinal if the group by ordinal reference is a
+# constant int
+select -1, id, count(1) from functional.dimtbl group by 1, 2
+---- PLAN
+PLAN-ROOT SINK
+|
+01:AGGREGATE [FINALIZE]
+| output: count(*)
+| group by: -1, id
+| row-size=17B cardinality=10
+|
+00:SCAN HDFS [functional.dimtbl]
+ HDFS partitions=1/1 files=1 size=171B
+ row-size=8B cardinality=10
+====
+# IMPALA-10096: use the original ordinal if the group by ordinal reference is a
+# constant int
+select 2, id, count(1) from functional.dimtbl group by 1, 2
+---- PLAN
+PLAN-ROOT SINK
+|
+01:AGGREGATE [FINALIZE]
+| output: count(*)
+| group by: 2, id
+| row-size=17B cardinality=10
+|
+00:SCAN HDFS [functional.dimtbl]
+ HDFS partitions=1/1 files=1 size=171B
+ row-size=8B cardinality=10
+====
+# IMPALA-10096: use the original ordinal if the group by ordinal reference is a
+# constant int
+select 2 + 1, id, count(1) from functional.dimtbl group by 1, 2
+---- PLAN
+PLAN-ROOT SINK
+|
+01:AGGREGATE [FINALIZE]
+| output: count(*)
+| group by: 2 + 1, id
+| row-size=18B cardinality=10
+|
+00:SCAN HDFS [functional.dimtbl]
+ HDFS partitions=1/1 files=1 size=171B
+ row-size=8B cardinality=10
+====
