From dfbf000dc142fb54cf1b8c68d3734fd47d52816f Mon Sep 17 00:00:00 2001 From: Dustin Smith Date: Sat, 12 Sep 2026 00:07:45 +0700 Subject: [PATCH 1/5] fix: dispatch map lookups with normalized keys and nondeterministic null-guarded children Map lookups with float, collated or complex keys were declined by the serde without a dispatch fallback, so the whole projection fell back to Spark. Four array and map serdes reproduce NULL propagation with a guard that serializes the child twice, so a stateful child was evaluated twice natively and returned wrong answers. Mix the codegen dispatch fallback into the map lookups, and decline any nondeterministic child in size, array_append, arrays_zip and map_from_arrays through one shared gate so Spark's generated code evaluates it once. Closes #5580 Closes #5781 --- docs/source/user-guide/latest/expressions.md | 12 +-- .../scala/org/apache/comet/serde/arrays.scala | 76 ++++++++++++++----- .../scala/org/apache/comet/serde/maps.scala | 26 +++++-- .../array_append_nondeterministic_child.sql | 55 ++++++++++++++ .../arrays_zip_nondeterministic_child.sql | 51 +++++++++++++ .../expressions/array/element_at_ansi.sql | 8 +- .../array/size_nondeterministic_child.sql | 47 ++++++++++++ .../expressions/map/element_at_map.sql | 42 +++++++--- .../map/element_at_map_collation.sql | 7 +- .../expressions/map/get_map_value.sql | 36 ++++++++- .../map/map_from_arrays_dedup_policy.sql | 8 +- ...map_from_arrays_nondeterministic_child.sql | 51 +++++++++++++ .../comet/CometMapExpressionSuite.scala | 59 ++++++++------ 13 files changed, 401 insertions(+), 77 deletions(-) create mode 100644 spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql create mode 100644 spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql create mode 100644 spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql create mode 100644 spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index 208a5f3f124..b41a704088e 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -148,7 +148,7 @@ The tables below list every Spark built-in expression with its current status. | Function | Status | Implementation | Notes | | --- | --- | --- | --- | | `array` | ✅ | Native | | -| `array_append` | ✅ | Native | | +| `array_append` | ✅ | Hybrid | | | `array_compact` | ✅ | — | | | `array_contains` | ✅ | Native | NaN/signed-zero handling may differ ([details](compatibility/floating-point.md)) | | `array_distinct` | ✅ | Native | NaN/signed-zero handling may differ ([details](compatibility/floating-point.md)) | @@ -164,8 +164,8 @@ The tables below list every Spark built-in expression with its current status. | `array_repeat` | ✅ | Native | | | `array_union` | ✅ | Native | NaN/signed-zero handling may differ ([details](compatibility/floating-point.md)) | | `arrays_overlap` | ✅ | Native | | -| `arrays_zip` | ✅ | Native | | -| `element_at` | ✅ | Native | | +| `arrays_zip` | ✅ | Hybrid | | +| `element_at` | ✅ | Hybrid | | | `flatten` | ✅ | Native | Binary/struct/map elements fall back | | `get` | ✅ | — | | | `sequence` | ✅ | Hybrid | Integral types run natively; date/timestamp sequences use codegen dispatch | @@ -202,7 +202,7 @@ The tables below list every Spark built-in expression with its current status. | `cardinality` | ✅ | Native | | | `concat` | ✅ | Hybrid | Binary/array children fall back | | `reverse` | ✅ | Hybrid | Binary-element arrays fall back (Incompatible) ([details](compatibility/expressions/array.md)) | -| `size` | ✅ | Native | | +| `size` | ✅ | Hybrid | | --- @@ -395,12 +395,12 @@ The type-name conversion functions (`bigint`, `binary`, `boolean`, `date`, `deci | Function | Status | Implementation | Notes | | --- | --- | --- | --- | -| `element_at` | ✅ | Native | | +| `element_at` | ✅ | Hybrid | | | `map` | ✅ | Codegen dispatch | Routed through the JVM codegen dispatcher | | `map_concat` | ✅ | Codegen dispatch | | | `map_contains_key` | ✅ | — | | | `map_entries` | ✅ | Native | | -| `map_from_arrays` | ✅ | Native | | +| `map_from_arrays` | ✅ | Hybrid | | | `map_from_entries` | ✅ | Hybrid | BinaryType key/value falls back (Incompatible) ([details](compatibility/expressions/map.md)) | | `map_keys` | ✅ | Native | | | `map_values` | ✅ | Native | | diff --git a/spark/src/main/scala/org/apache/comet/serde/arrays.scala b/spark/src/main/scala/org/apache/comet/serde/arrays.scala index cca9f63f8bf..be725f1583b 100644 --- a/spark/src/main/scala/org/apache/comet/serde/arrays.scala +++ b/spark/src/main/scala/org/apache/comet/serde/arrays.scala @@ -51,7 +51,37 @@ object CometArrayRemove } } -object CometArrayAppend extends CometExpressionSerde[ArrayAppend] with ArraysBase { +/** + * Shared gate for serdes whose native NULL guard (`CASE WHEN child IS NOT NULL`) serializes the + * child twice: a stateful child drifts between the two copies, so it is declined and runs through + * the JVM codegen dispatcher, where Spark evaluates it once. Nullability is not consulted: a + * non-nullable stateful child only stays in step because DataFusion skips the filter when the + * guard matches every row, which is not a contract to lean on. + */ +private[serde] object NullGuardSupport { + + val nondeterministicReason: String = + "a nondeterministic operand: the native NULL guard serializes the operand twice, " + + "and the two copies of a stateful operand drift apart" + + /** `Unsupported` when any of `children` is nondeterministic, otherwise `None`. */ + def nondeterministicChild(children: Seq[Expression]): Option[SupportLevel] = + children + .find(child => !child.deterministic) + .map(_ => Unsupported(Some(nondeterministicReason))) +} + +object CometArrayAppend + extends CometExpressionSerde[ArrayAppend] + with ArraysBase + with CodegenDispatchFallback { + + override def getUnsupportedReasons(): Seq[String] = + Seq(NullGuardSupport.nondeterministicReason) + + // Only the array operand sits under the NULL guard, so only it needs the decline. + override def getSupportLevel(expr: ArrayAppend): SupportLevel = + NullGuardSupport.nondeterministicChild(Seq(expr.children.head)).getOrElse(Compatible()) override def convert( expr: ArrayAppend, @@ -614,7 +644,7 @@ object CometArrayReverse extends CometExpressionSerde[Reverse] with ArraysBase { } -object CometElementAt extends CometExpressionSerde[ElementAt] { +object CometElementAt extends CometExpressionSerde[ElementAt] with CodegenDispatchFallback { /** * Under ANSI, neither native shape reproduces Spark for a nullable nondeterministic operand. @@ -623,8 +653,9 @@ object CometElementAt extends CometExpressionSerde[ElementAt] { * whole batch first, so a throwing index fires on rows whose operand is NULL. `convert` * reproduces the short-circuit with a `CASE WHEN IS NOT NULL` guard, but that guard * serializes the operand twice, which a stateful operand cannot survive: the two copies advance - * its state independently and silently move values and NULLs. Declining leaves the lookup on - * Spark. Lifting this needs a native lookup that evaluates the operand once and masks the index + * its state independently and silently move values and NULLs. Declining routes the lookup + * through the JVM codegen dispatcher, where Spark's own `doGenCode` evaluates the operand once. + * Lifting this needs a native lookup that evaluates the operand once and masks the index * evaluation with the result, at which point the guard becomes unnecessary for every operand. */ private val eagerIndexReason: String = @@ -635,6 +666,8 @@ object CometElementAt extends CometExpressionSerde[ElementAt] { private def needsNullGuard(expr: ElementAt): Boolean = expr.failOnError && expr.left.nullable + override def getUnsupportedReasons(): Seq[String] = eagerIndexReason +: MapKeySupport.reasons + override def getSupportLevel(expr: ElementAt): SupportLevel = { if (needsNullGuard(expr) && !expr.left.deterministic) { Unsupported(Some(eagerIndexReason)) @@ -764,14 +797,19 @@ object CometArrayFilter extends CometExpressionSerde[ArrayFilter] { } } -object CometSize extends CometExpressionSerde[Size] { +object CometSize extends CometExpressionSerde[Size] with CodegenDispatchFallback { + + override def getUnsupportedReasons(): Seq[String] = + Seq(NullGuardSupport.nondeterministicReason) override def getSupportLevel(expr: Size): SupportLevel = { - expr.child.dataType match { - case _: ArrayType => Compatible() - case _: MapType => Compatible() - case other => - Unsupported(Some(s"Unsupported child data type: $other")) + NullGuardSupport.nondeterministicChild(Seq(expr.child)).getOrElse { + expr.child.dataType match { + case _: ArrayType => Compatible() + case _: MapType => Compatible() + case other => + Unsupported(Some(s"Unsupported child data type: $other")) + } } } @@ -843,10 +881,12 @@ object CometArrayPosition extends CometExpressionSerde[ArrayPosition] with Array } } -object CometArraysZip extends CometExpressionSerde[ArraysZip] { +object CometArraysZip extends CometExpressionSerde[ArraysZip] with CodegenDispatchFallback { override def getUnsupportedReasons(): Seq[String] = Seq( - "Not all input data types are supported; falls back to Spark for unsupported types") + "Not all input data types are supported; unsupported types run through the JVM codegen " + + "dispatcher", + NullGuardSupport.nondeterministicReason) private def isTypeSupported(dt: DataType): Boolean = { import DataTypes._ @@ -862,13 +902,13 @@ object CometArraysZip extends CometExpressionSerde[ArraysZip] { } override def getSupportLevel(expr: ArraysZip): SupportLevel = { - val inputTypes = expr.children.map(_.dataType).toSet - for (dt <- inputTypes) { - if (!isTypeSupported(dt)) { - return Unsupported(Some(s"Unsupported child data type: $dt")) - } + NullGuardSupport.nondeterministicChild(expr.children).getOrElse { + expr.children + .map(_.dataType) + .collectFirst { case dt if !isTypeSupported(dt) => dt } + .map(dt => Unsupported(Some(s"Unsupported child data type: $dt"))) + .getOrElse(Compatible()) } - Compatible() } override def convert( 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 51fa428b543..5dd2842553a 100644 --- a/spark/src/main/scala/org/apache/comet/serde/maps.scala +++ b/spark/src/main/scala/org/apache/comet/serde/maps.scala @@ -29,7 +29,8 @@ import org.apache.comet.shims.CometTypeShim /** * Shared gate for the native map kernels that compare a lookup key against a map's stored keys - * (`map_extract`, reached from both `GetMapValue` and `ElementAt`). + * (`map_extract`, reached from both `GetMapValue` and `ElementAt`). Both consumers mix in + * `CodegenDispatchFallback`, so a declined key type runs through Spark's own `doGenCode`. */ private[serde] object MapKeySupport { @@ -46,6 +47,8 @@ private[serde] object MapKeySupport { "cannot reproduce Spark's equality for a complex key type (for example a `NULL` inside the " + "lookup key aborts the cast against a non-nullable nested component)." + val reasons: Seq[String] = Seq(floatingPointReason, collationReason, complexKeyReason) + /** * The `SupportLevel` for a map-consuming expression whose stored-key type is `keyType`. Spark * finds a key with `TypeUtils.getInterpretedOrdering` over the keys `ArrayBasedMapBuilder` @@ -114,7 +117,9 @@ object CometMapValues extends CometExpressionSerde[MapValues] { } } -object CometMapExtract extends CometExpressionSerde[GetMapValue] { +object CometMapExtract extends CometExpressionSerde[GetMapValue] with CodegenDispatchFallback { + + override def getUnsupportedReasons(): Seq[String] = MapKeySupport.reasons override def getSupportLevel(expr: GetMapValue): SupportLevel = expr.child.dataType match { case MapType(keyType, _, _) => MapKeySupport.keySupport(keyType) @@ -151,19 +156,26 @@ 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) + override def getUnsupportedReasons(): Seq[String] = + Seq(NullGuardSupport.nondeterministicReason) + override def getCompatibleNotes(): Seq[String] = Seq(MapKeyDedupPolicySupport.nullKeyReason) override def getSupportLevel(expr: MapFromArrays): SupportLevel = { - if (MapKeyDedupPolicySupport.isLastWin) { - Incompatible(Some(MapKeyDedupPolicySupport.incompatibleReason)) - } else { - Compatible(None) + NullGuardSupport.nondeterministicChild(expr.children).getOrElse { + if (MapKeyDedupPolicySupport.isLastWin) { + Incompatible(Some(MapKeyDedupPolicySupport.incompatibleReason)) + } else { + Compatible(None) + } } } diff --git a/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql new file mode 100644 index 00000000000..4b608f0480f --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql @@ -0,0 +1,55 @@ +-- 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. + +-- `CometArrayAppend` reproduces Spark's NULL propagation with a `CASE WHEN array IS NOT NULL` +-- guard that serializes the array twice. A stateful array advances each copy independently, so +-- the guard and the `array_append` see different rows and the result silently drifts from Spark. +-- The serde declines a nondeterministic array and routes it through the JVM codegen +-- dispatcher, which evaluates it once. A deterministic nullable array keeps the native guard. +-- +-- Spark 4.0 rewrites `array_append` to `array_insert(-1)` before serde, so `CometArrayAppend` is +-- only reachable on Spark 3.x. + +-- MaxSparkVersion: 3.5 + +statement +CREATE TABLE test_array_append_nondet(_1 int) USING parquet + +statement +INSERT INTO test_array_append_nondet VALUES + (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) + +-- Spark returns [1, 2] on every row whose array is non-NULL and NULL on the rest. +query expect_dispatch(array_append) +SELECT _1, array_append(IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY)), 2) AS a +FROM test_array_append_nondet + +-- A deterministic nullable array stays on the native guarded path. +-- A non-nullable stateful array is declined too, rather than relying on the guard matching +-- every row. +query expect_dispatch(array_append) +SELECT _1, array_append(array(monotonically_increasing_id()), 2) AS a +FROM test_array_append_nondet + +-- Only the array operand sits under the guard; a stateful item stays native. +query expect_native(array_append) +SELECT _1, array_append(array(1), monotonically_increasing_id()) AS a +FROM test_array_append_nondet + +query expect_native(array_append) +SELECT _1, array_append(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), 2) AS a +FROM test_array_append_nondet diff --git a/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql new file mode 100644 index 00000000000..273205ce2d1 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql @@ -0,0 +1,51 @@ +-- 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. + +-- `CometArraysZip` reproduces Spark's NULL propagation with a `CASE WHEN` guard over every +-- child's `IS NOT NULL`, which serializes each child twice. A stateful child advances each copy +-- independently, so the guard and the `arrays_zip` see different rows and the result silently +-- drifts from Spark. The serde declines a nondeterministic child and routes it through +-- the JVM codegen dispatcher, which evaluates it once. A deterministic nullable child keeps the +-- native guard. + +statement +CREATE TABLE test_arrays_zip_nondet(_1 int) USING parquet + +statement +INSERT INTO test_arrays_zip_nondet VALUES + (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) + +-- Spark returns [{1, 2}] on every row whose first array is non-NULL and NULL on the rest. +query expect_dispatch(arrays_zip) +SELECT _1, arrays_zip(IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY)), array(2)) AS z +FROM test_arrays_zip_nondet + +-- The guard covers every child, so a stateful second child is declined the same way. +query expect_dispatch(arrays_zip) +SELECT _1, arrays_zip(array(2), IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY))) AS z +FROM test_arrays_zip_nondet + +-- A deterministic nullable child stays on the native guarded path. +-- A non-nullable stateful child is declined too, rather than relying on the guard matching +-- every row. +query expect_dispatch(arrays_zip) +SELECT _1, arrays_zip(array(monotonically_increasing_id()), array(2)) AS z +FROM test_arrays_zip_nondet + +query expect_native(arrays_zip) +SELECT _1, arrays_zip(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), array(2)) AS z +FROM test_arrays_zip_nondet diff --git a/spark/src/test/resources/sql-tests/expressions/array/element_at_ansi.sql b/spark/src/test/resources/sql-tests/expressions/array/element_at_ansi.sql index 30ab1bba587..0965dfff87b 100644 --- a/spark/src/test/resources/sql-tests/expressions/array/element_at_ansi.sql +++ b/spark/src/test/resources/sql-tests/expressions/array/element_at_ansi.sql @@ -101,14 +101,16 @@ query SELECT id, element_at(CASE WHEN id <> 2 THEN array(1) END, 1) AS v FROM ansi_element_at_null --- A nondeterministic operand is declined outright. Neither native shape reproduces Spark: the +-- A nondeterministic operand has no native shape that reproduces Spark: the -- `CASE WHEN IS NOT NULL` guard serializes the operand twice, so a stateful operand's two -- copies drift, and the unguarded lookup evaluates the index over the whole batch, raising --- DIVIDE_BY_ZERO at id = 2 on the very row whose array is NULL. `rand(7L) < 2` is always true, so +-- DIVIDE_BY_ZERO at id = 2 on the very row whose array is NULL. The serde declines it and the +-- lookup runs through the JVM codegen dispatcher, where Spark's own `ElementAt.doGenCode` +-- evaluates the operand once and short-circuits the index. `rand(7L) < 2` is always true, so -- both operands are NULL on every row and Spark returns NULL without evaluating either index. -- The non-ANSI spelling stays native and is covered in element_at.sql. -- https://github.com/apache/datafusion-comet/issues/5544 -query expect_fallback(nullable nondeterministic array or map operand) +query expect_dispatch(element_at) SELECT id, element_at(IF(monotonically_increasing_id() % 2 = 0, CAST(NULL AS ARRAY), array(1)), 1) AS v1, element_at(IF(rand(7L) < 2, CAST(NULL AS ARRAY), array(1)), 1 + (id % (id - 2))) AS v2 diff --git a/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql new file mode 100644 index 00000000000..f91cc3784ea --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql @@ -0,0 +1,47 @@ +-- 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. + +-- `CometSize` reproduces Spark's NULL propagation with a `CASE WHEN child IS NOT NULL` guard +-- that serializes the child twice. A stateful child advances each copy independently, so the +-- guard and the `size` see different rows and the result silently drifts from Spark. The serde +-- declines any nondeterministic child and routes it through the JVM codegen dispatcher, which +-- evaluates the child once. A deterministic nullable child keeps the native guard. + +-- ConfigMatrix: spark.sql.legacy.sizeOfNull=true,false + +statement +CREATE TABLE test_size_nondet(_1 int) USING parquet + +statement +INSERT INTO test_size_nondet VALUES + (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) + +-- Spark returns 1 on every row whose array is non-NULL and the sentinel on the rest. +query expect_dispatch(size) +SELECT _1, size(IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY))) AS s +FROM test_size_nondet + +-- A non-nullable stateful child is declined too, rather than relying on the guard matching +-- every row. +query expect_dispatch(size) +SELECT _1, size(array(monotonically_increasing_id())) AS s +FROM test_size_nondet + +-- A deterministic nullable child stays on the native guarded path. +query expect_native(size) +SELECT _1, size(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY))) AS s +FROM test_size_nondet 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..4d906627441 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 @@ -52,31 +52,55 @@ SELECT element_at(mi, CAST(1 AS BIGINT)), element_at(mi, CAST(2 AS SMALLINT)) FR query SELECT element_at(map('a', 1, 'b', 2), 'a'), element_at(map('a', 1, 'b', 2), 'missing'), element_at(map('a', 1, 'b', 2), NULL) --- Map key types whose Spark equality Comet's native `map_extract` cannot reproduce fall back to --- Spark. These stay on the constructor path here because the SQL harness excludes --- `ConstantFolding`; `CometMapExpressionSuite` covers the folded-literal form of each. +-- Map key types whose Spark equality Comet's native `map_extract` cannot reproduce are routed +-- through the JVM codegen dispatcher, where Spark's own `ElementAt.doGenCode` supplies the key +-- normalization and interpreted-ordering equality. 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) +query expect_dispatch(element_at) 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_dispatch(element_at) SELECT element_at(map(CAST(0 AS FLOAT), 7), CAST(-0.0 AS FLOAT)) +-- Spark's `nanSafeCompareDoubles` treats NaN as equal to itself, so a NaN lookup finds a NaN key. +query expect_dispatch(element_at) +SELECT element_at(map(CAST('NaN' AS DOUBLE), 7), CAST('NaN' AS DOUBLE)) + -- 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) +-- is dispatched for the same reason. +query expect_dispatch(element_at) 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 -- NULL inside the lookup key would abort the cast instead of missing the lookup. -query expect_fallback(casts the lookup key to the map's exact Arrow key type) +query expect_dispatch(element_at) SELECT element_at(map(array(1), 7), array(CAST(NULL AS INT))) -query expect_fallback(casts the lookup key to the map's exact Arrow key type) +query expect_dispatch(element_at) SELECT element_at(map(named_struct('a', 1), 7), named_struct('a', 1)) +-- A struct-keyed map column with a per-row lookup key, including a NULL inside the lookup struct +-- and a NULL key. +statement +CREATE TABLE test_element_at_struct_key(m map, int>, k int) USING parquet + +statement +INSERT INTO test_element_at_struct_key VALUES + (map(named_struct('a', 1, 'b', 'x'), 7), 1), + (map(named_struct('a', 2, 'b', 'y'), 8), 3), + (map(named_struct('a', 3, 'b', 'z'), 9), NULL), + (NULL, 1) + +query expect_dispatch(element_at) +SELECT element_at(m, named_struct('a', k, 'b', 'x')), + element_at(m, named_struct('a', k, 'b', CAST(NULL AS STRING))), + element_at(m, CAST(NULL AS STRUCT)) +FROM test_element_at_struct_key + -- `BinaryType` keys need no decline: Arrow compares them by content, as Spark's ordering does. query SELECT element_at(map(CAST('a' AS BINARY), 1, CAST('b' AS BINARY), 2), CAST('b' AS BINARY)) diff --git a/spark/src/test/resources/sql-tests/expressions/map/element_at_map_collation.sql b/spark/src/test/resources/sql-tests/expressions/map/element_at_map_collation.sql index 763e1b31f12..6c690e2b435 100644 --- a/spark/src/test/resources/sql-tests/expressions/map/element_at_map_collation.sql +++ b/spark/src/test/resources/sql-tests/expressions/map/element_at_map_collation.sql @@ -19,8 +19,9 @@ -- Spark 4.0+ supports string collations. Spark compares a map key under its declared collation, -- so a `UTF8_LCASE` map matches `A1` against a dynamic `a1` lookup and returns `7`. Comet's native --- `map_extract` compares string keys as `UTF8_BINARY`, so `CometElementAt` declines the lookup and --- Spark evaluates the projection. The `element_at` form is used rather than `m[key]` because +-- `map_extract` compares string keys as `UTF8_BINARY`, so `CometElementAt` declines the native +-- lookup and routes it through the JVM codegen dispatcher, where Spark's own `ElementAt.doGenCode` +-- compares under the declared collation. The `element_at` form is used rather than `m[key]` because -- Spark's `SimplifyExtractValueOps` rewrites `map(...)[key]` over a literal map into a `CASE` -- before it can reach the native map lookup. @@ -30,6 +31,6 @@ CREATE TABLE test_element_at_collation(k string) USING parquet statement INSERT INTO test_element_at_collation VALUES ('a1'), ('A1'), ('zz'), (NULL) -query expect_fallback(cannot honour a non-default collation) +query expect_dispatch(element_at) SELECT element_at(map(CAST('A1' AS STRING COLLATE UTF8_LCASE), 7), CAST(k AS STRING COLLATE UTF8_LCASE)) FROM test_element_at_collation 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..e0459da4b36 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 @@ -31,9 +31,11 @@ SELECT m['x'], m['missing'] FROM test_map query spark_answer_only SELECT map('a', 1, 'b', 2)['a'], map('a', 1, 'b', 2)['missing'], map('a', 1, 'b', 2)[NULL] --- Map key types whose Spark equality Comet's native `map_extract` cannot reproduce fall back to --- Spark. `x[key]` routes through `GetMapValue` / `CometMapExtract` (the `element_at` form in --- `element_at_map.sql` exercises the same guard on `CometElementAt`). A `map` column +-- Map key types whose Spark equality Comet's native `map_extract` cannot reproduce are routed +-- through the JVM codegen dispatcher, where Spark's own `GetMapValue.doGenCode` supplies the key +-- normalization and interpreted-ordering equality. `x[key]` routes through `GetMapValue` / +-- `CometMapExtract` (the `element_at` form in `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 @@ -44,5 +46,31 @@ 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_dispatch(getmapvalue) SELECT m[CAST(-0.0 AS DOUBLE)] FROM test_map_double + +-- A NaN key is found by a NaN lookup, as `nanSafeCompareDoubles` treats NaN as equal to itself. +statement +INSERT INTO test_map_double VALUES (map(CAST('NaN' AS DOUBLE), 9)) + +query expect_dispatch(getmapvalue) +SELECT m[CAST('NaN' AS DOUBLE)] FROM test_map_double + +-- A struct-keyed map column with a per-row lookup key, including a NULL inside the lookup struct +-- and a NULL key. Native `map_extract` would cast the lookup key to the map's exact Arrow key type, +-- which cannot reproduce Spark's equality for a complex key, so the lookup is dispatched. +statement +CREATE TABLE test_map_struct_key(m map, int>, k int) USING parquet + +statement +INSERT INTO test_map_struct_key VALUES + (map(named_struct('a', 1, 'b', 'x'), 7), 1), + (map(named_struct('a', 2, 'b', 'y'), 8), 3), + (map(named_struct('a', 3, 'b', 'z'), 9), NULL), + (NULL, 1) + +query expect_dispatch(getmapvalue) +SELECT m[named_struct('a', k, 'b', 'x')], + m[named_struct('a', k, 'b', CAST(NULL AS STRING))], + m[CAST(NULL AS STRUCT)] +FROM test_map_struct_key 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..16974885219 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,14 @@ -- 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 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_arrays_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql new file mode 100644 index 00000000000..fd7491a0dbc --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql @@ -0,0 +1,51 @@ +-- 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. + +-- `CometMapFromArrays` reproduces Spark's NULL propagation with a `CASE WHEN keys IS NOT NULL AND +-- values IS NOT NULL` guard that serializes both children twice. A stateful child advances each +-- copy independently, so the guard and the `map` see different rows and the result silently +-- drifts from Spark. The serde declines a nondeterministic child and routes it through +-- the JVM codegen dispatcher, which evaluates it once. A deterministic nullable child keeps the +-- native guard. + +statement +CREATE TABLE test_map_from_arrays_nondet(_1 int) USING parquet + +statement +INSERT INTO test_map_from_arrays_nondet VALUES + (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) + +-- Spark returns {1 -> 2} on every row whose keys array is non-NULL and NULL on the rest. +query expect_dispatch(map_from_arrays) +SELECT _1, map_from_arrays(IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY)), array(2)) AS m +FROM test_map_from_arrays_nondet + +-- The guard covers both children, so a stateful values array is declined the same way. +query expect_dispatch(map_from_arrays) +SELECT _1, map_from_arrays(array(1), IF(monotonically_increasing_id() % 2 = 0, array(2), CAST(NULL AS ARRAY))) AS m +FROM test_map_from_arrays_nondet + +-- A deterministic nullable child stays on the native guarded path. +-- A non-nullable stateful child is declined too, rather than relying on the guard matching +-- every row. +query expect_dispatch(map_from_arrays) +SELECT _1, map_from_arrays(array(monotonically_increasing_id()), array(2)) AS m +FROM test_map_from_arrays_nondet + +query expect_native(map_from_arrays) +SELECT _1, map_from_arrays(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), array(2)) AS m +FROM test_map_from_arrays_nondet diff --git a/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala b/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala index ebbdce406a3..cab171d2927 100644 --- a/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala +++ b/spark/src/test/scala/org/apache/comet/CometMapExpressionSuite.scala @@ -299,30 +299,33 @@ class CometMapExpressionSuite extends CometTestBase { // Finding E: a map nested inside a map value is handed to the JVM dispatcher whole, so its double // keys never revisit `CometLiteral`. The guard has to live on the lookup instead. The outer key - // type is INT, so inspecting only the outermost map would admit the query; the fallback comes - // from the inner `element_at`, whose child is `MapType(DoubleType, IntegerType)`. Constant folding - // is on here, which is the only configuration where the inner map becomes such a literal, so this - // regression cannot be expressed in a SQL fixture (the harness disables folding). The direct - // single-level double-key lookups live in `element_at_map.sql` / `get_map_value.sql`. - test("nested map lookup with floating-point keys falls back") { + // type is INT, so inspecting only the outermost map would admit the query; the dispatch comes + // from the `element_at` whose child is `MapType(DoubleType, IntegerType)`, which takes the whole + // nested lookup with it. Constant folding is on here, which is the only configuration where the + // inner map becomes such a literal, so this regression cannot be expressed in a SQL fixture (the + // harness disables folding). The direct single-level double-key lookups live in + // `element_at_map.sql` / `get_map_value.sql`. + test("nested map lookup with floating-point keys is dispatched") { withParquetTable((1 until 4).map(i => (i, i.toLong)), "tbl") { val negZero = "CAST(concat('-', CAST(_1 - 1 AS STRING), '.0') AS DOUBLE)" - checkSparkAnswerAndFallbackReason( + checkSparkAnswerAndImpl( "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") + native = Seq.empty, + dispatched = Seq("element_at")) } } // Finding E for a collated inner key. Same folding-on-only bypass as the double-key case above; // the direct single-level collated lookup lives in `element_at_map_collation.sql`. - test("nested map lookup with collated string keys falls back") { + test("nested map lookup with collated string keys is dispatched") { assume(isSpark40Plus) withParquetTable(Seq(("a1", 0)), "tbl") { - checkSparkAnswerAndFallbackReason( + checkSparkAnswerAndImpl( "SELECT element_at(element_at(map(1, map(CAST('A1' AS STRING COLLATE UTF8_LCASE), 7)), 1), " + "CAST(_1 AS STRING COLLATE UTF8_LCASE)) AS v FROM tbl", - "cannot honour a non-default collation") + native = Seq.empty, + dispatched = Seq("element_at")) } } @@ -395,39 +398,45 @@ 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. - // 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)") { + // Native `map_extract` compares raw Arrow values, so `MapKeySupport` declines it at `element_at` + // and the lookup runs through the JVM codegen dispatcher. 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 is dispatched (multirow)") { withParquetTable((1 until 4).map(i => (i, i.toLong)), "tbl") { val lookup = "CAST(concat('-', CAST(_1 - 1 AS STRING), '.0') AS DOUBLE)" - checkSparkAnswerAndFallbackReason( + checkSparkAnswerAndImpl( s"SELECT _1 AS id, element_at(map(CAST(0 AS DOUBLE), 7), $lookup) AS v FROM tbl", - "Spark normalizes floating-point map keys") + native = Seq.empty, + dispatched = Seq("element_at")) } } // Direct single-level folded map with collated string keys. Native lookup is bytewise, so a - // case-insensitive `a1` lookup against a stored `A1` cannot match; `MapKeySupport` declines it. - test("folded map literal with collated string keys in element_at falls back (multirow)") { + // case-insensitive `a1` lookup against a stored `A1` cannot match; `MapKeySupport` declines it + // and the dispatcher compares under the declared collation. + test("folded map literal with collated string keys in element_at is dispatched (multirow)") { assume(isSpark40Plus) withParquetTable(Seq(("a1", 0)), "tbl") { - checkSparkAnswerAndFallbackReason( + checkSparkAnswerAndImpl( "SELECT element_at(map(CAST('A1' AS STRING COLLATE UTF8_LCASE), 7), " + "CAST(_1 AS STRING COLLATE UTF8_LCASE)) AS v FROM tbl", - "cannot honour a non-default collation") + native = Seq.empty, + dispatched = Seq("element_at")) } } // Folded map with a complex (array) key. Spark permits a dynamic lookup array containing a NULL // element; native `map_extract` casts the lookup to the key's exact Arrow type and cannot - // reproduce Spark's equality, so `MapKeySupport` declines every complex key type. Spark returns - // 7, NULL, NULL. (`element_at_map.sql` covers the constructor form with a non-null lookup.) - test("folded map literal with complex array key in element_at falls back (multirow)") { + // reproduce Spark's equality, so `MapKeySupport` declines every complex key type and the lookup + // is dispatched. Spark returns 7, NULL, NULL. (`element_at_map.sql` covers the constructor form + // with a non-null lookup.) + test("folded map literal with complex array key in element_at is dispatched (multirow)") { withParquetTable((1 until 4).map(i => (i, i.toLong)), "tbl") { - checkSparkAnswerAndFallbackReason( + checkSparkAnswerAndImpl( "SELECT _1 AS id, element_at(map(array(1), 7), " + "array(IF(_1 = 2, CAST(NULL AS INT), _1))) AS v FROM tbl", - "casts the lookup key to the map's exact Arrow key type") + native = Seq.empty, + dispatched = Seq("element_at")) } } From e79fcf85cd2037631556fefcf90820ccf60dd3ce Mon Sep 17 00:00:00 2001 From: Dustin Smith Date: Mon, 14 Sep 2026 21:27:44 +0700 Subject: [PATCH 2/5] test: pin the dispatched LAST_WIN route and read arrays from columns in the dispatched fixtures --- docs/source/user-guide/latest/expressions.md | 2 +- .../array/size_nondeterministic_child.sql | 12 ++++-- .../map/map_from_arrays_dedup_policy.sql | 23 +++++----- ...rom_arrays_dedup_policy_dispatcher_off.sql | 43 +++++++++++++++++++ ...map_from_arrays_nondeterministic_child.sql | 14 ++++-- 5 files changed, 74 insertions(+), 20 deletions(-) create mode 100644 spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy_dispatcher_off.sql diff --git a/docs/source/user-guide/latest/expressions.md b/docs/source/user-guide/latest/expressions.md index b41a704088e..b38e8c3cc87 100644 --- a/docs/source/user-guide/latest/expressions.md +++ b/docs/source/user-guide/latest/expressions.md @@ -199,7 +199,7 @@ The tables below list every Spark built-in expression with its current status. | Function | Status | Implementation | Notes | | --- | --- | --- | --- | | `array_size` | ✅ | — | | -| `cardinality` | ✅ | Native | | +| `cardinality` | ✅ | Hybrid | | | `concat` | ✅ | Hybrid | Binary/array children fall back | | `reverse` | ✅ | Hybrid | Binary-element arrays fall back (Incompatible) ([details](compatibility/expressions/array.md)) | | `size` | ✅ | Hybrid | | diff --git a/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql index f91cc3784ea..686aff2e878 100644 --- a/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql +++ b/spark/src/test/resources/sql-tests/expressions/array/size_nondeterministic_child.sql @@ -24,11 +24,11 @@ -- ConfigMatrix: spark.sql.legacy.sizeOfNull=true,false statement -CREATE TABLE test_size_nondet(_1 int) USING parquet +CREATE TABLE test_size_nondet(_1 int, arr array) USING parquet statement -INSERT INTO test_size_nondet VALUES - (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) +INSERT INTO test_size_nondet +SELECT id, IF(id % 4 = 3, NULL, array(id, id + 1)) FROM range(0, 16) -- Spark returns 1 on every row whose array is non-NULL and the sentinel on the rest. query expect_dispatch(size) @@ -45,3 +45,9 @@ FROM test_size_nondet query expect_native(size) SELECT _1, size(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY))) AS s FROM test_size_nondet + +-- The dispatched kernel reads the array from a column, so it copies a ListVector rather than +-- building the array inline. +query expect_dispatch(size) +SELECT _1, size(IF(monotonically_increasing_id() % 2 = 0, arr, CAST(NULL AS ARRAY))) AS s +FROM test_size_nondet 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 16974885219..d844980821f 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 @@ -15,16 +15,15 @@ -- specific language governing permissions and limitations -- under the License. --- 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. `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 default `EXCEPTION` mode agrees with Comet and is --- covered by `map_from_arrays.sql`. +-- Verifies that `map_from_arrays` leaves the native path when `spark.sql.mapKeyDedupPolicy` is +-- set to `LAST_WIN`. Spark's ArrayBasedMapBuilder keeps the last occurrence of each duplicate key +-- and Comet's native `map` scalar has no LAST_WIN path, so `CometMapFromArrays` reports the +-- policy as `Incompatible` and its `CodegenDispatchFallback` routes the call through the JVM +-- codegen dispatcher, which runs Spark's own builder. The dispatcher-off fallback is pinned by +-- `map_from_arrays_dedup_policy_dispatcher_off.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 @@ -35,11 +34,11 @@ INSERT INTO test_map_from_arrays_dedup VALUES (array('a', 'a', 'b'), array(1, 2, 3)), (array('x', 'x'), array(10, 20)) --- literal duplicate keys under LAST_WIN: Spark keeps the last value; Comet must fall back. -query expect_fallback(mapKeyDedupPolicy) +-- literal duplicate keys under LAST_WIN: Spark keeps the last value; the dispatcher runs it. +query expect_dispatch(map_from_arrays) SELECT map_from_arrays(array('a', 'a', 'b'), array(1, 2, 3)) --- column input falls back the same way; the incompat branch is triggered by the SQLConf value, +-- column input is dispatched the same way; the incompat branch is triggered by the SQLConf value, -- not per-row content. -query expect_fallback(mapKeyDedupPolicy) +query expect_dispatch(map_from_arrays) SELECT map_from_arrays(k, v) FROM test_map_from_arrays_dedup diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy_dispatcher_off.sql b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy_dispatcher_off.sql new file mode 100644 index 00000000000..51a3cebe321 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_dedup_policy_dispatcher_off.sql @@ -0,0 +1,43 @@ +-- 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. + +-- Pins the genuine Spark fallback for `map_from_arrays` under `spark.sql.mapKeyDedupPolicy=LAST_WIN` +-- when the JVM codegen dispatcher is off. Spark's ArrayBasedMapBuilder keeps the last occurrence +-- of each duplicate key and Comet's native `map` scalar has no LAST_WIN path, so the serde's +-- `Incompatible` branch surfaces as a fallback. The dispatched route is covered by +-- `map_from_arrays_dedup_policy.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 + +statement +INSERT INTO test_map_from_arrays_dedup VALUES + (array('a', 'b', 'c'), array(1, 2, 3)), + (array('a', 'a', 'b'), array(1, 2, 3)), + (array('x', 'x'), array(10, 20)) + +-- literal duplicate keys under LAST_WIN: Spark keeps the last value; Comet must fall back. +query expect_fallback(mapKeyDedupPolicy) +SELECT map_from_arrays(array('a', 'a', 'b'), array(1, 2, 3)) + +-- column input falls back the same way; the incompat branch is triggered by the SQLConf value, +-- not per-row content. +query expect_fallback(mapKeyDedupPolicy) +SELECT map_from_arrays(k, v) FROM test_map_from_arrays_dedup diff --git a/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql index fd7491a0dbc..a28813c1c67 100644 --- a/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql +++ b/spark/src/test/resources/sql-tests/expressions/map/map_from_arrays_nondeterministic_child.sql @@ -23,11 +23,11 @@ -- native guard. statement -CREATE TABLE test_map_from_arrays_nondet(_1 int) USING parquet +CREATE TABLE test_map_from_arrays_nondet(_1 int, k array, v array) USING parquet statement -INSERT INTO test_map_from_arrays_nondet VALUES - (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) +INSERT INTO test_map_from_arrays_nondet +SELECT id, IF(id % 4 = 3, NULL, array(id, id + 100)), array(id * 2, id * 2 + 1) FROM range(0, 16) -- Spark returns {1 -> 2} on every row whose keys array is non-NULL and NULL on the rest. query expect_dispatch(map_from_arrays) @@ -39,13 +39,19 @@ query expect_dispatch(map_from_arrays) SELECT _1, map_from_arrays(array(1), IF(monotonically_increasing_id() % 2 = 0, array(2), CAST(NULL AS ARRAY))) AS m FROM test_map_from_arrays_nondet --- A deterministic nullable child stays on the native guarded path. -- A non-nullable stateful child is declined too, rather than relying on the guard matching -- every row. query expect_dispatch(map_from_arrays) SELECT _1, map_from_arrays(array(monotonically_increasing_id()), array(2)) AS m FROM test_map_from_arrays_nondet +-- The dispatched kernel reads both arrays from columns, so it copies ListVectors rather than +-- building the arrays inline. +query expect_dispatch(map_from_arrays) +SELECT _1, map_from_arrays(IF(monotonically_increasing_id() % 2 = 0, k, CAST(NULL AS ARRAY)), v) AS m +FROM test_map_from_arrays_nondet + +-- A deterministic nullable child stays on the native guarded path. query expect_native(map_from_arrays) SELECT _1, map_from_arrays(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), array(2)) AS m FROM test_map_from_arrays_nondet From d49a2fbfbc87469017e4c8109ae388c75834ec54 Mon Sep 17 00:00:00 2001 From: Dustin Smith Date: Mon, 14 Sep 2026 22:10:53 +0700 Subject: [PATCH 3/5] fix: decline a nondeterministic item in array_append, since the native guard filters its evaluation --- .../scala/org/apache/comet/serde/arrays.scala | 6 ++++-- .../array_append_nondeterministic_child.sql | 19 ++++++++++++++----- 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/spark/src/main/scala/org/apache/comet/serde/arrays.scala b/spark/src/main/scala/org/apache/comet/serde/arrays.scala index be725f1583b..32a5ac69a2e 100644 --- a/spark/src/main/scala/org/apache/comet/serde/arrays.scala +++ b/spark/src/main/scala/org/apache/comet/serde/arrays.scala @@ -79,9 +79,11 @@ object CometArrayAppend override def getUnsupportedReasons(): Seq[String] = Seq(NullGuardSupport.nondeterministicReason) - // Only the array operand sits under the NULL guard, so only it needs the decline. + // The item sits inside the guard's THEN branch, and DataFusion's CaseExpr evaluates that + // branch only on the rows the guard selects, while Spark's codegen evaluates the item on + // every row. A stateful item therefore drifts the same way a stateful array does. override def getSupportLevel(expr: ArrayAppend): SupportLevel = - NullGuardSupport.nondeterministicChild(Seq(expr.children.head)).getOrElse(Compatible()) + NullGuardSupport.nondeterministicChild(expr.children).getOrElse(Compatible()) override def convert( expr: ArrayAppend, diff --git a/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql index 4b608f0480f..e7d68d1ccf1 100644 --- a/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql +++ b/spark/src/test/resources/sql-tests/expressions/array/array_append_nondeterministic_child.sql @@ -18,8 +18,10 @@ -- `CometArrayAppend` reproduces Spark's NULL propagation with a `CASE WHEN array IS NOT NULL` -- guard that serializes the array twice. A stateful array advances each copy independently, so -- the guard and the `array_append` see different rows and the result silently drifts from Spark. --- The serde declines a nondeterministic array and routes it through the JVM codegen --- dispatcher, which evaluates it once. A deterministic nullable array keeps the native guard. +-- The item sits inside the guard's THEN branch, which DataFusion evaluates only on the selected +-- rows while Spark evaluates it on every row, so a stateful item drifts too. The serde declines +-- either nondeterministic operand and routes it through the JVM codegen dispatcher, which +-- evaluates each once. A deterministic nullable array keeps the native guard. -- -- Spark 4.0 rewrites `array_append` to `array_insert(-1)` before serde, so `CometArrayAppend` is -- only reachable on Spark 3.x. @@ -38,18 +40,25 @@ query expect_dispatch(array_append) SELECT _1, array_append(IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY)), 2) AS a FROM test_array_append_nondet --- A deterministic nullable array stays on the native guarded path. -- A non-nullable stateful array is declined too, rather than relying on the guard matching -- every row. query expect_dispatch(array_append) SELECT _1, array_append(array(monotonically_increasing_id()), 2) AS a FROM test_array_append_nondet --- Only the array operand sits under the guard; a stateful item stays native. -query expect_native(array_append) +-- A stateful item over a nullable array: Spark advances the counter on all 16 rows, so the +-- even rows carry [1, 0], [1, 2], [1, 4] and so on, which the filtered native branch cannot +-- reproduce. +query expect_dispatch(array_append) +SELECT _1, array_append(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), monotonically_increasing_id()) AS a +FROM test_array_append_nondet + +-- A stateful item over a non-nullable array is declined the same way. +query expect_dispatch(array_append) SELECT _1, array_append(array(1), monotonically_increasing_id()) AS a FROM test_array_append_nondet +-- A deterministic nullable array stays on the native guarded path. query expect_native(array_append) SELECT _1, array_append(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), 2) AS a FROM test_array_append_nondet From 0d61cac1eda1643bf4eb2da85bd4f9985359b0c9 Mon Sep 17 00:00:00 2001 From: Dustin Smith Date: Tue, 22 Sep 2026 21:55:59 +0700 Subject: [PATCH 4/5] fix: route array_append through the dispatcher under ANSI and name the arrays_zip types With ANSI on, an item that raises on a row whose array is NULL raises in Spark but not on the native path, because the NULL guard skips the item there. The serde now reports that case as incompatible when the array is nullable, so it runs through the JVM codegen dispatcher by default and the native guard stays behind allowIncompatible. The divergence is recorded in the compatibility guide with its tracking issue, and a fixture pins the raise, the dispatch and the native path for a non-nullable array. The shared nondeterministic reason is now a full sentence for the generated guide, the arrays_zip type bullet names the element types the native kernel declines, and the arrays_zip fixture gets its comments in the right order plus a dispatched query over an array column. --- .../scala/org/apache/comet/serde/arrays.scala | 37 ++++++++++-- .../array/array_append_ansi_null_array.sql | 59 +++++++++++++++++++ .../arrays_zip_nondeterministic_child.sql | 15 +++-- 3 files changed, 101 insertions(+), 10 deletions(-) create mode 100644 spark/src/test/resources/sql-tests/expressions/array/array_append_ansi_null_array.sql diff --git a/spark/src/main/scala/org/apache/comet/serde/arrays.scala b/spark/src/main/scala/org/apache/comet/serde/arrays.scala index 32a5ac69a2e..ccb1e8065d7 100644 --- a/spark/src/main/scala/org/apache/comet/serde/arrays.scala +++ b/spark/src/main/scala/org/apache/comet/serde/arrays.scala @@ -61,8 +61,8 @@ object CometArrayRemove private[serde] object NullGuardSupport { val nondeterministicReason: String = - "a nondeterministic operand: the native NULL guard serializes the operand twice, " + - "and the two copies of a stateful operand drift apart" + "Comet has no native path for a nondeterministic operand such as `rand()` or " + + "`monotonically_increasing_id()`, because the native `NULL` guard would evaluate it twice." /** `Unsupported` when any of `children` is nondeterministic, otherwise `None`. */ def nondeterministicChild(children: Seq[Expression]): Option[SupportLevel] = @@ -79,11 +79,35 @@ object CometArrayAppend override def getUnsupportedReasons(): Seq[String] = Seq(NullGuardSupport.nondeterministicReason) + private val ansiItemNote: String = + "With `spark.sql.ansi.enabled=true` and a nullable array, the native `NULL` guard skips " + + "the item on a row whose array is `NULL`, so an item that raises there (for example a " + + "division by zero) raises in Spark but not on the native path. Such an expression runs " + + "through the JVM codegen dispatcher by default; enabling the native path can swallow " + + "that error ([#6086](https://github.com/apache/datafusion-comet/issues/6086))." + + override def getIncompatibleReasons(): Seq[String] = Seq(ansiItemNote) + + // Only the ANSI nullable-array case is routed through the dispatcher; every other compatible + // instance runs natively by default. + override def hasConditionalNativeDefault: Boolean = true + // The item sits inside the guard's THEN branch, and DataFusion's CaseExpr evaluates that // branch only on the rows the guard selects, while Spark's codegen evaluates the item on - // every row. A stateful item therefore drifts the same way a stateful array does. + // every row. A stateful item therefore drifts the same way a stateful array does, and is + // declined. The same shape lets an item that raises under ANSI mode go unevaluated on a row + // whose array is NULL, so Spark raises where the native path returns NULL. That case is + // reported as incompatible, which routes it through the JVM codegen dispatcher by default + // and reserves the native guard for allowIncompatible=true. A non-nullable array evaluates + // the item on every row on both paths, so it stays native. override def getSupportLevel(expr: ArrayAppend): SupportLevel = - NullGuardSupport.nondeterministicChild(expr.children).getOrElse(Compatible()) + NullGuardSupport.nondeterministicChild(expr.children).getOrElse { + if (SQLConf.get.ansiEnabled && expr.left.nullable) { + Incompatible(Some(ansiItemNote)) + } else { + Compatible() + } + } override def convert( expr: ArrayAppend, @@ -886,8 +910,9 @@ object CometArrayPosition extends CometExpressionSerde[ArrayPosition] with Array object CometArraysZip extends CometExpressionSerde[ArraysZip] with CodegenDispatchFallback { override def getUnsupportedReasons(): Seq[String] = Seq( - "Not all input data types are supported; unsupported types run through the JVM codegen " + - "dispatcher", + "An array whose element type is a map, a calendar, day-time or year-month interval, a " + + "variant, a `TIME` value or a user-defined type has no native `arrays_zip` kernel, and " + + "neither does a struct or inner array that holds one of those.", NullGuardSupport.nondeterministicReason) private def isTypeSupported(dt: DataType): Boolean = { diff --git a/spark/src/test/resources/sql-tests/expressions/array/array_append_ansi_null_array.sql b/spark/src/test/resources/sql-tests/expressions/array/array_append_ansi_null_array.sql new file mode 100644 index 00000000000..931ca843e58 --- /dev/null +++ b/spark/src/test/resources/sql-tests/expressions/array/array_append_ansi_null_array.sql @@ -0,0 +1,59 @@ +-- 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. + +-- `CometArrayAppend` reproduces Spark's NULL propagation with a `CASE WHEN array IS NOT NULL` +-- guard, and the item sits inside the guard's THEN branch. DataFusion evaluates that branch only +-- on the rows the guard selects, while Spark's codegen evaluates the item on every row. Under +-- ANSI mode an item that raises on a row whose array is NULL therefore raises in Spark and stays +-- silent on the native path. The serde reports a nullable array under ANSI as incompatible, so +-- by default the expression runs through the JVM codegen dispatcher, which raises like Spark. +-- A non-nullable array evaluates the item on every row on both paths, so it stays native. +-- +-- Spark 4.0 rewrites `array_append` to `array_insert(-1)` before serde, so `CometArrayAppend` is +-- only reachable on Spark 3.x. + +-- MaxSparkVersion: 3.5 + +-- Config: spark.sql.ansi.enabled=true + +statement +CREATE TABLE test_array_append_ansi(_1 int, arr array) USING parquet + +statement +INSERT INTO test_array_append_ansi +SELECT id, IF(id = 1, NULL, array(id)) FROM range(0, 4) + +-- The item divides by zero exactly on the row whose array is NULL, so Spark raises and the +-- dispatcher raises with it. +query expect_error(DIVIDE_BY_ZERO) +SELECT _1, array_append(arr, 1 / (_1 - 1)) AS a +FROM test_array_append_ansi + +-- The divide raises on its own, so the error above is not an artifact of the fixture. +query expect_error(DIVIDE_BY_ZERO) +SELECT _1, 1 / (_1 - 1) AS d +FROM test_array_append_ansi + +-- A nullable array under ANSI is routed through the dispatcher even when the item cannot raise. +query expect_dispatch(array_append) +SELECT _1, array_append(arr, _1) AS a +FROM test_array_append_ansi + +-- A non-nullable array literal cannot hit the gap and keeps the native guarded path. +query expect_native(array_append) +SELECT _1, array_append(array(1), _1) AS a +FROM test_array_append_ansi diff --git a/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql b/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql index 273205ce2d1..265eda1a32f 100644 --- a/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql +++ b/spark/src/test/resources/sql-tests/expressions/array/arrays_zip_nondeterministic_child.sql @@ -23,11 +23,11 @@ -- native guard. statement -CREATE TABLE test_arrays_zip_nondet(_1 int) USING parquet +CREATE TABLE test_arrays_zip_nondet(_1 int, arr array) USING parquet statement -INSERT INTO test_arrays_zip_nondet VALUES - (0), (1), (2), (3), (4), (5), (6), (7), (8), (9), (10), (11), (12), (13), (14), (15) +INSERT INTO test_arrays_zip_nondet +SELECT id, IF(id % 4 = 3, NULL, array(id, id + 1)) FROM range(0, 16) -- Spark returns [{1, 2}] on every row whose first array is non-NULL and NULL on the rest. query expect_dispatch(arrays_zip) @@ -39,13 +39,20 @@ query expect_dispatch(arrays_zip) SELECT _1, arrays_zip(array(2), IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY))) AS z FROM test_arrays_zip_nondet --- A deterministic nullable child stays on the native guarded path. -- A non-nullable stateful child is declined too, rather than relying on the guard matching -- every row. query expect_dispatch(arrays_zip) SELECT _1, arrays_zip(array(monotonically_increasing_id()), array(2)) AS z FROM test_arrays_zip_nondet +-- The dispatched kernel reads both arrays from a column, so it copies ListVectors into the +-- array> result rather than building the arrays inline. The column is NULL on every +-- fourth row, so the kernel also sees a NULL array that comes from the input, not from the IF. +query expect_dispatch(arrays_zip) +SELECT _1, arrays_zip(IF(monotonically_increasing_id() % 2 = 0, arr, CAST(NULL AS ARRAY)), arr) AS z +FROM test_arrays_zip_nondet + +-- A deterministic nullable child stays on the native guarded path. query expect_native(arrays_zip) SELECT _1, arrays_zip(IF(_1 % 2 = 0, array(1), CAST(NULL AS ARRAY)), array(2)) AS z FROM test_arrays_zip_nondet From 1b155533ae7d0e28c043a241cc60479be7c9ba0b Mon Sep 17 00:00:00 2001 From: Dustin Smith Date: Thu, 24 Sep 2026 00:39:20 +0700 Subject: [PATCH 5/5] test: look up a real negative zero in the map fixtures An unsuffixed -0.0 is a decimal literal with no signed zero, so CAST(-0.0 AS DOUBLE) looked up +0.0 and found the key on the native path too. The lookups now use -0.0D and CAST(-0.0D AS FLOAT), which keep the sign, so the fixtures fail on results when the float key decline is missing. The contributor guide names the literal form as well. --- docs/source/contributor-guide/sql-file-tests.md | 5 +++-- .../sql-tests/expressions/map/element_at_map.sql | 10 ++++++---- .../sql-tests/expressions/map/get_map_value.sql | 6 ++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/source/contributor-guide/sql-file-tests.md b/docs/source/contributor-guide/sql-file-tests.md index 57cc717547f..cb5d3b2e26d 100644 --- a/docs/source/contributor-guide/sql-file-tests.md +++ b/docs/source/contributor-guide/sql-file-tests.md @@ -360,8 +360,9 @@ common ones include: - **Signed zero** -- Spark parses a bare `-0.0` as `decimal(1,1)`, which has no signed zero, so coercion to `float`/`double` yields `+0.0`. `CAST(-0.0 AS DOUBLE)` and `CAST(-0.0 AS FLOAT)` have the same problem because the cast source is still the - decimal literal. Use `double('-0.0')` or `float('-0.0')` (equivalently - `CAST('-0.0' AS DOUBLE)`). Spark's array comparator also treats `+0.0` and `-0.0` as + decimal literal. Use the double literal `-0.0D`, `CAST(-0.0D AS FLOAT)` for a float, + or `double('-0.0')` / `float('-0.0')` (equivalently `CAST('-0.0' AS DOUBLE)`), all of + which keep the sign. Spark's array comparator also treats `+0.0` and `-0.0` as equal, so `sort_array(...)` is not a unique projection when both signs are present (the SQL test comparator distinguishes the bits). Prefer a sign-aware form such as `sort_array(transform(arr, x -> cast(x AS string)))`. A `query tolerance=...` check 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 4d906627441..6b7ed0fe74b 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 @@ -59,12 +59,14 @@ SELECT element_at(map('a', 1, 'b', 2), 'a'), element_at(map('a', 1, 'b', 2), 'mi -- 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. +-- lookup finds the `+0.0` key. Native lookup compares the raw Arrow values. The lookup must be +-- written `-0.0D`: an unsuffixed `-0.0` is a decimal literal, and a decimal has no signed zero, so +-- `CAST(-0.0 AS DOUBLE)` would look up `+0.0` and find the key on either path. query expect_dispatch(element_at) -SELECT element_at(map(CAST(0 AS DOUBLE), 7), CAST(-0.0 AS DOUBLE)) +SELECT element_at(map(CAST(0 AS DOUBLE), 7), -0.0D) query expect_dispatch(element_at) -SELECT element_at(map(CAST(0 AS FLOAT), 7), CAST(-0.0 AS FLOAT)) +SELECT element_at(map(CAST(0 AS FLOAT), 7), CAST(-0.0D AS FLOAT)) -- Spark's `nanSafeCompareDoubles` treats NaN as equal to itself, so a NaN lookup finds a NaN key. query expect_dispatch(element_at) @@ -73,7 +75,7 @@ SELECT element_at(map(CAST('NaN' AS DOUBLE), 7), CAST('NaN' AS DOUBLE)) -- The floating-point decline walks every nesting level of the key type, so an array-of-double key -- is dispatched for the same reason. query expect_dispatch(element_at) -SELECT element_at(map(array(CAST(0 AS DOUBLE)), 7), array(CAST(-0.0 AS DOUBLE))) +SELECT element_at(map(array(CAST(0 AS DOUBLE)), 7), array(-0.0D)) -- A complex key type: `map_extract` casts the lookup key to the map's exact Arrow key type, so a -- NULL inside the lookup key would abort the cast instead of missing the lookup. 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 e0459da4b36..13712c8fc29 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 @@ -39,7 +39,9 @@ SELECT map('a', 1, 'b', 2)['a'], map('a', 1, 'b', 2)['missing'], map('a', 1, 'b' -- 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. +-- `-0.0` lookup, while native lookup compares the raw Arrow values. The lookup must be written +-- `-0.0D`: an unsuffixed `-0.0` is a decimal literal, and a decimal has no signed zero, so +-- `CAST(-0.0 AS DOUBLE)` would look up `+0.0` and find the key on either path. statement CREATE TABLE test_map_double(m map) USING parquet @@ -47,7 +49,7 @@ statement INSERT INTO test_map_double VALUES (map(CAST(0 AS DOUBLE), 7)), (map(CAST(1 AS DOUBLE), 8)), (NULL) query expect_dispatch(getmapvalue) -SELECT m[CAST(-0.0 AS DOUBLE)] FROM test_map_double +SELECT m[-0.0D] FROM test_map_double -- A NaN key is found by a NaN lookup, as `nanSafeCompareDoubles` treats NaN as equal to itself. statement