Skip to content

[CALCITE-6522] MAP_KEYS and MAP_VALUES function should throw if a key value is null #3909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 21, 2024
Merged

[CALCITE-6522] MAP_KEYS and MAP_VALUES function should throw if a key value is null #3909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -933,6 +933,15 @@ ExInst<CalciteException> illegalArgumentForTableFunctionCall(String a0,
@BaseMessage("Illegal arguments for 'FORMAT_NUMBER' function: negative decimal value not allowed")
ExInst<CalciteException> illegalNegativeDecimalValue();

@BaseMessage("Illegal arguments for 'MAP_ENTRIES' function: using a map with a null key is not allowed")
ExInst<CalciteException> illegalMapEntriesWithNullKey();

@BaseMessage("Illegal arguments for 'MAP_KEYS' function: using a map with a null key is not allowed")
ExInst<CalciteException> illegalMapKeysWithNullKey();

@BaseMessage("Illegal arguments for 'MAP_VALUES' function: using a map with a null key is not allowed")
ExInst<CalciteException> illegalMapValuesWithNullKey();

@BaseMessage("Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function")
ExInst<CalciteException> illegalArgumentsInMapFromArraysFunc(int arg0, int arg1);

Expand Down
11 changes: 9 additions & 2 deletions core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5907,7 +5907,7 @@ public static List mapEntries(Map<Object, Object> map) {
final List result = new ArrayList(map.size());
for (Map.Entry<Object, Object> entry : map.entrySet()) {
if (entry.getKey() == null) {
throw new IllegalArgumentException("Cannot use null as map key");
throw RESOURCE.illegalMapEntriesWithNullKey().ex();
}
result.add(Arrays.asList(entry.getKey(), entry.getValue()));
}
Expand All @@ -5916,11 +5916,18 @@ public static List mapEntries(Map<Object, Object> map) {

/** Support the MAP_KEYS function. */
public static List mapKeys(Map map) {
return new ArrayList<>(map.keySet());
try {
return ImmutableList.copyOf(map.keySet());
} catch (NullPointerException e) {
throw RESOURCE.illegalMapKeysWithNullKey().ex();
}
}

/** Support the MAP_VALUES function. */
public static List mapValues(Map map) {
if (map.containsKey(null)) {
throw RESOURCE.illegalMapValuesWithNullKey().ex();
}
return new ArrayList<>(map.values());
}

Expand Down
5 changes: 4 additions & 1 deletion core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,11 @@ FunctionNotFound=Function ''{0}'' not found
DialectDoesNotSupportFeature=Dialect does not support feature: ''{0}''
IllegalNegativePadLength=Second argument for LPAD/RPAD must not be negative
IllegalEmptyPadPattern=Third argument (pad pattern) for LPAD/RPAD must not be empty
IllegalNegativeSubstringLength=Substring error: negative substring length not allowed
IllegalNegativeDecimalValue=Illegal arguments for 'FORMAT_NUMBER' function: negative decimal value not allowed
IllegalNegativeSubstringLength=Substring error: negative substring length not allowed
IllegalMapEntriesWithNullKey=Illegal arguments for 'MAP_ENTRIES' function: using a map with a null key is not allowed
IllegalMapKeysWithNullKey=Illegal arguments for 'MAP_KEYS' function: using a map with a null key is not allowed
IllegalMapValuesWithNullKey=Illegal arguments for 'MAP_VALUES' function: using a map with a null key is not allowed
IllegalArgumentsInMapFromArraysFunc=Illegal arguments: The length of the keys array {0,number,#} is not equal to the length of the values array {1,number,#} in MAP_FROM_ARRAYS function
TrimError=Trim error: trim character must be exactly 1 character
InvalidTypesForArithmetic=Invalid types for arithmetic: {0} {1} {2}
Expand Down
52 changes: 36 additions & 16 deletions testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8513,13 +8513,17 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,

// 3. check key is not allowed to be null
f.checkFails("map_entries(map[cast(1 as decimal), 1, null, 2])",
"Cannot use null as map key", true);
"Illegal arguments for MAP_ENTRIES function: using a map with a null key is not allowed",
true);
f.checkFails("map_entries(map[1, cast(1 as bigint), null, 2])",
"Cannot use null as map key", true);
"Illegal arguments for MAP_ENTRIES function: using a map with a null key is not allowed",
true);
f.checkFails("map_entries(map[1, cast(1 as decimal), null, 2])",
"Cannot use null as map key", true);
"Illegal arguments for MAP_ENTRIES function: using a map with a null key is not allowed",
true);
f.checkFails("map_entries(map['foo', 1, null, 2])",
"Cannot use null as map key", true);
"Illegal arguments for MAP_ENTRIES function: using a map with a null key is not allowed",
true);
}

