Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16128 )
Change subject: WIP - IMPALA-9898: generate grouping set plans ...................................................................... Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/16128/5/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/5/fe/src/main/java/org/apache/impala/analysis/GroupByClause.java@145 PS5, Line 145: } There's a problem here that affects plan generation - we need to include grouping expressions that were added during rewrites and aren't part of the original grouping sets. http://gerrit.cloudera.org:8080/#/c/16128/5/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/5/fe/src/main/java/org/apache/impala/analysis/MultiAggregateInfo.java@227 PS5, Line 227: return; > Set isAnalyzed_ = true ? Seems analyzeGroupingSets() is not setting this f I'm relying on analyzeCustomClasses to do that, but it's kinda buried - added a comment in analyzeGroupingSets(). -- 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: 6 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Aman Sinha <amsi...@cloudera.com> Gerrit-Reviewer: David Rorke <dro...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 01 Jul 2020 03:59:19 +0000 Gerrit-HasComments: Yes
