----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56140/#review175299 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectSortTransposeRule.java Lines 70-71 (original), 81-83 (patched) <https://reviews.apache.org/r/56140/#comment248786> This change looks correct. But don't understand why it was needed. Can you describe the need for it? ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Line 2870 (original), 2870 (patched) <https://reviews.apache.org/r/56140/#comment248793> Add comment about need to pass true for last argument. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Lines 3093 (patched) <https://reviews.apache.org/r/56140/#comment248789> This log message is confusing. It should be at debug level and should be like, didn't find column in parent of order by, will look into its parent next. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Lines 3214 (patched) <https://reviews.apache.org/r/56140/#comment248790> Add comment about we need to add select since order by schema may have more columns than result schema. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Line 3516 (original), 3551 (patched) <https://reviews.apache.org/r/56140/#comment248787> Add javadoc for this new boolean argument. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Lines 3857-3859 (patched) <https://reviews.apache.org/r/56140/#comment248788> This log message is confusing, because we expect this to happen as there will be overlap in columns of select and order by. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Line 4047 (original), 4128 (patched) <https://reviews.apache.org/r/56140/#comment248791> Remove the comment. ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java Lines 4049-4059 (original) <https://reviews.apache.org/r/56140/#comment248792> Aren't all these restrictions still valid? ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java Lines 183-189 (patched) <https://reviews.apache.org/r/56140/#comment248769> This seems to undo rest of logic of the function, ie it skips the duplicate checking which is required. Whats the need of this change ? ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java Line 463 (original), 471-475 (patched) <https://reviews.apache.org/r/56140/#comment248770> Whats the need of deep-copy here? ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java Lines 1619 (patched) <https://reviews.apache.org/r/56140/#comment248751> Comment : If there are aggregations in order by, we need to remember them in qb. - Ashutosh Chauhan On May 1, 2017, 5:30 p.m., pengcheng xiong wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56140/ > ----------------------------------------------------------- > > (Updated May 1, 2017, 5:30 p.m.) > > > Review request for hive and Ashutosh Chauhan. > > > Repository: hive-git > > > Description > ------- > > HIVE-15160 > > > Diffs > ----- > > > ql/src/java/org/apache/hadoop/hive/ql/optimizer/calcite/rules/HiveProjectSortTransposeRule.java > 1487ed4f8e > ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java 1b054a7e24 > ql/src/java/org/apache/hadoop/hive/ql/parse/RowResolver.java 262dafb487 > ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java > 654f3b1772 > ql/src/java/org/apache/hadoop/hive/ql/parse/TypeCheckProcFactory.java > 8f8eab0d9c > ql/src/test/queries/clientpositive/order_by_expr_1.q PRE-CREATION > ql/src/test/queries/clientpositive/order_by_expr_2.q PRE-CREATION > ql/src/test/results/clientpositive/annotate_stats_select.q.out 873f1abb25 > ql/src/test/results/clientpositive/cp_sel.q.out 1778ccd6a6 > ql/src/test/results/clientpositive/druid_basic2.q.out 6177d56987 > ql/src/test/results/clientpositive/dynamic_rdd_cache.q.out 2abb819558 > ql/src/test/results/clientpositive/groupby_grouping_sets_grouping.q.out > 473d17a1bd > ql/src/test/results/clientpositive/llap/bucket_groupby.q.out d724131fca > ql/src/test/results/clientpositive/llap/explainuser_1.q.out 584c3b5520 > ql/src/test/results/clientpositive/llap/limit_pushdown.q.out dd54dd22a6 > ql/src/test/results/clientpositive/llap/limit_pushdown3.q.out 24645b6426 > ql/src/test/results/clientpositive/llap/offset_limit_ppd_optimizer.q.out > 83de1fbea1 > ql/src/test/results/clientpositive/llap/vector_coalesce.q.out 578f849bdb > ql/src/test/results/clientpositive/llap/vector_date_1.q.out a4f1050c89 > ql/src/test/results/clientpositive/llap/vector_decimal_2.q.out 144356c108 > ql/src/test/results/clientpositive/llap/vector_decimal_round.q.out > 8bd80cf860 > > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets_grouping.q.out > 5af9e61b0a > > ql/src/test/results/clientpositive/llap/vector_groupby_grouping_sets_limit.q.out > f731ceecdc > ql/src/test/results/clientpositive/llap/vector_interval_1.q.out debf5ab39e > ql/src/test/results/clientpositive/llap/vector_interval_arithmetic.q.out > aadb6e72cd > ql/src/test/results/clientpositive/order3.q.out 898f7a8853 > ql/src/test/results/clientpositive/order_by_expr_1.q.out PRE-CREATION > ql/src/test/results/clientpositive/order_by_expr_2.q.out PRE-CREATION > ql/src/test/results/clientpositive/pcr.q.out a1301fdf79 > ql/src/test/results/clientpositive/perf/query31.q.out 3ed312d3e3 > ql/src/test/results/clientpositive/perf/query36.q.out 57ab26acc6 > ql/src/test/results/clientpositive/perf/query39.q.out 19472c4d5e > ql/src/test/results/clientpositive/perf/query42.q.out 3bebac3321 > ql/src/test/results/clientpositive/perf/query52.q.out 74ecaf28ba > ql/src/test/results/clientpositive/perf/query64.q.out 6b42393aad > ql/src/test/results/clientpositive/perf/query66.q.out 072bfee92b > ql/src/test/results/clientpositive/perf/query70.q.out 8e42fac9c5 > ql/src/test/results/clientpositive/perf/query75.q.out b1e236d325 > ql/src/test/results/clientpositive/perf/query81.q.out a09d5c99b5 > ql/src/test/results/clientpositive/perf/query85.q.out 168bcd2a4a > ql/src/test/results/clientpositive/perf/query86.q.out 734e6a480b > ql/src/test/results/clientpositive/perf/query89.q.out 66481f710b > ql/src/test/results/clientpositive/perf/query91.q.out e592bba8d9 > ql/src/test/results/clientpositive/pointlookup2.q.out 3438c74608 > ql/src/test/results/clientpositive/pointlookup3.q.out 2c3e39fd15 > ql/src/test/results/clientpositive/ppd_udf_case.q.out 7678d03415 > ql/src/test/results/clientpositive/spark/dynamic_rdd_cache.q.out 6572511967 > ql/src/test/results/clientpositive/spark/limit_pushdown.q.out ede0096c73 > ql/src/test/results/clientpositive/spark/pcr.q.out 77ac020d07 > ql/src/test/results/clientpositive/vector_coalesce.q.out f158236beb > ql/src/test/results/clientpositive/vector_date_1.q.out c2389e6b1e > ql/src/test/results/clientpositive/vector_decimal_round.q.out de49c170cf > ql/src/test/results/clientpositive/vector_interval_1.q.out f53a2c2db5 > ql/src/test/results/clientpositive/vector_interval_arithmetic.q.out > 75250e30a4 > ql/src/test/results/clientpositive/view_alias.q.out 90bf28dd9b > > > Diff: https://reviews.apache.org/r/56140/diff/8/ > > > Testing > ------- > > > Thanks, > > pengcheng xiong > >
