Skip to content

Commit 54ff0a1

Browse files
infvgyingsu00
authored andcommitted
Fix ARRAYS_OVERLAP function bug
In some arrays containing arrays that contain null values, arrays_overlap was not properly comparing values. Switch to array_overlap to set based implementation. Resolves: #23730
1 parent 842a3eb commit 54ff0a1

2 files changed

Lines changed: 24 additions & 4 deletions

File tree

presto-main-base/src/main/java/com/facebook/presto/operator/scalar/ArraysOverlapFunction.java

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,19 @@
1818
import com.facebook.presto.common.type.Type;
1919
import com.facebook.presto.operator.aggregation.TypedSet;
2020
import com.facebook.presto.spi.function.Description;
21+
import com.facebook.presto.spi.function.OperatorDependency;
2122
import com.facebook.presto.spi.function.ScalarFunction;
2223
import com.facebook.presto.spi.function.SqlNullable;
2324
import com.facebook.presto.spi.function.SqlType;
2425
import com.facebook.presto.spi.function.TypeParameter;
2526

27+
import java.lang.invoke.MethodHandle;
28+
29+
import static com.facebook.presto.common.function.OperatorType.IS_DISTINCT_FROM;
30+
import static com.facebook.presto.common.type.TypeUtils.readNativeValue;
31+
import static com.facebook.presto.util.Failures.internalError;
32+
import static com.google.common.base.Defaults.defaultValue;
33+
2634
@ScalarFunction("arrays_overlap")
2735
@Description("Returns true if arrays have common elements")
2836
public final class ArraysOverlapFunction
@@ -36,6 +44,7 @@ private ArraysOverlapFunction() {}
3644
@SqlNullable
3745
public static Boolean arraysOverlap(
3846
@TypeParameter("T") Type elementType,
47+
@OperatorDependency(operator = IS_DISTINCT_FROM, argumentTypes = {"T", "T"}) MethodHandle elementIsDistinctFrom,
3948
@SqlType("array(T)") Block leftArray,
4049
@SqlType("array(T)") Block rightArray)
4150
{
@@ -59,8 +68,17 @@ public static Boolean arraysOverlap(
5968
hasNull = true;
6069
continue;
6170
}
62-
if (elementType.equalTo(leftArray, i, rightArray, j)) {
63-
return true;
71+
try {
72+
boolean firstValueNull = leftArray.isNull(i);
73+
Object firstValue = firstValueNull ? defaultValue(elementType.getJavaType()) : readNativeValue(elementType, leftArray, i);
74+
boolean secondValueNull = rightArray.isNull(j);
75+
Object secondValue = secondValueNull ? defaultValue(elementType.getJavaType()) : readNativeValue(elementType, rightArray, j);
76+
if (!(boolean) elementIsDistinctFrom.invoke(firstValue, firstValueNull, secondValue, secondValueNull)) {
77+
return true;
78+
}
79+
}
80+
catch (Throwable t) {
81+
throw internalError(t);
6482
}
6583
}
6684
}

presto-main-base/src/test/java/com/facebook/presto/type/TestArrayOperators.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1295,6 +1295,8 @@ public void testArraysOverlap()
12951295
assertFunction("ARRAYS_OVERLAP(ARRAY [NULL, 3], ARRAY [2, 1])", BooleanType.BOOLEAN, null);
12961296
assertFunction("ARRAYS_OVERLAP(ARRAY [3, NULL], ARRAY [2, 1])", BooleanType.BOOLEAN, null);
12971297
assertFunction("ARRAYS_OVERLAP(ARRAY [3, NULL], ARRAY [2, 1, NULL])", BooleanType.BOOLEAN, null);
1298+
assertFunction("ARRAYS_OVERLAP(ARRAY [ARRAY [1, 2], ARRAY [1, NULL]], ARRAY [ARRAY [1, 2]])", BooleanType.BOOLEAN, true);
1299+
assertFunction("ARRAYS_OVERLAP(ARRAY [ARRAY [1, NULL], ARRAY [1, 2]], ARRAY [ARRAY [1, 2]])", BooleanType.BOOLEAN, true);
12981300

12991301
assertFunction("ARRAYS_OVERLAP(ARRAY [CAST(1 AS BIGINT), 2], ARRAY [NULL, CAST(2 AS BIGINT)])", BooleanType.BOOLEAN, true);
13001302
assertFunction("ARRAYS_OVERLAP(ARRAY [CAST(1 AS BIGINT), 2], ARRAY [CAST(2 AS BIGINT), NULL])", BooleanType.BOOLEAN, true);
@@ -1405,8 +1407,8 @@ public void testComparison()
14051407
assertFunction("ARRAY [1, 2, null] != ARRAY [1, 2, null]", BOOLEAN, null);
14061408
assertFunction("ARRAY [1, 2, null] != ARRAY [1, null]", BOOLEAN, true);
14071409
assertFunction("ARRAY [1, 3, null] != ARRAY [1, 2, null]", BOOLEAN, true);
1408-
assertFunction("ARRAY [ARRAY[1], ARRAY[null], ARRAY[2]] != ARRAY [ARRAY[1], ARRAY[2], ARRAY[3]]", BOOLEAN, true);
1409-
assertFunction("ARRAY [ARRAY[1], ARRAY[null], ARRAY[3]] != ARRAY [ARRAY[1], ARRAY[2], ARRAY[3]]", BOOLEAN, null);
1410+
assertFunction("ARRAY [ARRAY [1], ARRAY [null], ARRAY [2]] != ARRAY [ARRAY [1], ARRAY [2], ARRAY [3]]", BOOLEAN, true);
1411+
assertFunction("ARRAY [ARRAY [1], ARRAY [null], ARRAY [3]] != ARRAY [ARRAY [1], ARRAY [2], ARRAY [3]]", BOOLEAN, null);
14101412

14111413
assertFunction("ARRAY [10, 20, 30] < ARRAY [10, 20, 40, 50]", BOOLEAN, true);
14121414
assertFunction("ARRAY [10, 20, 30] >= ARRAY [10, 20, 40, 50]", BOOLEAN, false);

0 commit comments

Comments
 (0)