diff --git a/docs/source/contributor-guide/expression-audits/map_funcs.md b/docs/source/contributor-guide/expression-audits/map_funcs.md index f50475a449e..d37a29df1bb 100644 --- a/docs/source/contributor-guide/expression-audits/map_funcs.md +++ b/docs/source/contributor-guide/expression-audits/map_funcs.md @@ -46,8 +46,9 @@ - Spark 3.4.3 (audited 2026-05-27): identical to 3.5.8. - 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. -- Spark 4.0.1 (audited 2026-05-27): semantics unchanged; `NullIntolerant` trait replaced by `nullIntolerant: Boolean`. +- Spark 4.0.1 (audited 2026-05-27): `NullIntolerant` trait replaced by `nullIntolerant: Boolean`. `ArrayBasedMapBuilder` gains the `keyNormalizer` described below. - Spark 4.1.1 (audited 2026-05-27): identical to 4.0.1. +- 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. ## map_from_entries @@ -56,6 +57,7 @@ - Spark 4.0.1 (audited 2026-05-27): semantics unchanged; trait refactor. - Spark 4.1.1 (audited 2026-05-27): identical to 4.0.1. - Known limitation: input arrays where the struct's key or value type contains `BinaryType` are marked `Incompatible` and fall back unless `spark.comet.expression.MapFromEntries.allowIncompatible=true`. +- 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_entries` routes them through the codegen dispatcher. It always calls `build()`, so on Spark 4.0+ it stores a `-0.0` key as `0.0`. ## map_keys diff --git a/docs/source/contributor-guide/floating_point.md b/docs/source/contributor-guide/floating_point.md index 165b280c634..758350db8f1 100644 --- a/docs/source/contributor-guide/floating_point.md +++ b/docs/source/contributor-guide/floating_point.md @@ -62,10 +62,11 @@ it runs against: (before 3.5.2, unless probing merges them). Spark 4.2.0 normalizes their arguments in the plan (SPARK-54918). From 4.0.5, 4.1.4 and 4.2.1 they normalize while they evaluate instead (SPARK-59602). -- Map construction (`ArrayBasedMapBuilder`) finds duplicate keys by `Double.equals` in Spark 3.4 - and 3.5. From Spark 4.0 it normalizes each key first, unless - `spark.sql.legacy.disableMapKeyNormalization` is set - ([#6549](https://github.com/apache/datafusion-comet/issues/6549)). +- Map construction (`ArrayBasedMapBuilder`) finds a duplicate `FLOAT` or `DOUBLE` key in a + `HashMap` of boxed values, which follows `Double.equals`, and a duplicate struct or array key in a + `TreeMap` in the SQL ordering. From Spark 4.0 it normalizes a `FLOAT` or `DOUBLE` key before the + lookup, unless `spark.sql.legacy.disableMapKeyNormalization` is set, and `map_from_entries` + stores the normalized key ([#6549](https://github.com/apache/datafusion-comet/issues/6549)). ## How Arrow and DataFusion differ diff --git a/docs/source/user-guide/latest/compatibility/floating-point.md b/docs/source/user-guide/latest/compatibility/floating-point.md index f00ceceaf6e..77d76f53c61 100644 --- a/docs/source/user-guide/latest/compatibility/floating-point.md +++ b/docs/source/user-guide/latest/compatibility/floating-point.md @@ -164,3 +164,11 @@ representations tie, and keeps equal elements in their original order. In an arr can be null, such as one built from nullable columns, `-0.0` and `0.0` therefore tie. Sorting an array whose elements cannot be null in ascending order, Spark's generated code puts `-0.0` before `0.0`, and so does Comet. All three expressions run natively in strict floating-point mode. + +## Map construction + +`map_from_arrays` and `map_from_entries` don't build a map natively when its key type holds a +`FLOAT` or `DOUBLE` at any depth. They run Spark's code through the codegen dispatcher instead, so +duplicate keys, including NaNs and zeros of either sign, behave as in Spark. `FLOAT` and `DOUBLE` +values don't affect either function. The expressions' entries in the +[expression compatibility guide](expressions/map.md) give the reason. diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index d11393fa448..4a85b907f0e 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -408,8 +408,8 @@ to Spark ([#2837](https://github.com/apache/datafusion-comet/issues/2837)). Enab | `map_concat` | ✅ | Codegen dispatch | | | `map_contains_key` | ✅ | — | | | `map_entries` | ✅ | Native | | -| `map_from_arrays` | ✅ | Native | | -| `map_from_entries` | ✅ | Hybrid | BinaryType keys/values and `spark.sql.mapKeyDedupPolicy=LAST_WIN` route through the JVM codegen dispatcher ([details](compatibility/expressions/map.md)) | +| `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)) | +| `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)) | | `map_keys` | ✅ | Native | | | `map_values` | ✅ | Native | | | `str_to_map` | ✅ | Hybrid | | diff --git a/spark/src/main/scala/org/apache/comet/serde/maps.scala b/spark/src/main/scala/org/apache/comet/serde/maps.scala index ae32794d7fb..86376ccfca8 100644 --- a/spark/src/main/scala/org/apache/comet/serde/maps.scala +++ b/spark/src/main/scala/org/apache/comet/serde/maps.scala @@ -34,7 +34,7 @@ import org.apache.comet.shims.CometTypeShim private[serde] object MapKeySupport { private val floatingPointReason: String = - "Spark normalizes floating-point map keys, so `-0.0` matches a `+0.0` key and all `NaN`s " + + "Spark looks a key up in its SQL ordering, so `-0.0` matches a `+0.0` key and all `NaN`s " + "match each other; Comet's native map lookup compares the raw Arrow values." private val collationReason: String = @@ -48,12 +48,13 @@ private[serde] object MapKeySupport { /** * The `SupportLevel` for a map-consuming expression whose stored-key type is `keyType`. Spark - * finds a key with `TypeUtils.getInterpretedOrdering` over the keys `ArrayBasedMapBuilder` - * stored, having first normalized them, while native `map_extract` compares the Arrow values as - * they are, so decline the key types where those disagree: - * - `ArrayBasedMapBuilder` rewrites a `-0.0` key to `+0.0` and canonicalises `NaN`, and - * `nanSafeCompareDoubles` treats `-0.0` and `+0.0` as equal, so Spark answers a `-0.0` - * lookup from a `+0.0` key where native finds nothing. + * finds a key with `TypeUtils.getInterpretedOrdering` over the map's stored keys, while native + * `map_extract` compares the Arrow values as they are, so decline the key types where those + * disagree: + * - `nanSafeCompareDoubles` treats `-0.0` and `+0.0` as equal and all `NaN`s as equal, so + * Spark answers a `-0.0` lookup from a `+0.0` key where native finds nothing, on every + * version. (How `ArrayBasedMapBuilder` treats float keys when it builds a map differs by + * version; see `MapBuilderKeySupport`.) * - a non-default collation compares under rules native applies as `UTF8_BINARY`. * - for a complex key type, `map_extract`'s `coerce_types` returns the map's exact key field * type, so Comet's planner casts the lookup key to it. That cannot reproduce Spark's @@ -132,6 +133,23 @@ object CometMapExtract extends CometExpressionSerde[GetMapValue] { } } +/** + * Shared gate for the native map builders, `map_from_arrays` and `map_from_entries`, which tell + * keys apart by their bits. A key type that holds a float at any depth is `Incompatible` on every + * Spark version; `floatingPointReason` explains why. + */ +private object MapBuilderKeySupport { + val floatingPointReason: String = + "Spark finds duplicate map keys that hold a `FLOAT` or `DOUBLE` by value, so NaNs with " + + "different bits are one key, and so are `-0.0` and `0.0` inside a struct or array key. " + + "From Spark 4.0, a `-0.0` key is also normalized to `0.0`, unless " + + "`spark.sql.legacy.disableMapKeyNormalization` is set. Comet's native map construction " + + "compares the keys' bits ([#6549](https://github.com/apache/datafusion-comet/issues/6549))." + + def hasFloatingPointKey(dataType: MapType): Boolean = + SupportLevel.containsType(dataType.keyType, classOf[FloatType], classOf[DoubleType]) +} + private object MapKeyDedupPolicySupport { val incompatibleReason: String = s"`${SQLConf.MAP_KEY_DEDUP_POLICY.key}` is set to " + @@ -151,16 +169,20 @@ private object MapKeyDedupPolicySupport { .equalsIgnoreCase(SQLConf.MapKeyDedupPolicy.LAST_WIN.toString) } -object CometMapFromArrays extends CometExpressionSerde[MapFromArrays] { +object CometMapFromArrays + extends CometExpressionSerde[MapFromArrays] + with CodegenDispatchFallback { override def getIncompatibleReasons(): Seq[String] = - Seq(MapKeyDedupPolicySupport.incompatibleReason) + Seq(MapBuilderKeySupport.floatingPointReason, MapKeyDedupPolicySupport.incompatibleReason) override def getCompatibleNotes(): Seq[String] = Seq(MapKeyDedupPolicySupport.nullKeyReason) override def getSupportLevel(expr: MapFromArrays): SupportLevel = { - if (MapKeyDedupPolicySupport.isLastWin) { + if (MapBuilderKeySupport.hasFloatingPointKey(expr.dataType)) { + Incompatible(Some(MapBuilderKeySupport.floatingPointReason)) + } else if (MapKeyDedupPolicySupport.isLastWin) { Incompatible(Some(MapKeyDedupPolicySupport.incompatibleReason)) } else { Compatible(None) @@ -214,7 +236,11 @@ object CometMapFromEntries "`BinaryType` is not supported as a map value in `map_from_entries`" override def getIncompatibleReasons(): Seq[String] = - Seq(keyUnsupportedReason, valueUnsupportedReason, MapKeyDedupPolicySupport.incompatibleReason) + Seq( + keyUnsupportedReason, + valueUnsupportedReason, + MapBuilderKeySupport.floatingPointReason, + MapKeyDedupPolicySupport.incompatibleReason) override def getCompatibleNotes(): Seq[String] = Seq(MapKeyDedupPolicySupport.nullKeyReason) @@ -224,6 +250,8 @@ object CometMapFromEntries Incompatible(Some(keyUnsupportedReason)) } else if (SupportLevel.containsType(expr.dataType.valueType, classOf[BinaryType])) { Incompatible(Some(valueUnsupportedReason)) + } else if (MapBuilderKeySupport.hasFloatingPointKey(expr.dataType)) { + Incompatible(Some(MapBuilderKeySupport.floatingPointReason)) } else if (MapKeyDedupPolicySupport.isLastWin) { Incompatible(Some(MapKeyDedupPolicySupport.incompatibleReason)) } else { diff --git a/spark/src/test/resources/sql-tests/expressions/map/element_at_map.sql b/spark/src/test/resources/sql-tests/expressions/map/element_at_map.sql index 95450b81592..b98854320f8 100644 --- a/spark/src/test/resources/sql-tests/expressions/map/element_at_map.sql +++ b/spark/src/test/resources/sql-tests/expressions/map/element_at_map.sql @@ -56,17 +56,17 @@ SELECT element_at(map('a', 1, 'b', 2), 'a'), element_at(map('a', 1, 'b', 2), 'mi -- Spark. These stay on the constructor path here because the SQL harness excludes -- `ConstantFolding`; `CometMapExpressionSuite` covers the folded-literal form of each. --- Spark stores `-0.0` map keys as `+0.0` and compares with `nanSafeCompareDoubles`, so a `-0.0` --- lookup finds the `+0.0` key. Native lookup compares the raw Arrow values. -query expect_fallback(Spark normalizes floating-point map keys) +-- Spark compares with `nanSafeCompareDoubles`, so a `-0.0` lookup finds the `+0.0` key. Native +-- lookup compares the raw Arrow values. +query expect_fallback(Spark looks a key up in its SQL ordering) SELECT element_at(map(CAST(0 AS DOUBLE), 7), CAST(-0.0 AS DOUBLE)) -query expect_fallback(Spark normalizes floating-point map keys) +query expect_fallback(Spark looks a key up in its SQL ordering) SELECT element_at(map(CAST(0 AS FLOAT), 7), CAST(-0.0 AS FLOAT)) -- The floating-point decline walks every nesting level of the key type, so an array-of-double key -- falls back for the same reason. -query expect_fallback(Spark normalizes floating-point map keys) +query expect_fallback(Spark looks a key up in its SQL ordering) SELECT element_at(map(array(CAST(0 AS DOUBLE)), 7), array(CAST(-0.0 AS DOUBLE))) -- A complex key type: `map_extract` casts the lookup key to the map's exact Arrow key type, so a diff --git a/spark/src/test/resources/sql-tests/expressions/map/get_map_value.sql b/spark/src/test/resources/sql-tests/expressions/map/get_map_value.sql index 2c6c23846eb..b693cf5ae15 100644 --- a/spark/src/test/resources/sql-tests/expressions/map/get_map_value.sql +++ b/spark/src/test/resources/sql-tests/expressions/map/get_map_value.sql @@ -36,13 +36,13 @@ SELECT map('a', 1, 'b', 2)['a'], map('a', 1, 'b', 2)['missing'], map('a', 1, 'b' -- `element_at_map.sql` exercises the same guard on `CometElementAt`). A `map` column -- is used rather than a `map(...)` literal because Spark's `SimplifyExtractValueOps` rewrites -- `map(...)[key]` over a literal map into a `CASE` before it can reach the native lookup. Spark --- stores a `-0.0` key as `+0.0` and finds it with `nanSafeCompareDoubles`, so it returns `7` for a --- `-0.0` lookup; native lookup compares the raw Arrow values. +-- compares with `nanSafeCompareDoubles`, so a `-0.0` lookup finds the `+0.0` key and returns `7`; +-- native lookup compares the raw Arrow values. statement CREATE TABLE test_map_double(m map) USING parquet statement INSERT INTO test_map_double VALUES (map(CAST(0 AS DOUBLE), 7)), (map(CAST(1 AS DOUBLE), 8)), (NULL) -query expect_fallback(Spark normalizes floating-point map keys) +query expect_fallback(Spark looks a key up in its SQL ordering) SELECT m[CAST(-0.0 AS DOUBLE)] FROM test_map_double diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate.sql b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate.sql new file mode 100644 index 00000000000..6708e62e3c7 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate.sql @@ -0,0 +1,99 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- Map keys that hold a FLOAT or DOUBLE (#6549) are declined by the native builders. See +-- `MapBuilderKeySupport.floatingPointReason` for how Spark treats them. map_from_entries runs +-- through the codegen dispatcher, as does map_from_arrays. Top-level zeros, which only Spark 4.0+ +-- merges, are in map_builders_float_key_gate_spark_4_0.sql. + +-- Parquet canonicalizes NaN, so negating a stored NaN gives one with the sign bit set. +statement +CREATE TABLE float_map_keys(id int, d double, f float) USING parquet + +statement +INSERT INTO float_map_keys VALUES + (1, 0.0, 0.0), + (2, double('NaN'), float('NaN')), + (3, 1.5, 1.5) + +-- A lone -0.0 key: Spark 4.0+ stores 0.0, earlier releases keep -0.0. +query expect_dispatch(map_from_entries) +SELECT id, + map_keys(map_from_entries(array(named_struct('k', -d, 'v', 1), named_struct('k', 2.5D, 'v', 2)))), + map_keys(map_from_entries(array(named_struct('k', -f, 'v', 1), named_struct('k', 2.5F, 'v', 2)))) +FROM float_map_keys + +query expect_dispatch(map_from_arrays) +SELECT id, + map_keys(map_from_arrays(array(-d, 2.5D), array(1, 2))), + map_keys(map_from_arrays(array(-f, 2.5F), array(1, 2))) +FROM float_map_keys + +-- NaNs that differ only in the sign bit are one key on every version. +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2))) +FROM float_map_keys WHERE id = 2 + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2))) +FROM float_map_keys WHERE id = 2 + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(d, -d), array(1, 2)) FROM float_map_keys WHERE id = 2 + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(f, -f), array(1, 2)) FROM float_map_keys WHERE id = 2 + +-- Struct and array keys compare in Spark's SQL ordering, where -0.0 equals 0.0, on every version. +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_entries(array( + named_struct('k', named_struct('x', d), 'v', 1), + named_struct('k', named_struct('x', -d), 'v', 2))) +FROM float_map_keys WHERE id = 1 + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_entries(array( + named_struct('k', array(f), 'v', 1), + named_struct('k', array(-f), 'v', 2))) +FROM float_map_keys WHERE id = 1 + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(named_struct('x', d), named_struct('x', -d)), array(1, 2)) +FROM float_map_keys WHERE id = 1 + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(array(d), array(-d)), array(1, 2)) FROM float_map_keys WHERE id = 1 + +-- NaNs that differ only in the sign bit are one key inside an array key too. +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(array(d), array(-d)), array(1, 2)) FROM float_map_keys WHERE id = 2 + +-- Distinct keys still build a map. +query expect_dispatch(map_from_entries) +SELECT id, map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', d + 1, 'v', 2))) +FROM float_map_keys WHERE id IN (1, 3) + +query expect_dispatch(map_from_arrays) +SELECT id, map_from_arrays(array(d, d + 1), array(1, 2)) FROM float_map_keys WHERE id IN (1, 3) + +-- Only the key type matters: a FLOAT or DOUBLE value stays native. +query expect_native(map_from_entries) +SELECT id, map_from_entries(array(named_struct('k', id, 'v', d), named_struct('k', id + 10, 'v', -d))) +FROM float_map_keys + +query +SELECT id, map_from_arrays(array(id, id + 10), array(d, -d)) FROM float_map_keys diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_legacy.sql b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_legacy.sql new file mode 100644 index 00000000000..c1cbfab3358 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_legacy.sql @@ -0,0 +1,41 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- With spark.sql.legacy.disableMapKeyNormalization, Spark 4.0+ leaves FLOAT and DOUBLE map keys as +-- 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 +-- keys and a -0.0 key stays -0.0. map_from_entries and map_from_arrays run Spark's code in the +-- codegen dispatcher, which has to see the session's flag. + +-- Config: spark.sql.legacy.disableMapKeyNormalization=true + +statement +CREATE TABLE float_map_keys_legacy(d double, f float) USING parquet + +statement +INSERT INTO float_map_keys_legacy VALUES (0.0, 0.0) + +query expect_dispatch(map_from_entries) +SELECT + map_keys(map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))), + map_keys(map_from_entries(array(named_struct('k', -f, 'v', 1), named_struct('k', 2.5F, 'v', 2)))) +FROM float_map_keys_legacy + +query expect_dispatch(map_from_arrays) +SELECT + map_keys(map_from_arrays(array(d, -d), array(1, 2))), + map_keys(map_from_arrays(array(f, -f), array(1, 2))) +FROM float_map_keys_legacy diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_spark_4_0.sql b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_spark_4_0.sql new file mode 100644 index 00000000000..7813c6598c6 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_gate_spark_4_0.sql @@ -0,0 +1,42 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. + +-- From Spark 4.0, ArrayBasedMapBuilder normalizes a top-level FLOAT or DOUBLE key before it looks +-- for a duplicate, so 0.0 and -0.0 are one key (#6549). Earlier releases keep them apart, and so +-- does spark.sql.legacy.disableMapKeyNormalization (map_builders_float_key_gate_legacy.sql). + +-- MinSparkVersion: 4.0 + +statement +CREATE TABLE float_map_keys_zeros(d double, f float) USING parquet + +statement +INSERT INTO float_map_keys_zeros VALUES (0.0, 0.0) + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2))) +FROM float_map_keys_zeros + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2))) +FROM float_map_keys_zeros + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(d, -d), array(1, 2)) FROM float_map_keys_zeros + +query expect_error_class(DUPLICATED_MAP_KEY) +SELECT map_from_arrays(array(f, -f), array(1, 2)) FROM float_map_keys_zeros diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_last_win.sql b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_last_win.sql new file mode 100644 index 00000000000..b3c210dab1c --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/map/map_builders_float_key_last_win.sql @@ -0,0 +1,93 @@ +-- Licensed to the Apache Software Foundation (ASF) under one +-- or more contributor license agreements. See the NOTICE file +-- distributed with this work for additional information +-- regarding copyright ownership. The ASF licenses this file +-- to you under the Apache License, Version 2.0 (the +-- "License"); you may not use this file except in compliance +-- with the License. You may obtain a copy of the License at +-- +-- http://www.apache.org/licenses/LICENSE-2.0 +-- +-- Unless required by applicable law or agreed to in writing, +-- software distributed under the License is distributed on an +-- "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +-- KIND, either express or implied. See the License for the +-- specific language governing permissions and limitations +-- under the License. +-- Under `spark.sql.mapKeyDedupPolicy=LAST_WIN` a repeated map key is not an error: the builder keeps +-- the last value. `CometMapFromArrays` and `CometMapFromEntries` mix in `CodegenDispatchFallback`, +-- so with the dispatcher on (the default) both run Spark's own `ArrayBasedMapBuilder` in the JVM +-- codegen dispatcher. The dedup-policy fixtures turn the dispatcher off to cover the fallback, so +-- these queries cover the dispatched path with a key that really repeats: `from` then goes through +-- `build()`, and both the kept value and the stored key are compared with Spark's. +-- +-- A `FLOAT` or `DOUBLE` key is declined natively on every version (#6549), so those dispatch too. +-- NaNs that differ only in the sign bit are one key on every version. `-0.0` and `0.0` are one key +-- from Spark 4.0, unless `spark.sql.legacy.disableMapKeyNormalization` is set, and two keys on +-- earlier releases; either way the dispatcher has to agree with Spark. + +-- Config: spark.sql.mapKeyDedupPolicy=LAST_WIN +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=true + +-- Parquet canonicalizes NaN, so negating a stored NaN gives one with the sign bit set. +statement +CREATE TABLE float_map_keys_last_win(id int, d double, f float, k array, v array) +USING parquet + +statement +INSERT INTO float_map_keys_last_win VALUES + (1, 0.0, 0.0, array('a', 'a', 'b'), array(1, 2, 3)), + (2, double('NaN'), float('NaN'), array('x', 'x'), array(10, 20)), + (3, 1.5, 1.5, array('a', 'b', 'c'), array(1, 2, 3)) + +-- A repeated string key: the last value wins. +query expect_dispatch(map_from_arrays) +SELECT id, map_keys(map_from_arrays(k, v)), map_values(map_from_arrays(k, v)) +FROM float_map_keys_last_win + +query expect_dispatch(map_from_entries) +SELECT id, + map_keys(map_from_entries(arrays_zip(k, v))), + map_values(map_from_entries(arrays_zip(k, v))) +FROM float_map_keys_last_win + +-- NaNs with different sign bits are one DOUBLE or FLOAT key: one entry, holding the last value. +query expect_dispatch(map_from_arrays) +SELECT map_keys(map_from_arrays(array(d, -d), array(1, 2))), + map_values(map_from_arrays(array(d, -d), array(1, 2))), + map_keys(map_from_arrays(array(f, -f), array(1, 2))), + map_values(map_from_arrays(array(f, -f), array(1, 2))) +FROM float_map_keys_last_win WHERE id = 2 + +query expect_dispatch(map_from_entries) +SELECT map_keys(map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))), + map_values(map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))), + map_keys(map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2)))), + map_values(map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2)))) +FROM float_map_keys_last_win WHERE id = 2 + +-- 0.0 and -0.0: one key holding the last value from Spark 4.0, two keys before that. +query expect_dispatch(map_from_arrays) +SELECT map_keys(map_from_arrays(array(d, -d), array(1, 2))), + map_values(map_from_arrays(array(d, -d), array(1, 2))), + map_keys(map_from_arrays(array(f, -f), array(1, 2))), + map_values(map_from_arrays(array(f, -f), array(1, 2))) +FROM float_map_keys_last_win WHERE id = 1 + +query expect_dispatch(map_from_entries) +SELECT map_keys(map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))), + map_values(map_from_entries(array(named_struct('k', d, 'v', 1), named_struct('k', -d, 'v', 2)))), + map_keys(map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2)))), + map_values(map_from_entries(array(named_struct('k', f, 'v', 1), named_struct('k', -f, 'v', 2)))) +FROM float_map_keys_last_win WHERE id = 1 + +-- The stored key, when -0.0 is the first key: 0.0 from Spark 4.0 (normalized), -0.0 before that. +query expect_dispatch(map_from_arrays) +SELECT map_keys(map_from_arrays(array(-d, d), array(1, 2))), + map_values(map_from_arrays(array(-d, d), array(1, 2))) +FROM float_map_keys_last_win WHERE id = 1 + +query expect_dispatch(map_from_entries) +SELECT map_keys(map_from_entries(array(named_struct('k', -d, 'v', 1), named_struct('k', d, 'v', 2)))), + map_values(map_from_entries(array(named_struct('k', -d, 'v', 1), named_struct('k', d, 'v', 2)))) +FROM float_map_keys_last_win WHERE id = 1 diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy.sql b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy.sql index fffaf5f9a92..f1d49952461 100644 --- a/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy.sql +++ b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy.sql @@ -17,10 +17,15 @@ -- Verifies that `map_from_arrays` falls back to Spark when `spark.sql.mapKeyDedupPolicy` is set -- to `LAST_WIN`. Spark's ArrayBasedMapBuilder keeps the last occurrence of each duplicate key; --- Comet's native `map` scalar has no LAST_WIN path, so it must fall back. The default `EXCEPTION` --- mode agrees with Comet and is covered by `map_from_arrays.sql`. +-- Comet's native `map` scalar has no LAST_WIN path. `CometMapFromArrays` mixes in +-- `CodegenDispatchFallback`, so its native `Incompatible` normally routes through the JVM codegen +-- dispatcher; we disable the dispatcher here so the incompat branch surfaces as a genuine Spark +-- fallback rather than in-pipeline codegen. The dispatched path, with a key that really repeats, is +-- covered by `map_builders_float_key_last_win.sql`. The default `EXCEPTION` mode agrees with Comet +-- and is covered by `map_from_arrays.sql`. -- Config: spark.sql.mapKeyDedupPolicy=LAST_WIN +-- Config: spark.comet.exec.scalaUDF.codegen.enabled=false statement CREATE TABLE test_map_from_arrays_dedup(k array, v array) USING parquet diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_from_entries_dedup_policy.sql b/spark/src/test/resources/sql-tests/expressions/map/map_from_entries_dedup_policy.sql index feba7951933..73706cb1f47 100644 --- a/spark/src/test/resources/sql-tests/expressions/map/map_from_entries_dedup_policy.sql +++ b/spark/src/test/resources/sql-tests/expressions/map/map_from_entries_dedup_policy.sql @@ -19,8 +19,9 @@ -- to `LAST_WIN`. `CometMapFromEntries` mixes in `CodegenDispatchFallback`, so its native -- `Incompatible` normally routes through the JVM codegen dispatcher; we disable the dispatcher -- here so the incompat branch surfaces as a genuine Spark fallback rather than in-pipeline --- codegen. The default `EXCEPTION` mode agrees with Comet and is covered by --- `map_from_entries.sql`. +-- codegen. The dispatched path, with a key that really repeats, is covered by +-- `map_builders_float_key_last_win.sql`. The default `EXCEPTION` mode agrees with Comet and is +-- covered by `map_from_entries.sql`. -- Config: spark.sql.mapKeyDedupPolicy=LAST_WIN -- Config: spark.comet.exec.scalaUDF.codegen.enabled=false diff --git a/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala index da07c05f673..6b8202d240c 100644 --- a/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala @@ -399,7 +399,7 @@ class CometMapExpressionSuite extends CometTestBase { checkSparkAnswerAndFallbackReason( "SELECT _1 AS id, element_at(element_at(map(1, map(CAST(0 AS DOUBLE), 7)), _1), " + s"$negZero) AS v FROM tbl", - "Spark normalizes floating-point map keys") + "Spark looks a key up in its SQL ordering") } } @@ -489,7 +489,7 @@ class CometMapExpressionSuite extends CometTestBase { } // Direct single-level folded map with floating-point keys: `map(CAST(0 AS DOUBLE), 7)` folds to a - // literal and `element_at` with a dynamic `-0.0` lookup must match Spark's `+0.0`-normalized key. + // literal and `element_at` with a dynamic `-0.0` lookup must match Spark's `+0.0` key. // Native `map_extract` compares raw Arrow values, so `MapKeySupport` declines it at `element_at`. // Spark returns 7, NULL, NULL. (`element_at_map.sql` covers the folding-off constructor form.) test("folded map literal with floating-point keys in element_at falls back (multirow)") { @@ -497,7 +497,7 @@ class CometMapExpressionSuite extends CometTestBase { val lookup = "CAST(concat('-', CAST(_1 - 1 AS STRING), '.0') AS DOUBLE)" checkSparkAnswerAndFallbackReason( s"SELECT _1 AS id, element_at(map(CAST(0 AS DOUBLE), 7), $lookup) AS v FROM tbl", - "Spark normalizes floating-point map keys") + "Spark looks a key up in its SQL ordering") } }