You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Aman Sinha (Code Review)" <ge...@cloudera.org> on 2020/06/26 18:54:01 UTC
[Impala-ASF-CR] [WIP] IMPALA-9898: Plan generation and execution for grouping sets
Aman Sinha has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16115
Change subject: [WIP] IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
[WIP] IMPALA-9898: Plan generation and execution for grouping sets
This patch enhances the MultiAggregateInfo to handle grouping sets
and rollup (which is converted to grouping sets). This patch does
not itself do parsing/validation of grouping sets syntax but rather
provides the following supporting functionality:
- A separate analyze method that accepts aggregation classes and
aggregation info that have been created separately (TODO: this
part will be added in another patch).
- A modified Transpose phase that uses combination of aggif(),
valid_tid() functions and CASE exprs to choose exactly which
slots from the underlying aggregate classes need to be output
based on the tuple id.
- Modified materialization step where all aggregate slots and
grouping slots are materialized in case of grouping sets.
- Creates grouping_id value for grouping sets. The grouping_id
function in SQL describes which expression is grouped-by in a
particular row of a query with grouping sets.
Testing:
This patch is not individually testable but will be tested
as part of the overall grouping set support.
Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
---
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
1 file changed, 260 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/16115/1
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: [WIP] IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 1:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16115/1/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/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@355
PS1, Line 355: int outputSlotIdx = groupingExprs_.size() + 1; // add 1 to account for the tuple id column
line too long (94 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@357
PS1, Line 357: outputSmap_.put(aggExprs_.get(i).clone(), new SlotRef(outputSlots.get(outputSlotIdx)));
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@452
PS1, Line 452: * Note that all classes have the same aggregate output exprs but different grouping exprs.
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@523
PS1, Line 523: CaseExpr caseExpr = new CaseExpr(new ValidTupleIdExpr(aggTids), caseWhenClauses, null);
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@551
PS1, Line 551: CaseExpr caseExpr = new CaseExpr(new ValidTupleIdExpr(aggTids), caseWhenClauses, null);
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@564
PS1, Line 564: CaseExpr caseExprForTids = new CaseExpr(new ValidTupleIdExpr(aggTids), caseWhenClauses, null);
line too long (98 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@595
PS1, Line 595: CaseExpr caseExpr = new CaseExpr(new ValidTupleIdExpr(aggTids), caseWhenClauses, null);
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/16115/1/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@597
PS1, Line 597: // wrap this in an aggif expr because an AggregateInfo only allows either aggregate exprs
line too long (95 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Jun 2020 18:54:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16115/3/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/16115/3/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@339
PS3, Line 339: // Register value transfers to allow predicate assignment through GROUP BY.
I discovered a bug here with the below query where it's not safe to push the predicate through a group by with grouping sets since output columns may be NULL even if the input is non-NULL. I think it's a similar challenge to pushing predicates through outer joins.
The obvious fix that I can see is just to not add these equivalences. I think it's safe if the column is present in all grouping sets, but I think that's probably not a common case.
explain select int_col, count(*) from alltypesagg group by rollup(int_col) having int_col is NULL
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Jul 2020 22:00:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6467/ : 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/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 17:34:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 5:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6087/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:15:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6477/ : 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/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Jul 2020 23:57:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16115
to look at the new patch set (#3).
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
IMPALA-9898: Plan generation and execution for grouping sets
This patch enhances the MultiAggregateInfo to handle grouping sets
and rollup (which is converted to grouping sets). This patch does
not itself do parsing/validation of grouping sets syntax but rather
provides the following supporting functionality:
- A separate analyze method that accepts aggregation classes and
aggregation info that have been created separately.
- A modified Transpose phase that uses combination of aggif(),
valid_tid() functions and CASE exprs to choose exactly which
slots from the underlying aggregate classes need to be output
based on the tuple id.
- Modified materialization step where all aggregate slots and
grouping slots are materialized in case of grouping sets.
- Creates grouping_id value for grouping sets. The grouping_id
function in SQL describes which expression is grouped-by in a
particular row of a query with grouping sets.
Testing:
This patch is not individually testable but will be tested
as part of the overall grouping set support.
Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
---
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
1 file changed, 266 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/16115/3
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:15:00 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16115/2/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/16115/2/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@544
PS2, Line 544: if (aggInfo.getGroupingExprs().size() == 0) {
> I think it would be more efficient to support not providing the NULL groupi
Yes, the NULL exprs are included in the intermediate tuples. I felt that getting the positional index right (for grouping_id/grouping function support) could be prone to bugs if all grouping sets did not have the fixed number of exprs. I have left this as-is for now in the latest patchset but if needed can do some refactoring as Part-2 based on the integration with your patch
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 30 Jun 2020 17:25:32 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Hello Tim Armstrong, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16115
to look at the new patch set (#4).
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
IMPALA-9898: Plan generation and execution for grouping sets
This patch enhances the MultiAggregateInfo to handle grouping sets
and rollup (which is converted to grouping sets). This patch does
not itself do parsing/validation of grouping sets syntax but rather
provides the following supporting functionality:
- A separate analyze method that accepts aggregation classes and
aggregation info that have been created separately.
- A modified Transpose phase that uses combination of aggif(),
valid_tid() functions and CASE exprs to choose exactly which
slots from the underlying aggregate classes need to be output
based on the tuple id.
- Modified materialization step where all aggregate slots and
grouping slots are materialized in case of grouping sets.
- Creates grouping_id value for grouping sets. The grouping_id
function in SQL describes which expression is grouped-by in a
particular row of a query with grouping sets.
Testing:
This patch is not individually testable but will be tested
as part of the overall grouping set support.
Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
---
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
1 file changed, 261 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/15/16115/4
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16115/3/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/16115/3/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@339
PS3, Line 339: List<SlotDescriptor> outputSlots = transposeAggInfo_.getResultTupleDesc().getSlots();
> Good find . Yeah, makes sense to not add the auxiliary predicate. In t
Done. I haven't tested it though since I don't have your changes.
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Jul 2020 23:34:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 4: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16115/3/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/16115/3/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@339
PS3, Line 339: List<SlotDescriptor> outputSlots = transposeAggInfo_.getResultTupleDesc().getSlots();
> Done. I haven't tested it though since I don't have your changes.
I rebased my patch onto this and it works fine - I have a couple of planner and query tests
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 4
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jul 2020 00:14:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 5: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 5
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 02 Jul 2020 05:19:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
IMPALA-9898: Plan generation and execution for grouping sets
This patch enhances the MultiAggregateInfo to handle grouping sets
and rollup (which is converted to grouping sets). This patch does
not itself do parsing/validation of grouping sets syntax but rather
provides the following supporting functionality:
- A separate analyze method that accepts aggregation classes and
aggregation info that have been created separately.
- A modified Transpose phase that uses combination of aggif(),
valid_tid() functions and CASE exprs to choose exactly which
slots from the underlying aggregate classes need to be output
based on the tuple id.
- Modified materialization step where all aggregate slots and
grouping slots are materialized in case of grouping sets.
- Creates grouping_id value for grouping sets. The grouping_id
function in SQL describes which expression is grouped-by in a
particular row of a query with grouping sets.
Testing:
This patch is not individually testable but will be tested
as part of the overall grouping set support.
Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Reviewed-on: http://gerrit.cloudera.org:8080/16115
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java
1 file changed, 261 insertions(+), 0 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 6
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] [WIP] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: [WIP] IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/6436/ : 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/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 1
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 26 Jun 2020 19:31:32 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9898: Plan generation and execution for grouping sets
Posted by "Aman Sinha (Code Review)" <ge...@cloudera.org>.
Aman Sinha has posted comments on this change. ( http://gerrit.cloudera.org:8080/16115 )
Change subject: IMPALA-9898: Plan generation and execution for grouping sets
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16115/3/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/16115/3/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@339
PS3, Line 339: // Register value transfers to allow predicate assignment through GROUP BY.
> I discovered a bug here with the below query where it's not safe to push th
Good find . Yeah, makes sense to not add the auxiliary predicate. In the ROLLUP case, there a final grouping set where all group-by exprs are NULL even though the leading column will be common to all the other grouping sets.
I will remove this creation of aux predicate and update this patch.
--
To view, visit http://gerrit.cloudera.org:8080/16115
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id474c5373860b0d8014ee9c844a3fb90092be968
Gerrit-Change-Number: 16115
Gerrit-PatchSet: 3
Gerrit-Owner: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 01 Jul 2020 22:27:24 +0000
Gerrit-HasComments: Yes