----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71730/#review218593 -----------------------------------------------------------
ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java Lines 76 (patched) <https://reviews.apache.org/r/71730/#comment306355> Can we encapsulate this logic in a utility method in ObjectInspectorUtils? ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java Lines 76 (patched) <https://reviews.apache.org/r/71730/#comment306356> Can we encapsulate this logic in a utility method in ObjectInspectorUtils? ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java Lines 118 (patched) <https://reviews.apache.org/r/71730/#comment306357> Doesn't assigning MAXVALUE or MINVALUE for NULLS in the comparator depends on whether we use using ASC or DESC order, e.g., if ASC and NULLS LAST, MAXVALUE; if DESC and NULLS LAST, MINVALUE? Otherwise, `reverse` below would be reversing the nulls order too? ql/src/java/org/apache/hadoop/hive/ql/plan/TopNKeyDesc.java Lines 72 (patched) <https://reviews.apache.org/r/71730/#comment306358> <p>I think this property and the corresponding one in RS should be exposed at default level now. Can you create a follow-up JIRA for that? It will probably change all q files in Hive.</p> - Jesús Camacho Rodríguez On Nov. 7, 2019, 5:39 a.m., Krisztian Kasa wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71730/ > ----------------------------------------------------------- > > (Updated Nov. 7, 2019, 5:39 a.m.) > > > Review request for hive, Jesús Camacho Rodríguez, Zoltan Haindrich, and > Vineet Garg. > > > Bugs: HIVE-22464 > https://issues.apache.org/jira/browse/HIVE-22464 > > > Repository: hive-git > > > Description > ------- > > Implement support for NULLS FIRST/LAST in TopNKeyOperator > ========================================================= > When introducing the TopNKey operator get the nulls order from the Reduce > Sink operator and pass it to the newly created TopNKey opertor. > When initializing the TopNKey operator create object comparator for each key > column which wraps the specified nulls order. The `ObjectComparator.compare` > calls `ObjectInspectorUtils.compare` and passes nulls order. > > > Diffs > ----- > > ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java 4734824b73 > ql/src/java/org/apache/hadoop/hive/ql/optimizer/TopNKeyProcessor.java > 721a9b9998 > ql/src/java/org/apache/hadoop/hive/ql/plan/TopNKeyDesc.java c62c4a9b66 > ql/src/test/queries/clientpositive/topnkey.q e02a41dd57 > ql/src/test/results/clientpositive/llap/topnkey.q.out e3c0bc102f > ql/src/test/results/clientpositive/tez/topnkey.q.out 3267f792e0 > ql/src/test/results/clientpositive/topnkey.q.out 7ea1a7a93d > > serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectComparator.java > PRE-CREATION > > > Diff: https://reviews.apache.org/r/71730/diff/1/ > > > Testing > ------- > > - Extend topnkey.q with creating a test table and adding rows with null > values. > - Query from the test table. > > > Thanks, > > Krisztian Kasa > >
