Skip to content

Commit cf13958

Browse files
PG1204Jefffrey
andauthored
fix: support type coercion for MAP literals with NULL values in VALUES lists (#23521)
## Which issue does this PR close? - Closes #23474. ## Rationale for this change A bare `MAP {'k1': NULL}` literal infers a `Null` value type, producing `Map("entries": Struct("key": Utf8, "value": Null))`. Placing it in a VALUES list alongside concretely-typed map rows failed with: Error during planning: Inconsistent data type across values list at row 1 column 0. This is because `type_union_resolution_coercion` (the pairwise worker behind `type_union_resolution`, which the VALUES builder uses to widen row types) had recursion arms for `Dictionary`, `RunEndEncoded`, `Struct`, and `List` (via `list_coercion`), but none for `Map`. Scalar NULL inference across a VALUES list (`VALUES (1), (NULL)`) already worked; the same inference just never reached into a map's value type. This gap was noted in #23406, where the tests had to use a `CAST(NULL AS BIGINT)` workaround. ## What changes are included in this PR? - A one-line fix in `datafusion/expr-common/src/type_coercion/binary.rs`: wire the existing `map_coercion` helper (already used by `comparison_coercion` and `type_union_coercion`) into `type_union_resolution_coercion`'s fallback chain. The recursion into the map's key/value types then applies the exact same rules as scalar VALUES coercion (`null_coercion` for NULL inference, `binary_numeric_coercion` for numeric widening), symmetrically in either row order. - A unit test (`test_type_union_resolution_map`) covering Null + Int64 value types in both orders, Int64 + Null + Float64 widening, and Map vs. Struct still refusing to unify. - A new "map NULL value coercion in VALUES" section in `datafusion/sqllogictest/test_files/map.slt` covering: the issue reproducer, the reversed row order, all-NULL values (resulting type `Map("entries": Struct("key": Utf8, "value": Null))`, verified via `arrow_typeof`), multi-key rows with one NULL value, NULL scattered across three rows, numeric widening to Float64, two-level nested maps, NULL round-tripping as NULL through `SELECT`, and `INSERT INTO ... VALUES`. - Incompatible concrete value types still error: `(MAP {'k': 1}), (MAP {'k': 'hello'})` follows the scalar VALUES rule (coerce to the numeric type, then fail with `Cast error: Cannot cast string 'hello' to value of Int64 type`, which is the same behavior as `VALUES (1), ('hello')`). - A note above the #23406 tests documenting that the `CAST(NULL AS BIGINT)` workaround is no longer required (those tests are left unchanged). ## Are these changes tested? Yes. New unit test in `datafusion-expr-common` and new sqllogictests in `map.slt` as described above. The full sqllogictest suite (492 files) passes locally with zero regressions, as do `cargo test -p datafusion-expr -p datafusion-expr-common`, `cargo fmt`, and `cargo clippy --workspace --all-targets -- -D warnings`. ## Are there any user-facing changes? `VALUES` lists (and other contexts that use `type_union_resolution`, such as `CASE`/`COALESCE`/array literals) now unify Map types whose key/value types are coercible, including bare NULL map values. No API changes; previously failing queries now succeed, and incompatible-type errors are preserved. --------- Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
1 parent 0099876 commit cf13958

3 files changed

Lines changed: 200 additions & 0 deletions

File tree

datafusion/expr-common/src/type_coercion/binary.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -768,6 +768,7 @@ fn type_union_resolution_coercion(
768768
}
769769
_ => binary_numeric_coercion(lhs_type, rhs_type)
770770
.or_else(|| list_coercion(lhs_type, rhs_type, type_union_resolution_coercion))
771+
.or_else(|| map_coercion(lhs_type, rhs_type, type_union_resolution_coercion))
771772
.or_else(|| temporal_coercion_nonstrict_timezone(lhs_type, rhs_type))
772773
.or_else(|| string_coercion(lhs_type, rhs_type))
773774
.or_else(|| null_coercion(lhs_type, rhs_type))

datafusion/expr-common/src/type_coercion/binary/tests/comparison.rs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -908,6 +908,60 @@ fn test_type_union_coercion_prefers_finer_timestamp_unit() {
908908
);
909909
}
910910

