Skip to content

Commit

Permalink
Update the error type for ARRAY_NULL_ELEMENT_MSG to INVALID_FUNCTION_…
Browse files Browse the repository at this point in the history 
…ARGUMENT in order to suppress the error for try(map_keys_by_top_n_values ...) when the value is an array with null.
  • Loading branch information
@duxiao1212
duxiao1212 committed Feb 18, 2025
1 parent 1f61957 commit daba822
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 9 deletions.
4 changes: 2 additions & 2 deletions presto-common/src/main/java/com/facebook/presto/common/type/TypeUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/
package com.facebook.presto.common.type;

import com.facebook.presto.common.NotSupportedException;
import com.facebook.presto.common.InvalidFunctionArgumentException;
import com.facebook.presto.common.block.Block;
import com.facebook.presto.common.block.BlockBuilder;
import io.airlift.slice.Slice;
Expand Down Expand Up @@ -189,7 +189,7 @@ public static boolean isFloatingPointNaN(Type type, Object value)
static void checkElementNotNull(boolean isNull, String errorMsg)
{
if (isNull) {
throw new NotSupportedException(errorMsg);
throw new InvalidFunctionArgumentException(errorMsg);
}
}

Expand Down
3 changes: 2 additions & 1 deletion presto-main/src/main/java/com/facebook/presto/type/TypeUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import java.util.List;

import static com.facebook.presto.common.type.BigintType.BIGINT;
import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Throwables.throwIfUnchecked;
Expand Down Expand Up @@ -174,7 +175,7 @@ public static Page getHashPage(Page page, List<? extends Type> types, List<Integ
public static void checkElementNotNull(boolean isNull, String errorMsg)
{
if (isNull) {
throw new PrestoException(NOT_SUPPORTED, errorMsg);
throw new PrestoException(INVALID_FUNCTION_ARGUMENT, errorMsg);
}
}
}
10 changes: 5 additions & 5 deletions presto-main/src/test/java/com/facebook/presto/type/TestArrayOperators.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -710,13 +710,13 @@ public void testArrayMinWithNullsInBothArraysNotComparedSecondIsMin()
@Test
public void testArrayMinWithNullInFirstArrayIsCompared()
{
assertInvalidFunction("ARRAY_MIN(ARRAY [ARRAY[1, NULL], ARRAY[1, 2]])", NOT_SUPPORTED);
assertInvalidFunction("ARRAY_MIN(ARRAY [ARRAY[1, NULL], ARRAY[1, 2]])", INVALID_FUNCTION_ARGUMENT);
}

@Test
public void testArrayMinWithNullInSecondArrayIsCompared()
{
assertInvalidFunction("ARRAY_MIN(ARRAY [ARRAY[1, 2], ARRAY[1, NULL]])", NOT_SUPPORTED);
assertInvalidFunction("ARRAY_MIN(ARRAY [ARRAY[1, 2], ARRAY[1, NULL]])", INVALID_FUNCTION_ARGUMENT);
}

@Test
Expand Down Expand Up @@ -764,13 +764,13 @@ public void testArrayMaxWithNullsInBothArraysNotComparedFirstIsMax()
@Test
public void testArrayMaxWithNullInFirstArrayIsCompared()
{
assertInvalidFunction("ARRAY_MAX(ARRAY [ARRAY[1, NULL], ARRAY[1, 2]])", NOT_SUPPORTED);
assertInvalidFunction("ARRAY_MAX(ARRAY [ARRAY[1, NULL], ARRAY[1, 2]])", INVALID_FUNCTION_ARGUMENT);
}

@Test
public void testArrayMaxWithNullInSecondArrayIsCompared()
{
assertInvalidFunction("ARRAY_MAX(ARRAY [ARRAY[1, 2], ARRAY[1, NULL]])", NOT_SUPPORTED);
assertInvalidFunction("ARRAY_MAX(ARRAY [ARRAY[1, 2], ARRAY[1, NULL]])", INVALID_FUNCTION_ARGUMENT);
}

@Test
Expand Down Expand Up @@ -1145,7 +1145,7 @@ public void testSort()
assertInvalidFunction(
"ARRAY_SORT(ARRAY[ARRAY[1], ARRAY[null]])",
INVALID_FUNCTION_ARGUMENT,
"Array contains elements not supported for comparison");
"ARRAY comparison not supported for arrays with null elements");
assertInvalidFunction(
"ARRAY_SORT(ARRAY[ROW(1), ROW(null)])",
INVALID_FUNCTION_ARGUMENT,
Expand Down
2 changes: 1 addition & 1 deletion presto-main/src/test/java/com/facebook/presto/type/TestRowOperators.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -692,7 +692,7 @@ public void testRowComparison()
assertInvalidFunction("row(TRUE, ARRAY [1, 2], MAP(ARRAY[1, 3], ARRAY[2.0E0, 4.0E0])) > row(TRUE, ARRAY [1, 2], MAP(ARRAY[1, 3], ARRAY[2.0E0, 4.0E0]))",
SemanticErrorCode.TYPE_MISMATCH, "line 1:64: '>' cannot be applied to row(boolean,array(integer),map(integer,double)), row(boolean,array(integer),map(integer,double))");

assertInvalidFunction("row(1, CAST(NULL AS INTEGER)) < row(1, 2)", StandardErrorCode.NOT_SUPPORTED);
assertInvalidFunction("row(1, CAST(NULL AS INTEGER)) < row(1, 2)", StandardErrorCode.INVALID_FUNCTION_ARGUMENT);

assertComparisonCombination("row(1.0E0, ARRAY [1,2,3], row(2, 2.0E0))", "row(1.0E0, ARRAY [1,3,3], row(2, 2.0E0))");
assertComparisonCombination("row(TRUE, ARRAY [1])", "row(TRUE, ARRAY [1, 2])");
Expand Down
30 changes: 30 additions & 0 deletions presto-tests/src/main/java/com/facebook/presto/tests/AbstractTestQueries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3234,6 +3234,36 @@ public void testTry()

// test try with null
assertQuery("SELECT TRY(1 / x) FROM (SELECT NULL as x)", "SELECT NULL");

// Test try with map method and value parameter is optional and argument is an array with null,
// the error should be suppressed and just return null.
assertQuery("SELECT\n" +
" TRY(map_keys_by_top_n_values(c0, BIGINT '6455219767830808341'))\n" +
"FROM (\n" +
" VALUES\n" +
" MAP(\n" +
" ARRAY[1, 2], ARRAY[\n" +
" ARRAY[1, null],\n" +
" ARRAY[1, null]\n" +
" ]\n" +
" )\n" +
") t(c0)", "SELECT NULL");

assertQuery("SELECT\n" +
" TRY(map_keys_by_top_n_values(c0, BIGINT '6455219767830808341'))\n" +
"FROM (\n" +
" VALUES\n" +
" MAP(\n" +
" ARRAY[1, 2], ARRAY[\n" +
" ARRAY[null, null],\n" +
" ARRAY[1, 2]\n" +
" ]\n" +
" )\n" +
") t(c0)", "SELECT NULL");

// Test try with array method with an input array containing null values.
// the error should be suppressed and just return null.
assertQuery("SELECT TRY(ARRAY_MAX(ARRAY [ARRAY[1, NULL], ARRAY[1, 2]]))", "SELECT NULL");
}

@Test
Expand Down

0 comments on commit daba822

Please sign in to comment.