----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71669/#review218405 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g Line 2505 (original), 2505 (patched) <https://reviews.apache.org/r/71669/#comment306118> ` && !nullsLast()` not needed? ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g Line 2507 (original), 2507 (patched) <https://reviews.apache.org/r/71669/#comment306117> Can we add a few comments on top of each of this matching logic that helps to understand when each branch is triggered? For instance: // ORDER not present and NULLS ORDER present ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g Line 2509 (original), 2509 (patched) <https://reviews.apache.org/r/71669/#comment306121> `$nullSpec.tree == null` not needed here and in below conditions? ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g Lines 2511 (patched) <https://reviews.apache.org/r/71669/#comment306119> `!nullsLast()` not needed? // ASC ORDER, NULLS ORDER not present, default NULLS FIRST ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g Lines 2515 (patched) <https://reviews.apache.org/r/71669/#comment306120> `!nullsLast()` not needed? - Jesús Camacho Rodríguez On Oct. 24, 2019, 6:51 a.m., Krisztian Kasa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71669/ > ----------------------------------------------------------- > > (Updated Oct. 24, 2019, 6:51 a.m.) > > > Review request for hive, Jesús Camacho Rodríguez, Zoltan Haindrich, and > Vineet Garg. > > > Bugs: HIVE-22321 > https://issues.apache.org/jira/browse/HIVE-22321 > > > Repository: hive-git > > > Description > ------- > > Setting default nulls last does not take effect when order direction is > specified > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g bb23a4e076 > ql/src/test/results/clientpositive/llap/order_null.q.out c380844749 > ql/src/test/results/clientpositive/llap/sample10.q.out 20c38ace70 > ql/src/test/results/clientpositive/llap/vector_char_2.q.out 80ba95dd21 > ql/src/test/results/clientpositive/llap/vector_order_null.q.out 1379c399b3 > ql/src/test/results/clientpositive/llap/vector_windowing_gby2.q.out > 769e8ab665 > ql/src/test/results/clientpositive/perf/spark/query36.q.out cde0e93e0a > ql/src/test/results/clientpositive/perf/spark/query44.q.out c7c7965d91 > ql/src/test/results/clientpositive/perf/tez/cbo_query36.q.out f0204dd970 > ql/src/test/results/clientpositive/perf/tez/constraints/cbo_query36.q.out > 783245824a > ql/src/test/results/clientpositive/perf/tez/constraints/mv_query44.q.out > f56f9889b7 > ql/src/test/results/clientpositive/perf/tez/constraints/query36.q.out > 40b271649f > ql/src/test/results/clientpositive/perf/tez/constraints/query44.q.out > 80a9cdc356 > ql/src/test/results/clientpositive/perf/tez/query36.q.out 7e1b4d0af5 > ql/src/test/results/clientpositive/perf/tez/query44.q.out 0017e66090 > ql/src/test/results/clientpositive/vector_char_2.q.out aa627a8312 > ql/src/test/results/clientpositive/windowing_gby2.q.out d8de2fb86b > > > Diff: https://reviews.apache.org/r/71669/diff/1/ > > > Testing > ------- > > Run existing q test > > > Thanks, > > Krisztian Kasa > >
