[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 17:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6587/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 14 Jul 2020 03:41:35 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 17: Verified+1 Code-Review+2

The next patch was verified, verifying this too since I"m merging together.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 14 Jul 2020 03:13:15 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Reviewed-on: http://gerrit.cloudera.org:8080/16128
Reviewed-by: Tim Armstrong 
Tested-by: Tim Armstrong 
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,287 insertions(+), 65 deletions(-)

Approvals:
  Tim Armstrong: Looks good to me, approved; Verified

--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 18
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, Thomas 
Tauber-Marshall, David Rorke, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#17).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,287 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/17
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 17
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 16:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6583/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 22:21:04 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 15:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6581/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 22:10:18 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 16: Code-Review+2

carry +2


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 21:53:54 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, Thomas 
Tauber-Marshall, David Rorke, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#16).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,288 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/16
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 16
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 15: Code-Review+2

(2 comments)

carry +2

http://gerrit.cloudera.org:8080/#/c/16128/14/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/16128/14/fe/src/main/java/org/apache/impala/analysis/Expr.java@1125
PS14, Line 1125:
> nit: formatting
Done


http://gerrit.cloudera.org:8080/#/c/16128/14/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16128/14/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@4235
PS14, Line 4235: # TODO: This plan could be improved by removing the ROLLUP in 
the subquery or by
> Maybe mark this as a TODO?
Done



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 21:53:05 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, Thomas 
Tauber-Marshall, David Rorke, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#15).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,287 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/15
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 15
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Thomas Tauber-Marshall (Code Review)
Thomas Tauber-Marshall has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 14: Code-Review+2

(3 comments)

http://gerrit.cloudera.org:8080/#/c/16128/14/fe/src/main/java/org/apache/impala/analysis/Expr.java
File fe/src/main/java/org/apache/impala/analysis/Expr.java:

http://gerrit.cloudera.org:8080/#/c/16128/14/fe/src/main/java/org/apache/impala/analysis/Expr.java@1125
PS14, Line 1125: :
nit: formatting


http://gerrit.cloudera.org:8080/#/c/16128/14/testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test:

http://gerrit.cloudera.org:8080/#/c/16128/14/testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test@9
PS14, Line 9: |  group by: CASE valid_tid(1,2,3,4) WHEN 1 THEN int_col WHEN 2 
THEN int_col WHEN 3 THEN int_col WHEN 4 THEN NULL END, CASE valid_tid(1,2,3,4) 
WHEN 1 THEN bool_col WHEN 2 THEN bool_col WHEN 3 THEN NULL WHEN 4 THEN NULL 
END, CASE valid_tid(1,2,3,4) WHEN 1 THEN string_col WHEN 2 THEN NULL WHEN 3 
THEN NULL WHEN 4 THEN NULL END, CASE valid_tid(1,2,3,4) WHEN 1 THEN 1 WHEN 2 
THEN 2 WHEN 3 THEN 3 WHEN 4 THEN 4 END
Obviously doesn't need to be fixed in this patch, but it would be nice if we 
had a way to make this output more decipherable


http://gerrit.cloudera.org:8080/#/c/16128/14/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16128/14/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@4235
PS14, Line 4235: # This plan could be improved by removing the ROLLUP in the 
subquery or by
Maybe mark this as a TODO?



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Thomas Tauber-Marshall 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 21:12:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 14:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6573/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 17:35:41 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, David Rorke, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#14).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,287 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/14
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-13 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 14: Code-Review+1

rebased, carry +1


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 14
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 13 Jul 2020 17:06:52 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-11 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 13: Code-Review+1


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sat, 11 Jul 2020 18:38:47 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 13:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6543/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 09 Jul 2020 23:05:21 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, David Rorke, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#13).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,288 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/13
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 13
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 12:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6542/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 09 Jul 2020 19:35:43 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 11:

(4 comments)

Addressed the comments. Some good points about missing optimisations.

http://gerrit.cloudera.org:8080/#/c/16128/11/fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
File fe/src/main/java/org/apache/impala/analysis/GroupByClause.java:

http://gerrit.cloudera.org:8080/#/c/16128/11/fe/src/main/java/org/apache/impala/analysis/GroupByClause.java@134
PS11, Line 134: addtlGroupingExprs additional grouping exprs that will be added 
to each
  :*grouping set
> May mention that the additional expressions can be the correlated columns w
I don't want to get into too much detail about explaining how other components 
work, but I think the context about correlated columns is useful to understand 
the function, so I added that.


http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@3567
PS11, Line 3567: group by: id, int_col, bool_col, b.tinyint_col, b.string_col
   : |  |  Class 1
   : |  |group by: id, int_col, NULL, b.tinyint_col, 
b.string_col
   : |  |  Class 2
   : |  |group by: id, NULL, NULL, b.tinyint_col, b.string_col
   : |  |  Class 3
   : |  |group by: NULL, NULL, NULL, b.tinyint_col, b.string_col
> The first GROUPBY probably is not necessary as it will hide the rows needed
Yeah the plan is missing some obvious optimisations. I think using the rollup 
in a subquery is a bit goofy so don't want to worry too much at this point 
about optimising it further. I guess it might arise accidentally with more 
complex queries, etc.

I left a comment pointing out that the plan is suboptimal.

I added another test in grouping-sets.test to illustrate a missing optimisation 
where HAVING predicates don't eliminate unnecessary agg classes.


http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@3601
PS11, Line 3601: RIGHT SEMI JOIN
> May mention due to the size of the two tables, the join order is flipped.
Done


http://gerrit.cloudera.org:8080/#/c/16128/11/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/16128/11/tests/util/parse_util.py@25
PS11, Line 25: 91
> Nice!
ack



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 09 Jul 2020 19:08:37 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, David Rorke, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#12).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,288 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/12
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 12
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Qifan Chen (Code Review)
Qifan Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 11:

(1 comment)

An update on the comment.

http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@3567
PS11, Line 3567: group by: id, int_col, bool_col, b.tinyint_col, b.string_col
   : |  |  Class 1
   : |  |group by: id, int_col, NULL, b.tinyint_col, 
b.string_col
   : |  |  Class 2
   : |  |group by: id, NULL, NULL, b.tinyint_col, b.string_col
   : |  |  Class 3
   : |  |group by: NULL, NULL, NULL, b.tinyint_col, b.string_col
> If b.tinyint_col and string_col are low in selectivity against a.tinyint_co
The first GROUPBY probably is not necessary as it will hide the rows needed to 
compute the Rollup group by. Here is the updated version.

1. hashjoin(a,b) on (tinyint_col, string_col). The output is b(id, int_col, 
bool_col).
2. Rollup groupby on (id, int_col, bool_col) from the join output.



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 09 Jul 2020 15:56:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-09 Thread Qifan Chen (Code Review)
Qifan Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 11:

(1 comment)

Just a thought.

http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@3567
PS11, Line 3567: group by: id, int_col, bool_col, b.tinyint_col, b.string_col
   : |  |  Class 1
   : |  |group by: id, int_col, NULL, b.tinyint_col, 
b.string_col
   : |  |  Class 2
   : |  |group by: id, NULL, NULL, b.tinyint_col, b.string_col
   : |  |  Class 3
   : |  |group by: NULL, NULL, NULL, b.tinyint_col, b.string_col
If b.tinyint_col and string_col are low in selectivity against a.tinyint_col 
and a.string_col, another efficient implementation (plan) would be to apply two 
GROUPBYs separately.

1. Group on b(tinyint_col, string_col) first to compute the input for the hash 
join;
2. Rollup group on (id, int_col, bool_col) against the surviving rows from the 
hash join for the final result.



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 09 Jul 2020 13:22:20 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-08 Thread Qifan Chen (Code Review)
Qifan Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 11: Code-Review+1

(3 comments)

Looks very good to me.

http://gerrit.cloudera.org:8080/#/c/16128/11/fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
File fe/src/main/java/org/apache/impala/analysis/GroupByClause.java:

http://gerrit.cloudera.org:8080/#/c/16128/11/fe/src/main/java/org/apache/impala/analysis/GroupByClause.java@134
PS11, Line 134: addtlGroupingExprs additional grouping exprs that will be added 
to each
  :*grouping set
May mention that the additional expressions can be the correlated columns when 
the GROUPBY appears in a correlated subquery, and the subquery is implemented 
with a HASH JOIN.


http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test:

http://gerrit.cloudera.org:8080/#/c/16128/11/testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test@3601
PS11, Line 3601: RIGHT SEMI JOIN
May mention due to the size of the two tables, the join order is flipped.

This is a nice test.


http://gerrit.cloudera.org:8080/#/c/16128/11/tests/util/parse_util.py
File tests/util/parse_util.py:

http://gerrit.cloudera.org:8080/#/c/16128/11/tests/util/parse_util.py@25
PS11, Line 25: 91
Nice!



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 08 Jul 2020 13:34:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-07 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 11:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6521/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 22:14:24 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 11: Code-Review+1

rebased onto latest master


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 21:49:44 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-07 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, David Rorke, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#11).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,208 insertions(+), 66 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/11
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 11
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-07 Thread Qifan Chen (Code Review)
Qifan Chen has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 10:

(1 comment)

> Uploaded patch set 10.

http://gerrit.cloudera.org:8080/#/c/16128/6/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
File testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test:

http://gerrit.cloudera.org:8080/#/c/16128/6/testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test@8835
PS6, Line 8835: 75.61K
> You're right about this - we apply CARDINALITY_FILTER in the planner tests
Good to know. Thanks!



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Tue, 07 Jul 2020 18:03:43 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-06 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 10: Code-Review+1

carry


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 06 Jul 2020 23:49:49 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-06 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 10:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6503/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Mon, 06 Jul 2020 17:48:16 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-06 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Quanlong Huang, Fang-Yu Rao, Qifan Chen, David Rorke, Impala 
Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#10).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,207 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/10
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 10
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Quanlong Huang 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-04 Thread Aman Sinha (Code Review)
Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 9: Code-Review+1

LGTM


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Sun, 05 Jul 2020 03:52:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-03 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 9:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6486/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 03 Jul 2020 19:23:37 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-03 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Fang-Yu Rao, Qifan Chen, David Rorke, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#9).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,207 insertions(+), 65 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/9
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 9
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-03 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16128/8/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java:

http://gerrit.cloudera.org:8080/#/c/16128/8/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@354
PS8, Line 354: info that may be created by an external planner
> I did not update this comment in my patch.  It would be good if you can.  Y
Done


http://gerrit.cloudera.org:8080/#/c/16128/8/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@479
PS8, Line 479:* Currently, Impala does not support ROLLUP directly but 
suppose an external planner
> Same here.  Remove this statement and re-word it to something like 'caller
Done



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 03 Jul 2020 18:57:13 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-02 Thread Aman Sinha (Code Review)
Aman Sinha has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16128/8/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
File fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java:

http://gerrit.cloudera.org:8080/#/c/16128/8/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@354
PS8, Line 354: info that may be created by an external planner
I did not update this comment in my patch.  It would be good if you can.  You 
could just remove the 'that may be created by an external planner' statement.


http://gerrit.cloudera.org:8080/#/c/16128/8/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@479
PS8, Line 479:* Currently, Impala does not support ROLLUP directly but 
suppose an external planner
Same here.  Remove this statement and re-word it to something like 'caller 
should have converted the above to the following ...'



--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 21:41:15 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-02 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 8:

Exhaustive tests passed.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 21:19:10 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-01 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 8:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6481/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:23:34 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-01 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 7:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/6480/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 7
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:13:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-01 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16128 )