911+
/// Tests that `type_union_resolution` unifies Map types by recursing into the
912+
/// key/value types, so a Map whose value type is Null (e.g. `MAP {'k': NULL}`)
913+
/// unifies with a concretely-typed Map in a VALUES list.
914+
/// See <https://github.com/apache/datafusion/issues/23474>.
915+
#[test]
916+
fn test_type_union_resolution_map() {
917+
fn map_type(value_type: DataType) -> DataType {
918+
DataType::Map(
919+
Arc::new(Field::new(
920+
"entries",
921+
DataType::Struct(Fields::from(vec![
922+
Field::new("key", DataType::Utf8, false),
923+
Field::new("value", value_type, true),
924+
])),
925+
false,
926+
)),
927+
false,
928+
)
929+
}
930+
931+
// Null value type unifies with a concrete value type, in both orders
932+
assert_eq!(
933+
type_union_resolution(&[map_type(DataType::Int64), map_type(DataType::Null)]),
934+
Some(map_type(DataType::Int64))
935+
);
936+
assert_eq!(
937+
type_union_resolution(&[map_type(DataType::Null), map_type(DataType::Int64)]),
938+
Some(map_type(DataType::Int64))
939+
);
940+
941+
// Numeric value types widen following the scalar rules
942+
assert_eq!(
943+
type_union_resolution(&[
944+
map_type(DataType::Int64),
945+
map_type(DataType::Null),
946+
map_type(DataType::Float64),
947+
]),
948+
Some(map_type(DataType::Float64))
949+
);
950+
951+
// Map cannot unify with a non-Map composite type
952+
assert_eq!(
953+
type_union_resolution(&[
954+
map_type(DataType::Int64),
955+
DataType::Struct(Fields::from(vec![Field::new(
956+
"key",
957+
DataType::Utf8,
958+
false
959+
)])),
960+
]),
961+
None
962+
);
963+
}
964+
911965
/// Tests that comparison operators coerce to numeric when comparing
912966
/// numeric and string types.
913967
#[test]

datafusion/sqllogictest/test_files/map.slt

Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,10 @@ SELECT map([column1, column1 * 10], ['x','y']) FROM (VALUES (1), (2), (3)) t;
920920
# tests for DISTINCT / GROUP BY / aggregation on map columns
921921
# https://github.com/apache/datafusion/issues/15428
922922

923+
# NOTE: the CAST(NULL AS BIGINT) in the VALUES list below predates the fix for
924+
# https://github.com/apache/datafusion/issues/23474 and is no longer required.
925+
# It is kept as-is to document the historical workaround; the un-cast form is
926+
# exercised in the "map NULL value coercion in VALUES" section further below.
923927
statement ok
924928
CREATE TABLE map_distinct_table AS VALUES
925929
(MAP {'k1': 1, 'k2': 2}, 'a', 1),
@@ -1048,3 +1052,144 @@ PUT 25
10481052

