Fix null-handling bug in ArrayDistinctFunction

trinodb · Aug 27, 2016 · 9adc687 · 9adc687
1 parent 6a26d50
commit 9adc687
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 3 deletions.
diff --git a/presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayDistinctFunction.java b/presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayDistinctFunction.java
@@ -73,11 +73,14 @@ public static Block bigintDistinct(@SqlType("array(bigint)") Block array)
         LongSet set = new LongOpenHashSet(array.getPositionCount());
         BlockBuilder distinctElementBlockBuilder = BIGINT.createBlockBuilder(new BlockBuilderStatus(), array.getPositionCount());
         for (int i = 0; i < array.getPositionCount(); i++) {
-            if (!containsNull && array.isNull(i)) {
-                containsNull = true;
-                distinctElementBlockBuilder.appendNull();
+            if (array.isNull(i)) {
+                if (!containsNull) {
+                    containsNull = true;
+                    distinctElementBlockBuilder.appendNull();
+                }
                 continue;
             }
+
             long value = BIGINT.getLong(array, i);
             if (set.add(value)) {
                 BIGINT.writeLong(distinctElementBlockBuilder, value);

diff --git a/presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java b/presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java
@@ -538,6 +538,9 @@ public void testDistinct()
         assertFunction("ARRAY_DISTINCT(ARRAY [NULL])", new ArrayType(UNKNOWN), asList((Object) null));
         assertFunction("ARRAY_DISTINCT(ARRAY [NULL, NULL])", new ArrayType(UNKNOWN), asList((Object) null));
         assertFunction("ARRAY_DISTINCT(ARRAY [NULL, NULL, NULL])", new ArrayType(UNKNOWN), asList((Object) null));
+
+        // Test for BIGINT-optimized implementation
+        assertFunction("ARRAY_DISTINCT(ARRAY [CAST(5 AS BIGINT), NULL, CAST(12 AS BIGINT), NULL])", new ArrayType(BIGINT), asList(5L, null, 12L));
     }
 
     @Test