Change subject: IMPALA-9898: generate grouping set plans
..


Patch Set 8:

I rebased this onto the latest version of Aman's patch.


--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:02:25 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-9898: generate grouping set plans

2020-07-01 Thread Tim Armstrong (Code Review)
Hello Aman Sinha, Fang-Yu Rao, Qifan Chen, David Rorke, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/16128

to look at the new patch set (#8).

Change subject: IMPALA-9898: generate grouping set plans
..

IMPALA-9898: generate grouping set plans

Integrates the parsing and analysis with plan generation.

Testing:
* Add analysis test to make sure we reject unsupported queries.
* Added targeted planner tests to ensure we generate the correct
  aggregation classes for a variety of cases.
* Add targeted end-to-end functional tests.

Added five TPC-DS queries that use ROLLUP, building on some work done
by Fang-Yu Rao. Some tweaks were required for these tests.
* Add an extra ORDER BY clause to q77 to make fully deterministic.
* Add backticks around `returns` to avoid reserved word.
* Add INTERVAL keyword to date/timestamp arithmetic.

We can run q80, too, but I haven't added or verified results yet -
that can be done in a follow-up.

Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
---
M fe/src/main/java/org/apache/impala/analysis/Expr.java
M fe/src/main/java/org/apache/impala/analysis/GroupByClause.java
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/common/RuntimeEnv.java
M fe/src/main/java/org/apache/impala/planner/SingleNodePlanner.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
A testdata/workloads/functional-planner/queries/PlannerTest/grouping-sets.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
A testdata/workloads/functional-query/queries/QueryTest/grouping-sets.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q18.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q22.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q5.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q67.test
A testdata/workloads/tpcds/queries/tpcds-decimal_v2-q77.test
M tests/query_test/test_aggregation.py
M tests/query_test/test_tpcds_queries.py
M tests/util/parse_util.py
22 files changed, 4,203 insertions(+), 60 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/16128/8
--
To view, visit http://gerrit.cloudera.org:8080/16128
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie454c5bf7aee266321dee615548d7f2b71380197
Gerrit-Change-Number: 16128
Gerrit-PatchSet: 8
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Aman Sinha 
Gerrit-Reviewer: David Rorke 
Gerrit-Reviewer: Fang-Yu Rao 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Qifan Chen 
Gerrit-Reviewer: Tim Armstrong