10491053
statement ok
10501054
DROP TABLE map_data;
1055+
1056+
# map NULL value coercion in VALUES
1057+
# https://github.com/apache/datafusion/issues/23474
1058+
# A bare NULL map value used to fail type unification across a VALUES list
1059+
# ("Inconsistent data type across values list") and required an explicit
1060+
# CAST(NULL AS <type>). The map value type now unifies with concrete value
1061+
# types following the same rules as scalar VALUES coercion.
1062+
1063+
# concrete-typed row first, NULL-valued row second (the issue reproducer)
1064+
statement ok
1065+
CREATE TABLE map_null_concrete_first AS VALUES
1066+
(MAP {'k1': 1, 'k2': 2}),
1067+
(MAP {'k1': NULL});
1068+
1069+
# NULL must round-trip as NULL after coercion, not a default value
1070+
query ? rowsort
1071+
SELECT * FROM map_null_concrete_first;
1072+
----
1073+
{k1: 1, k2: 2}
1074+
{k1: NULL}
1075+
1076+
# the NULL value type is coerced to the concrete value type (Int64)
1077+
query T
1078+
SELECT arrow_typeof(column1) FROM map_null_concrete_first LIMIT 1;
1079+
----
1080+
Map("entries": non-null Struct("key": non-null Utf8, "value": Int64), unsorted)
1081+
1082+
statement ok
1083+
DROP TABLE map_null_concrete_first;
1084+
1085+
# NULL-valued row first, concrete-typed row second (coercion is symmetric)
1086+
statement ok
1087+
CREATE TABLE map_null_first AS VALUES
1088+
(MAP {'k1': NULL}),
1089+
(MAP {'k1': 1, 'k2': 2});
1090+
1091+
query ? rowsort
1092+
SELECT * FROM map_null_first;
1093+
----
1094+
{k1: 1, k2: 2}
1095+
{k1: NULL}
1096+
1097+
statement ok
1098+
DROP TABLE map_null_first;
1099+
1100+
# every row has a NULL value: succeeds and the value type stays Null
1101+
statement ok
1102+
CREATE TABLE map_all_null_values AS VALUES
1103+
(MAP {'k': NULL}),
1104+
(MAP {'k': NULL});
1105+
1106+
query ? rowsort
1107+
SELECT * FROM map_all_null_values;
1108+
----
1109+
{k: NULL}
1110+
{k: NULL}
1111+
1112+
query T
1113+
SELECT arrow_typeof(column1) FROM map_all_null_values LIMIT 1;
1114+
----
1115+
Map("entries": non-null Struct("key": non-null Utf8, "value": Null), unsorted)
1116+
1117+
statement ok
1118+
DROP TABLE map_all_null_values;
1119+
1120+
# multiple keys where only one value is NULL
1121+
query ? rowsort
1122+
SELECT * FROM (VALUES
1123+
(MAP {'a': 1, 'b': NULL}),
1124+
(MAP {'a': 2, 'b': 3})) t(column1);
1125+
----
1126+
{a: 1, b: NULL}
1127+
{a: 2, b: 3}
1128+
1129+
# three rows with the NULL-valued row in the middle
1130+
query ? rowsort
1131+
SELECT * FROM (VALUES
1132+
(MAP {'k': 1}),
1133+
(MAP {'k': NULL}),
1134+
(MAP {'k': 2})) t(column1);
1135+
----
1136+
{k: 1}
1137+
{k: 2}
1138+
{k: NULL}
1139+
1140+
# numeric widening across a NULL-valued row follows the scalar rule
1141+
# (Int64 + Float64 -> Float64)
1142+
statement ok
1143+
CREATE TABLE map_null_widening AS VALUES
1144+
(MAP {'k': 1}),
1145+
(MAP {'k': NULL}),
1146+
(MAP {'k': 1.5});
1147+
1148+
query ? rowsort
1149+
SELECT * FROM map_null_widening;
1150+
----
1151+
{k: 1.0}
1152+
{k: 1.5}
1153+
{k: NULL}
1154+
1155+
query T
1156+
SELECT arrow_typeof(column1) FROM map_null_widening LIMIT 1;
1157+
----
1158+
Map("entries": non-null Struct("key": non-null Utf8, "value": Float64), unsorted)
1159+
1160+
statement ok
1161+
DROP TABLE map_null_widening;
1162+
1163+
# incompatible concrete value types with a NULL-valued row in between still
1164+
# error; Int64/Utf8 follows the scalar VALUES rule (coerce to the numeric
1165+
# type, then fail to cast the non-numeric string)
1166+
query error Cast error: Cannot cast string 'hello' to value of Int64 type
1167+
SELECT * FROM (VALUES
1168+
(MAP {'k': 1}),
1169+
(MAP {'k': NULL}),
1170+
(MAP {'k': 'hello'})) t(column1);
1171+
1172+
# NULL value type unification recurses into nested maps
1173+
query ? rowsort
1174+
SELECT * FROM (VALUES
1175+
(MAP {'outer': MAP {'inner': 1}}),
1176+
(MAP {'outer': MAP {'inner': NULL}})) t(column1);
1177+
----
1178+
{outer: {inner: 1}}
1179+
{outer: {inner: NULL}}
1180+
1181+
# INSERT INTO ... VALUES also accepts a NULL map value without a cast
1182+
statement ok
1183+
CREATE TABLE map_null_insert AS VALUES (MAP {'k1': 1, 'k2': 2});
1184+
1185+
statement ok
1186+
INSERT INTO map_null_insert VALUES (MAP {'k1': NULL});
1187+
1188+
query ? rowsort
1189+
SELECT * FROM map_null_insert;
1190+
----
1191+
{k1: 1, k2: 2}
1192+
{k1: NULL}
1193+
1194+
statement ok
1195+
DROP TABLE map_null_insert;

0 commit comments

Comments
 (0)