Skip to content

Commit ee66473

Browse files
committed
fix: route map_from_arrays float keys through the codegen dispatcher
Address review feedback on the float map key gate: - Mix CodegenDispatchFallback into CometMapFromArrays so FLOAT or DOUBLE keys and LAST_WIN stay in the Comet pipeline, as they do for map_from_entries. - Use expect_error_class for the dispatched DUPLICATED_MAP_KEY queries, and fix the array-key zeros query to read the row that holds zeros. - Rename the fixtures to map_builders_float_key_gate* so they do not collide with the ones in #6682. - Correct the MapKeySupport reason, which described the lookup as Spark normalizing keys, and trim the repeated explanation of the builder rules down to MapBuilderKeySupport.floatingPointReason.
1 parent cb576fc commit ee66473

11 files changed

Lines changed: 56 additions & 53 deletions

File tree

‎docs/source/contributor-guide/expression-audits/map_funcs.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
- Spark 3.5.8 (audited 2026-05-27): baseline. `MapFromArrays(left, right) extends BinaryExpression with NullIntolerant`; Spark uses `ArrayBasedMapBuilder` to detect duplicate keys (subject to `spark.sql.mapKeyDedupPolicy`) and rejects null keys with `RuntimeException("Cannot use null as map key")`. Comet `CometMapFromArrays` emits nested `CaseWhen(IsNotNull(left), CaseWhen(IsNotNull(right), map(left, right), null), null)` guards so NULL-array inputs return NULL rather than triggering the previously reported native crash ([#3327](https://github.com/apache/datafusion-comet/issues/3327)). The outer key guard preserves Spark's left-before-right evaluation: one `IsNotNull(left) AND IsNotNull(right)` predicate can still evaluate the values child for a NULL-key row in a mixed batch. The native `SparkMapFromArrays` wrapper also validates key/value lengths per non-null row before DataFusion constructs the map, so matching flattened totals cannot pair values across row boundaries.
4949
- Spark 4.0.1 (audited 2026-05-27): `NullIntolerant` trait replaced by `nullIntolerant: Boolean`. `ArrayBasedMapBuilder` gains the `keyNormalizer` described below.
5050
- Spark 4.1.1 (audited 2026-05-27): identical to 4.0.1.
51-
- Floating-point keys (2026-10-14, [#6549](https://github.com/apache/datafusion-comet/issues/6549)): `ArrayBasedMapBuilder` finds a duplicate `FLOAT` or `DOUBLE` key in a `HashMap` of boxed values (`Double.equals`, so NaNs with different bits are one key) and a duplicate struct or array key in a `TreeMap` in the SQL ordering (`-0.0` equals `0.0`). From Spark 4.0 its `keyNormalizer` also normalizes a `FLOAT` or `DOUBLE` key before the lookup, unless `spark.sql.legacy.disableMapKeyNormalization` is set. The native builders compare the keys' bits, so a key type that holds a float at any depth is `Incompatible` on every version (`MapBuilderKeySupport`). `map_from_arrays` falls back to Spark for them. `from` returns its input arrays when no key repeats, so a lone `-0.0` key stays `-0.0` there.
51+
- Floating-point keys (2026-10-14, [#6549](https://github.com/apache/datafusion-comet/issues/6549)): `ArrayBasedMapBuilder` finds a duplicate `FLOAT` or `DOUBLE` key in a `HashMap` of boxed values (`Double.equals`, so NaNs with different bits are one key) and a duplicate struct or array key in a `TreeMap` in the SQL ordering (`-0.0` equals `0.0`). From Spark 4.0 its `keyNormalizer` also normalizes a `FLOAT` or `DOUBLE` key before the lookup, unless `spark.sql.legacy.disableMapKeyNormalization` is set. The native builders compare the keys' bits, so a key type that holds a float at any depth is `Incompatible` on every version (`MapBuilderKeySupport`). `map_from_arrays` routes them through the codegen dispatcher. `from` returns its input arrays when no key repeats, so a lone `-0.0` key stays `-0.0` there.
5252

5353
## map_from_entries
5454

‎docs/source/user-guide/latest/compatibility/floating-point.md‎

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,7 @@ array whose elements cannot be null in ascending order, Spark's generated code p
168168
## Map construction
169169

170170
`map_from_arrays` and `map_from_entries` don't build a map natively when its key type holds a
171-
`FLOAT` or `DOUBLE` at any depth. Spark finds duplicate keys by value, so NaNs with different bits
172-
are one key, and inside a struct or array key `-0.0` and `0.0` are one key too. From Spark 4.0,
173-
unless `spark.sql.legacy.disableMapKeyNormalization` is set, Spark also normalizes a top-level key,
174-
so `-0.0` and `0.0` are one key and `map_from_entries` stores `0.0`. Comet's native map
175-
construction compares the keys' bits
176-
([#6549](https://github.com/apache/datafusion-comet/issues/6549)), so `map_from_entries` runs
177-
through the codegen dispatcher instead, and `map_from_arrays` falls back to Spark. `FLOAT` and
178-
`DOUBLE` values don't affect either function.
171+
`FLOAT` or `DOUBLE` at any depth. They run Spark's code through the codegen dispatcher instead, so
172+
duplicate keys, including NaNs and zeros of either sign, behave as in Spark. `FLOAT` and `DOUBLE`
173+
values don't affect either function. The expressions' entries in the
174+
[expression compatibility guide](expressions/map.md) give the reason.

‎docs/source/user-guide/latest/expressions.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -408,7 +408,7 @@ to Spark ([#2837](https://github.com/apache/datafusion-comet/issues/2837)). Enab
408408
| `map_concat` | ✅ | Codegen dispatch | |
409409
| `map_contains_key` | ✅ | — | |
410410
| `map_entries` | ✅ | Native | |
411-
| `map_from_arrays` | ✅ | Native | `FLOAT` or `DOUBLE` keys and `spark.sql.mapKeyDedupPolicy=LAST_WIN` fall back to Spark ([details](compatibility/expressions/map.md)) |
411+
| `map_from_arrays` | ✅ | Hybrid | `FLOAT` or `DOUBLE` keys and `spark.sql.mapKeyDedupPolicy=LAST_WIN` route through the JVM codegen dispatcher ([details](compatibility/expressions/map.md)) |
412412
| `map_from_entries` | ✅ | Hybrid | BinaryType keys/values, `FLOAT` or `DOUBLE` keys and `spark.sql.mapKeyDedupPolicy=LAST_WIN` route through the JVM codegen dispatcher ([details](compatibility/expressions/map.md)) |
413413
| `map_keys` | ✅ | Native | |
414414
| `map_values` | ✅ | Native | |

‎spark/src/main/scala/org/apache/comet/serde/maps.scala‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import org.apache.comet.shims.CometTypeShim
3434
private[serde] object MapKeySupport {
3535

3636
private val floatingPointReason: String =
37-
"Spark normalizes floating-point map keys, so `-0.0` matches a `+0.0` key and all `NaN`s " +
37+
"Spark looks a key up in its SQL ordering, so `-0.0` matches a `+0.0` key and all `NaN`s " +
3838
"match each other; Comet's native map lookup compares the raw Arrow values."
3939

4040
private val collationReason: String =
@@ -51,9 +51,10 @@ private[serde] object MapKeySupport {
5151
* finds a key with `TypeUtils.getInterpretedOrdering` over the keys `ArrayBasedMapBuilder`
5252
* stored, having first normalized them, while native `map_extract` compares the Arrow values as
5353
* they are, so decline the key types where those disagree:
54-
* - `ArrayBasedMapBuilder` rewrites a `-0.0` key to `+0.0` and canonicalises `NaN`, and
55-
* `nanSafeCompareDoubles` treats `-0.0` and `+0.0` as equal, so Spark answers a `-0.0`
56-
* lookup from a `+0.0` key where native finds nothing.
54+
* - `nanSafeCompareDoubles` treats `-0.0` and `+0.0` as equal and all `NaN`s as equal, so
55+
* Spark answers a `-0.0` lookup from a `+0.0` key where native finds nothing, on every
56+
* version. (How `ArrayBasedMapBuilder` treats float keys when it builds a map differs by
57+
* version; see `MapBuilderKeySupport`.)
5758
* - a non-default collation compares under rules native applies as `UTF8_BINARY`.
5859
* - for a complex key type, `map_extract`'s `coerce_types` returns the map's exact key field
5960
* type, so Comet's planner casts the lookup key to it. That cannot reproduce Spark's
@@ -134,13 +135,8 @@ object CometMapExtract extends CometExpressionSerde[GetMapValue] {
134135

135136
/**
136137
* Shared gate for the native map builders, `map_from_arrays` and `map_from_entries`, which tell
137-
* keys apart by their bits. Spark's `ArrayBasedMapBuilder` finds a duplicate `FLOAT` or `DOUBLE`
138-
* key in a `HashMap` of boxed values, where every NaN is one key, and a duplicate struct or array
139-
* key in a `TreeMap` in Spark's SQL ordering, where `-0.0` equals `0.0` too. From Spark 4.0 it
140-
* also normalizes a `FLOAT` or `DOUBLE` key before the lookup, unless
141-
* `spark.sql.legacy.disableMapKeyNormalization` is set, so the two zeros are one key and
142-
* `map_from_entries` stores `0.0`. A key type that holds a float at any depth is therefore
143-
* `Incompatible` on every version.
138+
* keys apart by their bits. A key type that holds a float at any depth is `Incompatible` on every
139+
* Spark version; `floatingPointReason` explains why.
144140
*/
145141
private object MapBuilderKeySupport {
146142
val floatingPointReason: String =
@@ -173,7 +169,9 @@ private object MapKeyDedupPolicySupport {
173169
.equalsIgnoreCase(SQLConf.MapKeyDedupPolicy.LAST_WIN.toString)
174170
}
175171

176-
object CometMapFromArrays extends CometExpressionSerde[MapFromArrays] {
172+
object CometMapFromArrays
173+
extends CometExpressionSerde[MapFromArrays]
174+
with CodegenDispatchFallback {
177175

178176
override def getIncompatibleReasons(): Seq[String] =
179177
Seq(MapBuilderKeySupport.floatingPointReason, MapKeyDedupPolicySupport.incompatibleReason)

‎spark/src/test/resources/sql-tests/expressions/map/element_at_map.sql‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,15 +58,15 @@ SELECT element_at(map('a', 1, 'b', 2), 'a'), element_at(map('a', 1, 'b', 2), 'mi
5858

5959
-- Spark stores `-0.0` map keys as `+0.0` and compares with `nanSafeCompareDoubles`, so a `-0.0`
6060
-- lookup finds the `+0.0` key. Native lookup compares the raw Arrow values.
61-
query expect_fallback(Spark normalizes floating-point map keys)
61+
query expect_fallback(Spark looks a key up in its SQL ordering)
6262
SELECT element_at(map(CAST(0 AS DOUBLE), 7), CAST(-0.0 AS DOUBLE))
6363

64-
query expect_fallback(Spark normalizes floating-point map keys)
64+
query expect_fallback(Spark looks a key up in its SQL ordering)
6565
SELECT element_at(map(CAST(0 AS FLOAT), 7), CAST(-0.0 AS FLOAT))
6666

6767
-- The floating-point decline walks every nesting level of the key type, so an array-of-double key
6868
-- falls back for the same reason.
69-
query expect_fallback(Spark normalizes floating-point map keys)
69+
query expect_fallback(Spark looks a key up in its SQL ordering)
7070
SELECT element_at(map(array(CAST(0 AS DOUBLE)), 7), array(CAST(-0.0 AS DOUBLE)))
7171

7272
-- A complex key type: `map_extract` casts the lookup key to the map's exact Arrow key type, so a

‎spark/src/test/resources/sql-tests/expressions/map/get_map_value.sql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,5 +44,5 @@ CREATE TABLE test_map_double(m map<double, int>) USING parquet
4444
statement
4545
INSERT INTO test_map_double VALUES (map(CAST(0 AS DOUBLE), 7)), (map(CAST(1 AS DOUBLE), 8)), (NULL)
4646

47-
query expect_fallback(Spark normalizes floating-point map keys)
47+
query expect_fallback(Spark looks a key up in its SQL ordering)
4848
SELECT m[CAST(-0.0 AS DOUBLE)] FROM test_map_double

spark/src/test/resources/sql-tests/expressions/map/map_builders_float_keys.sql renamed to spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate.sql

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@
1515
-- specific language governing permissions and limitations
1616
-- under the License.
1717

18-
-- Map keys that hold a FLOAT or DOUBLE (#6549). Spark finds a duplicate top-level key in a HashMap
19-
-- of boxed values, where NaNs with different bits are one key, and a duplicate struct or array key
20-
-- in a TreeMap in its SQL ordering, where -0.0 equals 0.0 too. From Spark 4.0 it also normalizes a
21-
-- top-level key, so map_from_entries stores 0.0 for -0.0. The native builders compare the keys'
22-
-- bits, so map_from_entries runs through the codegen dispatcher and map_from_arrays falls back.
23-
-- Top-level zeros, which only Spark 4.0+ merges, are in map_builders_float_keys_spark_4_0.sql.
18+
-- Map keys that hold a FLOAT or DOUBLE (#6549) are declined by the native builders. See
19+
-- `MapBuilderKeySupport.floatingPointReason` for how Spark treats them. map_from_entries runs
20+
-- through the codegen dispatcher, as does map_from_arrays. Top-level zeros, which only Spark 4.0+
21+
-- merges, are in map_builders_float_key_gate_spark_4_0.sql.
2422

2523
-- Parquet canonicalizes NaN, so negating a stored NaN gives one with the sign bit set.
2624
statement
@@ -39,52 +37,59 @@ SELECT id,
3937
map_keys(map_from_entries(array(named_struct('k', -f, 'v', 1), named_struct('k', 2.5F, 'v', 2))))
4038
FROM float_map_keys
4139

42-
query expect_fallback(native map construction compares the keys)
40+
query expect_dispatch(map_from_arrays)
4341
SELECT id,
4442
map_keys(map_from_arrays(array(-d, 2.5D), array(1, 2))),
4543
map_keys(map_from_arrays(array(-f, 2.5F), array(1, 2)))
4644
FROM float_map_keys
4745

4846
-- NaNs that differ only in the sign bit are one key on every version.
49-
query expect_error(DUPLICATED_MAP_KEY)
47+
query expect_error_class(DUPLICATED_MAP_KEY)
5048
SELECT map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))
5149
FROM float_map_keys WHERE id = 2
5250

53-
query expect_error(DUPLICATED_MAP_KEY)
51+
query expect_error_class(DUPLICATED_MAP_KEY)
5452
SELECT map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2)))
5553
FROM float_map_keys WHERE id = 2
5654

57-
query expect_error(DUPLICATED_MAP_KEY)
55+
query expect_error_class(DUPLICATED_MAP_KEY)
5856
SELECT map_from_arrays(array(d, -d), array(1, 2)) FROM float_map_keys WHERE id = 2
5957

60-
query expect_error(DUPLICATED_MAP_KEY)
58+
query expect_error_class(DUPLICATED_MAP_KEY)
6159
SELECT map_from_arrays(array(f, -f), array(1, 2)) FROM float_map_keys WHERE id = 2
6260

6361
-- Struct and array keys compare in Spark's SQL ordering, where -0.0 equals 0.0, on every version.
64-
query expect_error(DUPLICATED_MAP_KEY)
62+
query expect_error_class(DUPLICATED_MAP_KEY)
6563
SELECT map_from_entries(array(
6664
named_struct('k', named_struct('x', d), 'v', 1),
6765
named_struct('k', named_struct('x', -d), 'v', 2)))
6866
FROM float_map_keys WHERE id = 1
6967

70-
query expect_error(DUPLICATED_MAP_KEY)
68+
query expect_error_class(DUPLICATED_MAP_KEY)
7169
SELECT map_from_entries(array(
7270
named_struct('k', array(f), 'v', 1),
7371
named_struct('k', array(-f), 'v', 2)))
7472
FROM float_map_keys WHERE id = 1
7573

76-
query expect_error(DUPLICATED_MAP_KEY)
74+
query expect_error_class(DUPLICATED_MAP_KEY)
7775
SELECT map_from_arrays(array(named_struct('x', d), named_struct('x', -d)), array(1, 2))
7876
FROM float_map_keys WHERE id = 1
7977

80-
query expect_error(DUPLICATED_MAP_KEY)
78+
query expect_error_class(DUPLICATED_MAP_KEY)
79+
SELECT map_from_arrays(array(array(d), array(-d)), array(1, 2)) FROM float_map_keys WHERE id = 1
80+
81+
-- NaNs that differ only in the sign bit are one key inside an array key too.
82+
query expect_error_class(DUPLICATED_MAP_KEY)
8183
SELECT map_from_arrays(array(array(d), array(-d)), array(1, 2)) FROM float_map_keys WHERE id = 2
8284

8385
-- Distinct keys still build a map.
8486
query expect_dispatch(map_from_entries)
8587
SELECT id, map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', d + 1, 'v', 2)))
8688
FROM float_map_keys WHERE id IN (1, 3)
8789

90+
query expect_dispatch(map_from_arrays)
91+
SELECT id, map_from_arrays(array(d, d + 1), array(1, 2)) FROM float_map_keys WHERE id IN (1, 3)
92+
8893
-- Only the key type matters: a FLOAT or DOUBLE value stays native.
8994
query expect_native(map_from_entries)
9095
SELECT id, map_from_entries(array(named_struct('k', id, 'v', d), named_struct('k', id + 10, 'v', -d)))

spark/src/test/resources/sql-tests/expressions/map/map_builders_float_keys_legacy.sql renamed to spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_legacy.sql

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717

1818
-- With spark.sql.legacy.disableMapKeyNormalization, Spark 4.0+ leaves FLOAT and DOUBLE map keys as
1919
-- they are, as Spark 3.4 and 3.5 always do (they don't have the flag). So 0.0 and -0.0 are two
20-
-- keys and a -0.0 key stays -0.0. map_from_entries runs Spark's code in the codegen dispatcher,
21-
-- which has to see the session's flag.
20+
-- keys and a -0.0 key stays -0.0. map_from_entries and map_from_arrays run Spark's code in the
21+
-- codegen dispatcher, which has to see the session's flag.
2222

2323
-- Config: spark.sql.legacy.disableMapKeyNormalization=true
2424

@@ -34,7 +34,7 @@ SELECT
3434
map_keys(map_from_entries(array(named_struct('k', -f, 'v', 1), named_struct('k', 2.5F, 'v', 2))))
3535
FROM float_map_keys_legacy
3636

37-
query expect_fallback(native map construction compares the keys)
37+
query expect_dispatch(map_from_arrays)
3838
SELECT
3939
map_keys(map_from_arrays(array(d, -d), array(1, 2))),
4040
map_keys(map_from_arrays(array(f, -f), array(1, 2)))

spark/src/test/resources/sql-tests/expressions/map/map_builders_float_keys_spark_4_0.sql renamed to spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_spark_4_0.sql

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
-- From Spark 4.0, ArrayBasedMapBuilder normalizes a top-level FLOAT or DOUBLE key before it looks
1919
-- for a duplicate, so 0.0 and -0.0 are one key (#6549). Earlier releases keep them apart, and so
20-
-- does spark.sql.legacy.disableMapKeyNormalization (map_builders_float_keys_legacy.sql).
20+
-- does spark.sql.legacy.disableMapKeyNormalization (map_builders_float_key_gate_legacy.sql).
2121

2222
-- MinSparkVersion: 4.0
2323

@@ -27,16 +27,16 @@ CREATE TABLE float_map_keys_zeros(d double, f float) USING parquet
2727
statement
2828
INSERT INTO float_map_keys_zeros VALUES (0.0, 0.0)
2929

30-
query expect_error(DUPLICATED_MAP_KEY)
30+
query expect_error_class(DUPLICATED_MAP_KEY)
3131
SELECT map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))
3232
FROM float_map_keys_zeros
3333

34-
query expect_error(DUPLICATED_MAP_KEY)
34+
query expect_error_class(DUPLICATED_MAP_KEY)
3535
SELECT map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2)))
3636
FROM float_map_keys_zeros
3737

38-
query expect_error(DUPLICATED_MAP_KEY)
38+
query expect_error_class(DUPLICATED_MAP_KEY)
3939
SELECT map_from_arrays(array(d, -d), array(1, 2)) FROM float_map_keys_zeros
4040

41-
query expect_error(DUPLICATED_MAP_KEY)
41+
query expect_error_class(DUPLICATED_MAP_KEY)
4242
SELECT map_from_arrays(array(f, -f), array(1, 2)) FROM float_map_keys_zeros

‎spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy.sql‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,14 @@
1717

1818
-- Verifies that `map_from_arrays` falls back to Spark when `spark.sql.mapKeyDedupPolicy` is set
1919
-- to `LAST_WIN`. Spark's ArrayBasedMapBuilder keeps the last occurrence of each duplicate key;
20-
-- Comet's native `map` scalar has no LAST_WIN path, so it must fall back. The default `EXCEPTION`
21-
-- mode agrees with Comet and is covered by `map_from_arrays.sql`.
20+
-- Comet's native `map` scalar has no LAST_WIN path. `CometMapFromArrays` mixes in
21+
-- `CodegenDispatchFallback`, so its native `Incompatible` normally routes through the JVM codegen
22+
-- dispatcher; we disable the dispatcher here so the incompat branch surfaces as a genuine Spark
23+
-- fallback rather than in-pipeline codegen. The default `EXCEPTION` mode agrees with Comet and is
24+
-- covered by `map_from_arrays.sql`.
2225

2326
-- Config: spark.sql.mapKeyDedupPolicy=LAST_WIN
27+
-- Config: spark.comet.exec.scalaUDF.codegen.enabled=false
2428

2529
statement
2630
CREATE TABLE test_map_from_arrays_dedup(k array<string>, v array<int>) USING parquet

0 commit comments

Comments
 (0)