hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jesús Camacho Rodríguez <jcamachorodrig...@hortonworks.com>
Subject Re: Review Request 53328: Support for standard ROLLUP syntax
Date Tue, 01 Nov 2016 00:07:41 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message