> On Feb. 7, 2018, 8:31 p.m., Jesús Camacho Rodríguez wrote: > > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > > Lines 3891 (patched) > > <https://reviews.apache.org/r/64900/diff/5-6/?file=1954041#file1954041line3891> > > > > The check should be on _groupByExpr.size()_? Even if we have a single > > grouping sets clause, if number of columns is greater than Long.SIZE, we > > will overflow in L3876. > > Prasanth_J wrote: > I guess that overflow in L3876 doesn't matter. If result bitmap list has > > 64 items (even if there are overflows) it means we are dealing with more > than 64 possible aggregations. > > Also just noticed we don't de-dedup columns/groups inside grouping sets > (that can be follow up). Not sure what the SQL standard is for duplicating > columns/groups inside grouping sets.
I am not sure what we do with duplicates, there are some checks in Calcite some should consolidate them, but not sure when we come from Hive. No need to tackle here, we can create a follow-up if we want to explore. I agree about your comment, but if we only check for the size of results, we miss the case that I mentioned: less than 64 grouping sets expressions, but the number of grouping columns greater than 64 (not all grouping sets need to be present). You can check for the size of groupByExpr before looping and fail accordingly. - Jesús ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64900/#review197026 ----------------------------------------------------------- On Feb. 7, 2018, 8:21 p.m., Prasanth_J wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64900/ > ----------------------------------------------------------- > > (Updated Feb. 7, 2018, 8:21 p.m.) > > > Review request for hive and Jesús Camacho Rodríguez. > > > Bugs: HIVE-18359 > https://issues.apache.org/jira/browse/HIVE-18359 > > > Repository: hive-git > > > Description > ------- > > HIVE-18359: Extend grouping set limits from int to long > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/ErrorMsg.java 134faee > ql/src/java/org/apache/hadoop/hive/ql/exec/GroupByOperator.java 6de979e > > ql/src/java/org/apache/hadoop/hive/ql/exec/vector/VectorGroupByOperator.java > e670409 > ql/src/java/org/apache/hadoop/hive/ql/metadata/VirtualColumn.java 2411d3a > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/reloperators/HiveGroupingID.java > 4ba27a2 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveExpandDistinctAggregatesRule.java > 864efa4 > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/translator/HiveGBOpConvUtil.java > f22cd94 > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 85a1f34 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java b67a03f > ql/src/java/org/apache/hadoop/hive/ql/plan/GroupByDesc.java e90a398 > ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFGrouping.java > c0c3015 > ql/src/test/queries/clientnegative/groupby_grouping_sets8.q PRE-CREATION > ql/src/test/queries/clientpositive/cte_1.q 15d3f06 > ql/src/test/queries/clientpositive/groupingset_high_columns.q PRE-CREATION > ql/src/test/results/clientnegative/groupby_grouping_sets8.q.out > PRE-CREATION > ql/src/test/results/clientpositive/annotate_stats_groupby.q.out ed3d594 > ql/src/test/results/clientpositive/annotate_stats_groupby2.q.out ffcb20f > ql/src/test/results/clientpositive/cbo_rp_annotate_stats_groupby.q.out > 3d92a0d > ql/src/test/results/clientpositive/groupby_cube1.q.out e5ece81 > ql/src/test/results/clientpositive/groupby_cube_multi_gby.q.out 9a6457c > ql/src/test/results/clientpositive/groupby_grouping_id3.q.out f13b6e5 > ql/src/test/results/clientpositive/groupby_grouping_sets1.q.out d70f065 > ql/src/test/results/clientpositive/groupby_grouping_sets2.q.out 453b9f7 > ql/src/test/results/clientpositive/groupby_grouping_sets3.q.out be8d20e > ql/src/test/results/clientpositive/groupby_grouping_sets4.q.out 0c6ead9 > ql/src/test/results/clientpositive/groupby_grouping_sets5.q.out 0bb12e1 > ql/src/test/results/clientpositive/groupby_grouping_sets6.q.out 5b990a1 > ql/src/test/results/clientpositive/groupby_grouping_sets_grouping.q.out > 1f2f86b > ql/src/test/results/clientpositive/groupby_grouping_sets_limit.q.out > b25b0e5 > ql/src/test/results/clientpositive/groupby_grouping_window.q.out 32135e4 > ql/src/test/results/clientpositive/groupby_rollup1.q.out bc1d8a9 > ql/src/test/results/clientpositive/groupby_rollup_empty.q.out 2756d38 > ql/src/test/results/clientpositive/groupingset_high_columns.q.out > PRE-CREATION > > ql/src/test/results/clientpositive/infer_bucket_sort_grouping_operators.q.out > 5f1d264 > ql/src/test/results/clientpositive/limit_pushdown2.q.out cdd221b > ql/src/test/results/clientpositive/llap/cte_1.q.out ddef9db > ql/src/test/results/clientpositive/llap/groupby_rollup_empty.q.out f2cda04 > ql/src/test/results/clientpositive/llap/llap_acid.q.out 38889b9 > ql/src/test/results/clientpositive/llap/llap_acid_fast.q.out 4a7297d > ql/src/test/results/clientpositive/llap/multi_count_distinct_null.q.out > 1f17f96 > ql/src/test/results/clientpositive/llap/vector_groupby_cube1.q.out 39de888 > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_id1.q.out > 7224d59 > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_id2.q.out > e6075c7 > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_id3.q.out > da4b81f > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets1.q.out > d2b738b > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets2.q.out > 1877bba > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets3.q.out > 7c9f668 > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets4.q.out > 6a5e679 > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets5.q.out > 4d8fa16 > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets6.q.out > 5e9e204 > > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets_grouping.q.out > b81a0d3 > > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets_limit.q.out > e8ca06e > > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_window.q.out > 4de6ebb > ql/src/test/results/clientpositive/llap/vector_groupby_rollup1.q.out > d1263cd > ql/src/test/results/clientpositive/llap/vector_grouping_sets.q.out 0fc4b06 > ql/src/test/results/clientpositive/perf/spark/query18.q.out 88d289c > ql/src/test/results/clientpositive/perf/spark/query22.q.out 15fe441 > ql/src/test/results/clientpositive/perf/spark/query27.q.out 627821f > ql/src/test/results/clientpositive/perf/spark/query36.q.out d313337 > ql/src/test/results/clientpositive/perf/spark/query5.q.out 14e0bdb > ql/src/test/results/clientpositive/perf/spark/query67.q.out 25d37bd > ql/src/test/results/clientpositive/perf/spark/query70.q.out 862bdb0 > ql/src/test/results/clientpositive/perf/spark/query77.q.out 6dcaf7c > ql/src/test/results/clientpositive/perf/spark/query80.q.out 606d23c > ql/src/test/results/clientpositive/perf/spark/query86.q.out 8e159fd > ql/src/test/results/clientpositive/spark/groupby_cube1.q.out fa1480e > ql/src/test/results/clientpositive/spark/groupby_rollup1.q.out 460e7db > ql/src/test/results/clientpositive/spark/limit_pushdown2.q.out f6f0043 > ql/src/test/results/clientpositive/tez/multi_count_distinct.q.out bb08f16 > ql/src/test/results/clientpositive/vector_grouping_sets.q.out f08c270 > ql/src/test/results/clientpositive/view_cbo.q.out e10cb7a > > > Diff: https://reviews.apache.org/r/64900/diff/6/ > > > Testing > ------- > > > Thanks, > > Prasanth_J > >
