sunchao commented on code in PR #5526:
URL: https://github.com/apache/datafusion-comet/pull/5526#discussion_r4125474663
##########
spark/src/main/scala/org/apache/comet/serde/maps.scala:
##########
@@ -159,11 +159,21 @@ object CometMapFromArrays extends
CometExpressionSerde[MapFromArrays] {
override def getCompatibleNotes(): Seq[String] =
Seq(MapKeyDedupPolicySupport.nullKeyReason)
+ private val scalarSideReason: String =
+ "native map takes the first row of a scalar list where the other argument
is per-row"
+
+ override def getUnsupportedReasons(): Seq[String] = Seq(scalarSideReason,
NullGuard.reason)
+
override def getSupportLevel(expr: MapFromArrays): SupportLevel = {
if (MapKeyDedupPolicySupport.isLastWin) {
Incompatible(Some(MapKeyDedupPolicySupport.incompatibleReason))
+ } else if (expr.left.foldable != expr.right.foldable) {
+ // DataFusion's `map` reads a scalar list argument through its first row
only, so a literal
+ // array beside a per-row one fails with "map requires key and value
lists to have the same
+ // length" as soon as the batch holds more than one row.
+ Unsupported(Some(scalarSideReason))
} else {
- Compatible(None)
+ NullGuard.supportLevel(expr.left, expr.right)
Review Comment:
[P2] Apply `NullGuard` before the `LAST_WIN` branch. With
`spark.sql.mapKeyDedupPolicy=LAST_WIN` and
`spark.comet.expression.MapFromArrays.allowIncompatible=true`, the earlier
`Incompatible` return bypasses this guard and permits duplicated stateful
evaluation. Over one Parquet batch containing IDs 0–3,
`map_from_arrays(IF(monotonically_increasing_id() % 2 = 0, array(id), NULL),
transform(array(id), x -> NULL))` should retain `{2: NULL}` at ID 2, but the
native CASE/map evaluation returns `NULL` there. There are no duplicate keys,
so this is unrelated to the documented deduplication incompatibility. The base
rejected the transform's `array<null>` output and retained Spark execution;
this PR newly admits the failing composition. Check nondeterminism before the
policy branch and add coverage with both settings enabled.
Evidence: Executed the SQL on Spark 4.1.3 using local[1] and
single-partition Parquet IDs 0–3: results were [{0: NULL}, NULL, {2: NULL},
NULL], and the optimized plan retained both expressions. A disposable Cargo
test reconstructed CometMapFromArrays.convert's native CASE/map tree using
exact-head production MonotonicallyIncreasingId and locked DataFusion
55.1.0/Arrow 59.2.0 dependencies. It produced [{0: NULL}, NULL, NULL, NULL],
failing the row-2 validity assertion. Source tracing confirms that LAST_WIN
returns before NullGuard and allowIncompatible calls convert directly.
Reproduction preserved at /tmp/review5526-1790617989-reproduction.rs. Full
Comet SQL execution was not available.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