/** Tests {@code MAP_KEYS} function from Spark. */
Expand All @@ -8533,23 +8537,15 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
final SqlOperatorFixture f = f0.withLibrary(SqlLibrary.SPARK);
f.checkScalar("map_keys(map['foo', 1, 'bar', 2])", "[foo, bar]",
"CHAR(3) NOT NULL ARRAY NOT NULL");
f.checkScalar("map_keys(map['foo', 1, null, 2])", "[foo, null]",
"CHAR(3) ARRAY NOT NULL");

// elements cast
// key cast
f.checkScalar("map_keys(map[cast(1 as tinyint), 1, 2, 2])", "[1, 2]",
"INTEGER NOT NULL ARRAY NOT NULL");
f.checkScalar("map_keys(map[cast(1 as bigint), 1, null, 2])", "[1, null]",
"BIGINT ARRAY NOT NULL");
f.checkScalar("map_keys(map[cast(1 as decimal), 1, null, 2])", "[1, null]",
"DECIMAL(19, 0) ARRAY NOT NULL");

// value cast
f.checkScalar("map_keys(map[1, cast(1 as tinyint), 2, 2])", "[1, 2]",
"INTEGER NOT NULL ARRAY NOT NULL");
f.checkScalar("map_keys(map[1, cast(1 as bigint), null, 2])", "[1, null]",
"INTEGER ARRAY NOT NULL");
f.checkScalar("map_keys(map[1, cast(1 as decimal), null, 2])", "[1, null]",
"INTEGER ARRAY NOT NULL");

// 2. check with map function, map(k, v ...)
final SqlOperatorFixture f1 = fixture()
Expand All @@ -8559,8 +8555,19 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
"UNKNOWN NOT NULL ARRAY NOT NULL");
f1.checkScalar("map_keys(map('foo', 1, 'bar', 2))", "[foo, bar]",
"CHAR(3) NOT NULL ARRAY NOT NULL");
f1.checkScalar("map_keys(map('foo', 1, null, 2))", "[foo, null]",
"CHAR(3) ARRAY NOT NULL");

f.checkFails("map_keys(map['foo', 1, null, 2])",
"Illegal arguments for MAP_KEYS function: using a map with a null key is not allowed",
true);
f.checkFails("map_keys(map[1, cast(1 as decimal), null, 2])",
"Illegal arguments for MAP_KEYS function: using a map with a null key is not allowed",
true);
f.checkFails("map_keys(map[1, cast(1 as bigint), null, 2])",
"Illegal arguments for MAP_KEYS function: using a map with a null key is not allowed",
true);
f.checkFails("map_keys(map[cast(1 as decimal), 1, null, 2])",
"Illegal arguments for MAP_KEYS function: using a map with a null key is not allowed",
true);
}

/** Tests {@code MAP_VALUES} function from Spark. */
Expand All @@ -8587,6 +8594,19 @@ void checkArrayReverseFunc(SqlOperatorFixture f0, SqlFunction function,
"INTEGER NOT NULL ARRAY NOT NULL");
f1.checkScalar("map_values(map('foo', 1, 'bar', cast(null as integer)))", "[1, null]",
"INTEGER ARRAY NOT NULL");

f.checkFails("map_values(map['foo', 1, null, 2])",
"Illegal arguments for MAP_VALUES function: using a map with a null key is not allowed",
true);
f.checkFails("map_values(map[1, cast(1 as decimal), null, 2])",
"Illegal arguments for MAP_VALUES function: using a map with a null key is not allowed",
true);
f.checkFails("map_values(map[1, cast(1 as bigint), null, 2])",
"Illegal arguments for MAP_VALUES function: using a map with a null key is not allowed",
true);
f.checkFails("map_values(map[cast(1 as decimal), 1, null, 2])",
"Illegal arguments for MAP_VALUES function: using a map with a null key is not allowed",
true);
}

/** Test case for
Expand Down
Loading