----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/53328/#review154349 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g (line 60) <https://reviews.apache.org/r/53328/#comment223868> Since sets is not used in that syntax, maybe it is easier to create a parser rule that rewrites GROUP BY (e1, e2, e3) WITH ROLLUP into ROLLUP(e1, e2, e3) and GROUP BY (e1, e2, e3) WITH CUBE into CUBE(e1, e2, e3) Then the rule with the old syntax will kick in. The advantage with this approach is that we will keep a single rule that actually generates the syntax that SemanticAnalyzer receives. What do you think? ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g (line 64) <https://reviews.apache.org/r/53328/#comment223865> Instead of leaving "WITH CUBE", could you add a new "cube" syntax (similar to rollup) to support GROUP BY CUBE(e1, e2, e3) ? ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g (line 65) <https://reviews.apache.org/r/53328/#comment223867> We can remove the sets option completely from the new syntax, since we will never reach it. - Jesús Camacho Rodríguez On Oct. 31, 2016, 11:27 p.m., Vineet Garg wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/53328/ > ----------------------------------------------------------- > > (Updated Oct. 31, 2016, 11:27 p.m.) > > > Review request for hive and Jesús Camacho Rodríguez. > > > Repository: hive-git > > > Description > ------- > > Standard ROLLUP syntax is GROUP BY ROLLUP (expression list)... but HIVE > allows GROUP BY <expression list> WITH ROLLUP syntax. We would like HIVE to > support standard ROLLUP syntax to allow out of the box support for TPCDS > queries i.e. without rewritting them. > > This patach includes update to grammar to allow ROLLUP in following syntax: > > SELECT.....GROUP BY ROLLUP ( expr1, expr2....) > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/parse/IdentifiersParser.g 13e2d17 > ql/src/test/queries/clientpositive/annotate_stats_groupby.q 854e401 > ql/src/test/queries/clientpositive/cbo_rp_annotate_stats_groupby.q 3159fc7 > ql/src/test/queries/clientpositive/cte_1.q 2956339 > ql/src/test/queries/clientpositive/groupby_grouping_id1.q de4a7c3 > ql/src/test/queries/clientpositive/groupby_grouping_id2.q 5c05aad > ql/src/test/queries/clientpositive/groupby_rollup1.q 23cac80 > ql/src/test/queries/clientpositive/infer_bucket_sort_grouping_operators.q > 928f6fb > ql/src/test/queries/clientpositive/limit_pushdown2.q 637b5b0 > ql/src/test/queries/clientpositive/vector_grouping_sets.q 09ba6b6 > ql/src/test/results/clientpositive/annotate_stats_groupby.q.out f6971a0 > ql/src/test/results/clientpositive/cbo_rp_annotate_stats_groupby.q.out > f5b4375 > ql/src/test/results/clientpositive/cte_1.q.out 61fd1af > ql/src/test/results/clientpositive/groupby_grouping_id1.q.out 136edeb > ql/src/test/results/clientpositive/groupby_rollup1.q.out 54e1a0d > > ql/src/test/results/clientpositive/infer_bucket_sort_grouping_operators.q.out > ebfce60 > ql/src/test/results/clientpositive/limit_pushdown2.q.out 2f68674 > ql/src/test/results/clientpositive/llap/cte_1.q.out e309ce8 > ql/src/test/results/clientpositive/llap/groupby_grouping_id2.q.out 544a7ae > ql/src/test/results/clientpositive/llap/vector_grouping_sets.q.out 8e55ce3 > ql/src/test/results/clientpositive/vector_grouping_sets.q.out 4207c19 > > Diff: https://reviews.apache.org/r/53328/diff/ > > > Testing > ------- > > Updated exsting tests to use new ROLLUP syntax in addition to non-standard > syntax. > > > Thanks, > > Vineet Garg > >
