diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapper.java ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapper.java index fee8081652..2459dc470a 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapper.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapper.java @@ -27,4 +27,5 @@ public abstract KeyWrapper copyKey(); public abstract void copyKey(KeyWrapper oldWrapper); public abstract Object[] getKeyArray(); + public abstract boolean isCopy(); } diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperComparator.java ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperComparator.java new file mode 100644 index 0000000000..eec4416538 --- /dev/null +++ ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperComparator.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hive.ql.exec; + +import java.util.Comparator; + +import org.apache.hadoop.hive.ql.util.NullOrdering; +import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector; +import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils; +import static org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils.NullValueOption; + +/** + * Comparator for {@link KeyWrapper}. + * This comparator uses the {@link ObjectInspector}s provided in the newObjectInspectors array to extract key values + * from wrapped Object arrays by default. + * {@link KeyWrapper} instances are cloned when {@link KeyWrapper#copyKey()} is called. In this case the wrapped + * key values are deep copied and converted to standard objects using {@link ObjectInspectorUtils#copyToStandardObject}. + * The comparator uses copyObjectInspectors when extracting key values from clones. + */ +public class KeyWrapperComparator implements Comparator { + + private final ObjectInspector[] newObjectInspectors; + private final ObjectInspector[] copyObjectInspectors; + private final boolean[] columnSortOrderIsDesc; + private final NullValueOption[] nullSortOrder; + + KeyWrapperComparator(ObjectInspector[] newObjectInspectors, ObjectInspector[] copyObjectInspectors, + String columnSortOrder, String nullSortOrder) { + this.newObjectInspectors = newObjectInspectors; + this.copyObjectInspectors = copyObjectInspectors; + this.columnSortOrderIsDesc = new boolean[columnSortOrder.length()]; + this.nullSortOrder = new NullValueOption[nullSortOrder.length()]; + for (int i = 0; i < columnSortOrder.length(); ++i) { + this.columnSortOrderIsDesc[i] = columnSortOrder.charAt(i) == '-'; + this.nullSortOrder[i] = NullOrdering.fromSign(nullSortOrder.charAt(i)).getNullValueOption(); + } + } + + @Override + public int compare(KeyWrapper key1, KeyWrapper key2) { + return ObjectInspectorUtils.compare( + key1.getKeyArray(), + key1.isCopy() ? copyObjectInspectors : newObjectInspectors, + key2.getKeyArray(), + key2.isCopy() ? copyObjectInspectors : newObjectInspectors, + columnSortOrderIsDesc, + nullSortOrder); + } +} diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperFactory.java ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperFactory.java index f1bf9023e1..93abc47525 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperFactory.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/KeyWrapperFactory.java @@ -67,6 +67,13 @@ public KeyWrapper getKeyWrapper() { class ListKeyWrapper extends KeyWrapper { int hashcode = -1; Object[] keys; + // true means this instance is a copy of another ListKeyWrapper instance. It was created using the copyKey() and the + // keys array contains StandardObjects created by ObjectInspectorUtils.copyToStandardObject() + // currentStructEqualComparer should be used for equality check + // false means this instance was created by the KeyWrapperFactory.getKeyWrapper() method. + // newKeyStructEqualComparer should be used for equality check + private final boolean isCopy; + @Override public String toString() { return "ListKeyWrapper [keys=" + Arrays.toString(keys) + "]"; @@ -86,6 +93,7 @@ private ListKeyWrapper(int hashcode, Object[] copiedKeys, this.hashcode = hashcode; keys = copiedKeys; setEqualComparer(isCopy); + this.isCopy = isCopy; } private void setEqualComparer(boolean copy) { @@ -150,6 +158,11 @@ public void copyKey(KeyWrapper oldWrapper) { return keys; } + @Override + public boolean isCopy() { + return isCopy; + } + private Object[] deepCopyElements(Object[] keys, ObjectInspector[] keyObjectInspectors, ObjectInspectorCopyOption copyOption) { @@ -257,5 +270,10 @@ public void copyKey(KeyWrapper oldWrapper) { singleEleArray[0] = key; return singleEleArray; } + + @Override + public boolean isCopy() { + return isCopy; + } } } diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java index 4e35922e5f..bbbde7978b 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/TopNKeyOperator.java @@ -24,16 +24,12 @@ import org.apache.hadoop.hive.ql.plan.ExprNodeDesc; import org.apache.hadoop.hive.ql.plan.TopNKeyDesc; import org.apache.hadoop.hive.ql.plan.api.OperatorType; -import org.apache.hadoop.hive.ql.util.NullOrdering; import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspector; import org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils; import java.io.Serializable; -import java.util.Comparator; -import java.util.List; import static org.apache.hadoop.hive.ql.plan.api.OperatorType.TOPNKEY; -import static org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils.NullValueOption; /** * TopNKeyOperator passes rows that contains top N keys only. @@ -55,27 +51,6 @@ public TopNKeyOperator(CompilationOpContext ctx) { super(ctx); } - public static class KeyWrapperComparator implements Comparator { - - private final List> comparatorList; - - KeyWrapperComparator(ObjectInspector[] keyObjectInspectors, String columnSortOrder, String nullSortOrder) { - boolean[] columnSortOrderIsDesc = new boolean[columnSortOrder.length()]; - NullValueOption[] nullSortOrderArray = new NullValueOption[nullSortOrder.length()]; - for (int i = 0; i < columnSortOrder.length(); ++i) { - columnSortOrderIsDesc[i] = columnSortOrder.charAt(i) == '-'; - nullSortOrderArray[i] = NullOrdering.fromSign(nullSortOrder.charAt(i)).getNullValueOption(); - } - comparatorList = ObjectInspectorUtils.getComparator( - keyObjectInspectors, keyObjectInspectors, columnSortOrderIsDesc, nullSortOrderArray); - } - - @Override - public int compare(KeyWrapper key1, KeyWrapper key2) { - return ObjectInspectorUtils.compare(comparatorList, key1.getKeyArray(), key2.getKeyArray()); - } - } - @Override protected void initializeOp(Configuration hconf) throws HiveException { super.initializeOp(hconf); @@ -102,8 +77,8 @@ protected void initializeOp(Configuration hconf) throws HiveException { standardKeyObjectInspectors[i] = standardKeyFields[i].initialize(standardObjInspector); } - this.topNKeyFilter = new TopNKeyFilter<>(conf.getTopN(), new TopNKeyOperator.KeyWrapperComparator( - standardKeyObjectInspectors, columnSortOrder, nullSortOrder)); + this.topNKeyFilter = new TopNKeyFilter<>(conf.getTopN(), new KeyWrapperComparator( + keyObjectInspectors, standardKeyObjectInspectors, columnSortOrder, nullSortOrder)); KeyWrapperFactory keyWrapperFactory = new KeyWrapperFactory(keyFields, keyObjectInspectors, standardKeyObjectInspectors); diff --git ql/src/java/org/apache/hadoop/hive/ql/exec/vector/wrapper/VectorHashKeyWrapperBase.java ql/src/java/org/apache/hadoop/hive/ql/exec/vector/wrapper/VectorHashKeyWrapperBase.java index 1bb224917e..04e8aad30b 100644 --- ql/src/java/org/apache/hadoop/hive/ql/exec/vector/wrapper/VectorHashKeyWrapperBase.java +++ ql/src/java/org/apache/hadoop/hive/ql/exec/vector/wrapper/VectorHashKeyWrapperBase.java @@ -220,4 +220,9 @@ public void setNull() { public boolean isNull(int keyIndex) { throw new RuntimeException("Not implemented"); } + + @Override + public boolean isCopy() { + throw new RuntimeException("Not implemented"); + } } diff --git ql/src/java/org/apache/hadoop/hive/ql/util/NullOrdering.java ql/src/java/org/apache/hadoop/hive/ql/util/NullOrdering.java index 46ff329981..3bd25cd075 100644 --- ql/src/java/org/apache/hadoop/hive/ql/util/NullOrdering.java +++ ql/src/java/org/apache/hadoop/hive/ql/util/NullOrdering.java @@ -24,8 +24,8 @@ * Enum for converting different Null ordering description types. */ public enum NullOrdering { - NULLS_FIRST(1, HiveParser.TOK_NULLS_FIRST, NullValueOption.MAXVALUE, 'a'), - NULLS_LAST(0, HiveParser.TOK_NULLS_LAST, NullValueOption.MINVALUE, 'z'); + NULLS_FIRST(1, HiveParser.TOK_NULLS_FIRST, NullValueOption.MINVALUE, 'a'), + NULLS_LAST(0, HiveParser.TOK_NULLS_LAST, NullValueOption.MAXVALUE, 'z'); NullOrdering(int code, int token, NullValueOption nullValueOption, char sign) { this.code = code; diff --git ql/src/test/queries/clientpositive/hypothetical_set_aggregates.q ql/src/test/queries/clientpositive/hypothetical_set_aggregates.q index 6b5f3765e9..9a8db07b36 100644 --- ql/src/test/queries/clientpositive/hypothetical_set_aggregates.q +++ ql/src/test/queries/clientpositive/hypothetical_set_aggregates.q @@ -45,6 +45,14 @@ rank(7) WITHIN GROUP (ORDER BY col1), rank(8) WITHIN GROUP (ORDER BY col1) from t_test; +select +rank(4) WITHIN GROUP (ORDER BY col1 nulls first), +rank(4) WITHIN GROUP (ORDER BY col1 nulls last), +rank(4) WITHIN GROUP (ORDER BY col1 desc), +rank(4) WITHIN GROUP (ORDER BY col1 desc nulls first), +rank(4) WITHIN GROUP (ORDER BY col1 desc nulls last) +from t_test; + select rank(1, 3) WITHIN GROUP (ORDER BY col1, col2), rank(2, 3) WITHIN GROUP (ORDER BY col1, col2), diff --git ql/src/test/results/clientpositive/hypothetical_set_aggregates.q.out ql/src/test/results/clientpositive/hypothetical_set_aggregates.q.out index 3ea6f1f4e5..65fee28227 100644 --- ql/src/test/results/clientpositive/hypothetical_set_aggregates.q.out +++ ql/src/test/results/clientpositive/hypothetical_set_aggregates.q.out @@ -130,7 +130,28 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 5 6 6 10 10 11 +1 1 1 2 2 6 6 7 +PREHOOK: query: select +rank(4) WITHIN GROUP (ORDER BY col1 nulls first), +rank(4) WITHIN GROUP (ORDER BY col1 nulls last), +rank(4) WITHIN GROUP (ORDER BY col1 desc), +rank(4) WITHIN GROUP (ORDER BY col1 desc nulls first), +rank(4) WITHIN GROUP (ORDER BY col1 desc nulls last) +from t_test +PREHOOK: type: QUERY +PREHOOK: Input: default@t_test +#### A masked pattern was here #### +POSTHOOK: query: select +rank(4) WITHIN GROUP (ORDER BY col1 nulls first), +rank(4) WITHIN GROUP (ORDER BY col1 nulls last), +rank(4) WITHIN GROUP (ORDER BY col1 desc), +rank(4) WITHIN GROUP (ORDER BY col1 desc nulls first), +rank(4) WITHIN GROUP (ORDER BY col1 desc nulls last) +from t_test +POSTHOOK: type: QUERY +POSTHOOK: Input: default@t_test +#### A masked pattern was here #### +6 2 17 13 17 PREHOOK: query: select rank(1, 3) WITHIN GROUP (ORDER BY col1, col2), rank(2, 3) WITHIN GROUP (ORDER BY col1, col2), @@ -157,7 +178,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 6 6 9 10 10 11 +1 1 2 2 5 6 6 7 PREHOOK: query: select dense_rank(1) WITHIN GROUP (ORDER BY col1), dense_rank(2) WITHIN GROUP (ORDER BY col1), @@ -184,7 +205,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 2 3 3 4 4 5 +1 1 1 2 2 3 3 4 PREHOOK: query: select dense_rank(1, 1) WITHIN GROUP (ORDER BY col1, col2), dense_rank(2, 1) WITHIN GROUP (ORDER BY col1, col2), @@ -211,7 +232,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 3 3 3 6 6 7 +1 1 2 2 2 5 5 6 PREHOOK: query: select percent_rank(1) WITHIN GROUP (ORDER BY col1), percent_rank(2) WITHIN GROUP (ORDER BY col1), @@ -238,7 +259,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.23529411764705882 0.23529411764705882 0.23529411764705882 0.29411764705882354 0.29411764705882354 0.5294117647058824 0.5294117647058824 0.5882352941176471 +0.0 0.0 0.0 0.058823529411764705 0.058823529411764705 0.29411764705882354 0.29411764705882354 0.35294117647058826 PREHOOK: query: select cume_dist(7) WITHIN GROUP (ORDER BY col1), cume_dist(2) WITHIN GROUP (ORDER BY col1), @@ -265,7 +286,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.6111111111111112 0.2777777777777778 0.3333333333333333 0.3333333333333333 0.5555555555555556 0.5555555555555556 0.6111111111111112 0.7222222222222222 +0.3888888888888889 0.05555555555555555 0.1111111111111111 0.1111111111111111 0.3333333333333333 0.3333333333333333 0.3888888888888889 0.5 PREHOOK: query: select rank(1) WITHIN GROUP (ORDER BY col1), rank(2) WITHIN GROUP (ORDER BY col1), @@ -292,7 +313,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 5 6 6 10 10 11 +1 1 1 2 2 6 6 7 PREHOOK: query: select rank(1, 3) WITHIN GROUP (ORDER BY col1, col2), rank(2, 3) WITHIN GROUP (ORDER BY col1, col2), @@ -319,7 +340,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 6 6 9 10 10 11 +1 1 2 2 5 6 6 7 PREHOOK: query: select dense_rank(1) WITHIN GROUP (ORDER BY col1), dense_rank(2) WITHIN GROUP (ORDER BY col1), @@ -346,7 +367,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 2 3 3 4 4 5 +1 1 1 2 2 3 3 4 PREHOOK: query: select dense_rank(1, 1) WITHIN GROUP (ORDER BY col1, col2), dense_rank(2, 1) WITHIN GROUP (ORDER BY col1, col2), @@ -373,7 +394,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 3 3 3 6 6 7 +1 1 2 2 2 5 5 6 PREHOOK: query: select percent_rank(1) WITHIN GROUP (ORDER BY col1), percent_rank(2) WITHIN GROUP (ORDER BY col1), @@ -400,7 +421,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.23529411764705882 0.23529411764705882 0.23529411764705882 0.29411764705882354 0.29411764705882354 0.5294117647058824 0.5294117647058824 0.5882352941176471 +0.0 0.0 0.0 0.058823529411764705 0.058823529411764705 0.29411764705882354 0.29411764705882354 0.35294117647058826 PREHOOK: query: select cume_dist(7) WITHIN GROUP (ORDER BY col1), cume_dist(2) WITHIN GROUP (ORDER BY col1), @@ -427,7 +448,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.6111111111111112 0.2777777777777778 0.3333333333333333 0.3333333333333333 0.5555555555555556 0.5555555555555556 0.6111111111111112 0.7222222222222222 +0.3888888888888889 0.05555555555555555 0.1111111111111111 0.1111111111111111 0.3333333333333333 0.3333333333333333 0.3888888888888889 0.5 PREHOOK: query: select rank(1) WITHIN GROUP (ORDER BY col1), rank(2) WITHIN GROUP (ORDER BY col1), @@ -454,7 +475,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 5 6 6 10 10 11 +1 1 1 2 2 6 6 7 PREHOOK: query: select rank(1, 3) WITHIN GROUP (ORDER BY col1, col2), rank(2, 3) WITHIN GROUP (ORDER BY col1, col2), @@ -481,7 +502,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 6 6 9 10 10 11 +1 1 2 2 5 6 6 7 PREHOOK: query: select dense_rank(1) WITHIN GROUP (ORDER BY col1), dense_rank(2) WITHIN GROUP (ORDER BY col1), @@ -508,7 +529,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 2 3 3 4 4 5 +1 1 1 2 2 3 3 4 PREHOOK: query: select dense_rank(1, 1) WITHIN GROUP (ORDER BY col1, col2), dense_rank(2, 1) WITHIN GROUP (ORDER BY col1, col2), @@ -535,7 +556,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 3 3 3 6 6 7 +1 1 2 2 2 5 5 6 PREHOOK: query: select percent_rank(1) WITHIN GROUP (ORDER BY col1), percent_rank(2) WITHIN GROUP (ORDER BY col1), @@ -562,7 +583,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.23529411764705882 0.23529411764705882 0.23529411764705882 0.29411764705882354 0.29411764705882354 0.5294117647058824 0.5294117647058824 0.5882352941176471 +0.0 0.0 0.0 0.058823529411764705 0.058823529411764705 0.29411764705882354 0.29411764705882354 0.35294117647058826 PREHOOK: query: select cume_dist(7) WITHIN GROUP (ORDER BY col1), cume_dist(2) WITHIN GROUP (ORDER BY col1), @@ -589,7 +610,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.6111111111111112 0.2777777777777778 0.3333333333333333 0.3333333333333333 0.5555555555555556 0.5555555555555556 0.6111111111111112 0.7222222222222222 +0.3888888888888889 0.05555555555555555 0.1111111111111111 0.1111111111111111 0.3333333333333333 0.3333333333333333 0.3888888888888889 0.5 PREHOOK: query: select rank(1) WITHIN GROUP (ORDER BY col1), rank(2) WITHIN GROUP (ORDER BY col1), @@ -616,7 +637,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 5 6 6 10 10 11 +1 1 1 2 2 6 6 7 PREHOOK: query: select rank(1, 3) WITHIN GROUP (ORDER BY col1, col2), rank(2, 3) WITHIN GROUP (ORDER BY col1, col2), @@ -643,7 +664,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -5 5 6 6 9 10 10 11 +1 1 2 2 5 6 6 7 PREHOOK: query: select dense_rank(1) WITHIN GROUP (ORDER BY col1), dense_rank(2) WITHIN GROUP (ORDER BY col1), @@ -670,7 +691,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 2 3 3 4 4 5 +1 1 1 2 2 3 3 4 PREHOOK: query: select dense_rank(1, 1) WITHIN GROUP (ORDER BY col1, col2), dense_rank(2, 1) WITHIN GROUP (ORDER BY col1, col2), @@ -697,7 +718,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -2 2 3 3 3 6 6 7 +1 1 2 2 2 5 5 6 PREHOOK: query: select percent_rank(1) WITHIN GROUP (ORDER BY col1), percent_rank(2) WITHIN GROUP (ORDER BY col1), @@ -724,7 +745,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.23529411764705882 0.23529411764705882 0.23529411764705882 0.29411764705882354 0.29411764705882354 0.5294117647058824 0.5294117647058824 0.5882352941176471 +0.0 0.0 0.0 0.058823529411764705 0.058823529411764705 0.29411764705882354 0.29411764705882354 0.35294117647058826 PREHOOK: query: select cume_dist(7) WITHIN GROUP (ORDER BY col1), cume_dist(2) WITHIN GROUP (ORDER BY col1), @@ -751,7 +772,7 @@ from t_test POSTHOOK: type: QUERY POSTHOOK: Input: default@t_test #### A masked pattern was here #### -0.6111111111111112 0.2777777777777778 0.3333333333333333 0.3333333333333333 0.5555555555555556 0.5555555555555556 0.6111111111111112 0.7222222222222222 +0.3888888888888889 0.05555555555555555 0.1111111111111111 0.1111111111111111 0.3333333333333333 0.3333333333333333 0.3888888888888889 0.5 PREHOOK: query: DROP TABLE t_test PREHOOK: type: DROPTABLE PREHOOK: Input: default@t_test diff --git serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java index 1f3bbaf87d..f970857e83 100644 --- serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java +++ serde/src/java/org/apache/hadoop/hive/serde2/objectinspector/ObjectInspectorUtils.java @@ -18,16 +18,12 @@ package org.apache.hadoop.hive.serde2.objectinspector; -import static java.util.Comparator.nullsFirst; -import static java.util.Comparator.nullsLast; - import java.lang.reflect.Field; import java.lang.reflect.Modifier; import java.lang.reflect.Type; import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Arrays; -import java.util.Comparator; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; @@ -948,40 +944,37 @@ public static int compare(Object[] o1, ObjectInspector[] oi1, Object[] o2, return 0; } - public static List> getComparator( - ObjectInspector[] oi1, ObjectInspector[] oi2, - boolean[] columnSortOrderIsDesc, NullValueOption[] nullSortOrder) { - assert (oi1.length == oi2.length); - assert (columnSortOrderIsDesc.length == oi1.length); - assert (nullSortOrder.length == oi1.length); - - List> comparators = new ArrayList<>(oi1.length); - for (int i = 0; i < oi1.length; i++) { - final int keyIndex = i; - - Comparator comparator = (o1, o2) -> compare( - o1, oi1[keyIndex], o2, oi2[keyIndex]); + public static int compare(Object[] o1, ObjectInspector[] oi1, Object[] o2, + ObjectInspector[] oi2, boolean[] columnSortOrderIsDesc, NullValueOption[] nullSortOrder) { + assert (o1.length == oi1.length); + assert (o2.length == oi2.length); + assert (o1.length == o2.length); + assert (o1.length == columnSortOrderIsDesc.length); + assert (o1.length == nullSortOrder.length); - if (columnSortOrderIsDesc[i]) { - comparator = comparator.reversed(); + for (int i = 0; i < o1.length; i++) { + int r = compareNull(o1[i], o2[i]); + if (r != 0) { + return nullSortOrder[i] == NullValueOption.MINVALUE ? -r : r; } - if (nullSortOrder[i] == NullValueOption.MAXVALUE) { - comparators.add(nullsFirst(comparator)); + if (columnSortOrderIsDesc[i]) { + r = compare(o2[i], oi2[i], o1[i], oi1[i]); } else { - comparators.add(nullsLast(comparator)); + r = compare(o1[i], oi1[i], o2[i], oi2[i]); + } + if (r != 0) { + return r; } } - - return comparators; + return 0; } - public static int compare(List> comparatorList, T[] o1, T[] o2) { - for (int i = 0; i < comparatorList.size(); ++i) { - int c = comparatorList.get(i).compare(o1[i], o2[i]); - if (c != 0) { - return c; - } + public static int compareNull(Object o1, Object o2) { + if (o1 == null) { + return o2 == null ? 0 : 1; + } else if (o2 == null) { + return -1; } return 0; } diff --git serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorUtilsCompare.java serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorUtilsCompare.java new file mode 100644 index 0000000000..9a0e974813 --- /dev/null +++ serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorUtilsCompare.java @@ -0,0 +1,198 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.serde2.objectinspector; + +import static org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils.NullValueOption; +import static org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils.NullValueOption.MAXVALUE; +import static org.apache.hadoop.hive.serde2.objectinspector.ObjectInspectorUtils.NullValueOption.MINVALUE; +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertThat; + +import org.apache.hadoop.hive.serde2.objectinspector.primitive.PrimitiveObjectInspectorFactory; +import org.junit.Test; + +/** + * TestObjectInspectorUtils.compare method. + * int compare(Object[] o1, ObjectInspector[] oi1, Object[] o2, + * ObjectInspector[] oi2, boolean[] columnSortOrderIsDesc, NullValueOption[] nullSortOrder) + */ +public class TestObjectInspectorUtilsCompare { + + private Integer[] objects1; + private ObjectInspector[] oi1; + private Integer[] objects2; + private ObjectInspector[] oi2; + boolean[] orderDesc; + NullValueOption[] nullValueOption; + + void o1(Integer... objects1) { + this.objects1 = objects1; + this.oi1 = createOI(objects1.length); + } + + private ObjectInspector[] createOI(int length) { + ObjectInspector[] objectInspectors = new ObjectInspector[length]; + for (int i = 0; i < length; ++i) { + objectInspectors[i] = PrimitiveObjectInspectorFactory.javaIntObjectInspector; + } + return objectInspectors; + } + + void o2(Integer... objects2) { + this.objects2 = objects2; + this.oi2 = createOI(objects2.length); + } + + void order(boolean... isDesc) { + this.orderDesc = isDesc; + } + + void nullOrder(NullValueOption... nullValueOption) { + this.nullValueOption = nullValueOption; + } + + @Test + public void testWhenTheFirstValueIsSmallerMinusOneIsReturned() { + o1(1); + o2(2); + order(false); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(-1)); + } + + @Test + public void testWhenTheFirstValueIsLargerOneIsReturned() { + o1(2); + o2(1); + order(false); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(1)); + } + + @Test + public void testWhenTheFirstValueIsSmallerAndDescendingOrderOneIsReturned() { + o1(1); + o2(2); + order(true); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(1)); + } + + @Test + public void testWhenValuesAreEqual0IsReturned() { + o1(2, 2, 2); + o2(2, 2, 2); + order(false, true, false); + nullOrder(MAXVALUE, MINVALUE, MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(0)); + } + + @Test + public void testWhenTheFirstPairIsEqualsButTheSecondNot() { + o1(2, 1, 2); + o2(2, 2, 2); + order(false, false, false); + nullOrder(MAXVALUE, MAXVALUE, MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(-1)); + } + + @Test + public void testWhenTheFirstValueIsNullAndNullsFirstMinusOneIsReturned() { + o1(new Integer[]{null}); + o2(2); + order(false); + nullOrder(MINVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(-1)); + } + + @Test + public void testWhenTheFirstValueIsNullAndNullsLastOneIsReturned() { + o1(new Integer[]{null}); + o2(2); + order(false); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(1)); + } + + @Test + public void testWhenTheFirstValueIsNullAndNullsFirstAndDescendingOrderMinusOneIsReturned() { + o1(new Integer[]{null}); + o2(2); + order(true); + nullOrder(MINVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(-1)); + } + + @Test + public void testWhenTheFirstValueIsNullAndNullsLastAndDescendingOrderOneIsReturned() { + o1(new Integer[]{null}); + o2(2); + order(true); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(1)); + } + + @Test + public void testWhenTheSecondValueIsNullAndNullsFirstOneIsReturned() { + o1(2); + o2(new Integer[]{null}); + order(false); + nullOrder(MINVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(1)); + } + + @Test + public void testWhenTheSecondValueIsNullAndNullsLastMinusOneIsReturned() { + o1(2); + o2(new Integer[]{null}); + order(false); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(-1)); + } + + @Test + public void testWhenTheSecondValueIsNullAndNullsFirstAndDescendingOrderOneIsReturned() { + o1(2); + o2(new Integer[]{null}); + order(true); + nullOrder(MINVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(1)); + } + + @Test + public void testWhenTheSecondValueIsNullAndNullsLastAndDescendingOrderMinusOneIsReturned() { + o1(2); + o2(new Integer[]{null}); + order(true); + nullOrder(MAXVALUE); + + assertThat(ObjectInspectorUtils.compare(objects1, oi1, objects2, oi2, orderDesc, nullValueOption), is(-1)); + } +} diff --git serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorUtilsCompareNull.java serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorUtilsCompareNull.java new file mode 100644 index 0000000000..c44beecd88 --- /dev/null +++ serde/src/test/org/apache/hadoop/hive/serde2/objectinspector/TestObjectInspectorUtilsCompareNull.java @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hive.serde2.objectinspector; + +import static org.hamcrest.core.Is.is; +import static org.junit.Assert.assertThat; + +import org.junit.Test; + +/** + * TestObjectInspectorUtils.compareNull method. + */ +public class TestObjectInspectorUtilsCompareNull { + + @Test + public void testcompareNullReturns0WhenBothInputsAreNull() { + assertThat(ObjectInspectorUtils.compareNull(null, null), is(0)); + } + + @Test + public void testcompareNullReturns1WhenFirstInputIsNull() { + assertThat(ObjectInspectorUtils.compareNull(null, 1), is(1)); + } + + @Test + public void testcompareNullReturnsMinus1WhenSecondInputIsNull() { + assertThat(ObjectInspectorUtils.compareNull(1, null), is(-1)); + } + + @Test + public void testcompareNullReturns0WhenNoneOfTheInputsAreNull() { + assertThat(ObjectInspectorUtils.compareNull(1, 1), is(0)); + } +}