sunchao commented on issue #5544: URL: https://github.com/apache/datafusion-comet/issues/5544#issuecomment-5464398708
Two **P1 updates** from the final pass of #5452 at [`ec9832366`](https://github.com/apache/datafusion-comet/commit/ec9832366bf05b2c645f72c8fabeacfb7af48925), against base [`215ab706a`](https://github.com/apache/datafusion-comet/commit/215ab706aa9af074e589f400cd6843dee9938030). The UTF-8 case is new; the stateful `element_at` case below strengthens and raises the priority of the previously reported P2. **1. [P1] Newly admitted malformed UTF-8 map literals can abort the JVM** With normal constant folding, ANSI disabled, and `t(id INT)` read from one Parquet file containing `1, 2, 3`: ```sql SET spark.sql.ansi.enabled = false; SELECT id, levenshtein(element_at(map(1, CAST(X'FF' AS STRING)), id), 'a') AS v FROM t; ``` Spark returns `(1, 1), (2, NULL), (3, NULL)`. The current head selects `CometProject` and aborts the entire JVM with **SIGABRT**, rather than returning a query exception: ```text unsafe precondition(s) violated: hint::unreachable_unchecked must never be reached thread caused non-unwinding panic. aborting. ``` The [new literal admission](https://github.com/apache/datafusion-comet/blob/ec9832366bf05b2c645f72c8fabeacfb7af48925/spark/src/main/scala/org/apache/comet/serde/literals.scala#L295-L302) rebuilds the folded map without validating its string values. The [generated writer copies Spark's raw string bytes into Arrow](https://github.com/apache/datafusion-comet/blob/ec9832366bf05b2c645f72c8fabeacfb7af48925/spark/src/main/scala/org/apache/comet/codegen/CometBatchKernelCodegenOutput.scala#L243-L264), and the [native bridge imports them through unchecked Arrow FFI](https://github.com/apache/datafusion-comet/blob/ec9832366bf05b2c645f72c8fabeacfb7af48925/native/spark-expr/src/jvm_udf/mod.rs#L245-L252). Native character iteration then receives bytes that violate its valid-UTF-8 requirement. This also has a silent-result manifestation: `length(element_at(map(1, CAST(X'C080' AS STRING)), id))` returns `1` for `id = 1`, where Spark returns `2`; a binary round-trip control preserves `C080` on both paths. The underlying bridge hazard predates this PR: disabling ConstantFolding also exposes it with the base serializers. The regression here is admitting these folded queries under normal optimizer settings, where the base retains safe Spark evaluation. A narrow guard can decline expansion of literals containing malformed UTF-8 until the broader bridge/string handling is fixed. Replacing the bytes with normalized text would not preserve Spark's raw-byte semantics. **2. [P1] The ANSI `element_at` guard silently changes values and NULL placement for stateful operands** This is the existing duplicated-left-operand family, now reproduced with a simpler primitive-array query. For one Parquet file written with IDs `1` through `16` in order: ```sql SET spark.sql.ansi.enabled = true; SELECT id, element_at( IF(monotonically_increasing_id() % 2 = 0, array(1), CAST(NULL AS ARRAY<INT>)), 1) AS v FROM t; ``` Spark returns non-NULL values for all eight odd IDs. Comet returns non-NULL values only for IDs `1, 5, 9, 13`: **four expected values silently become NULL**. The simpler random variant, `element_at(IF(rand(7L) < 0.5, array(1), CAST(NULL AS ARRAY<INT>)), 1)`, also changes NULL placement, including producing a value on a row where Spark returns NULL. The left operand is serialized [for the lookup](https://github.com/apache/datafusion-comet/blob/ec9832366bf05b2c645f72c8fabeacfb7af48925/spark/src/main/scala/org/apache/comet/serde/arrays.scala#L623-L628) and [again for the new null guard](https://github.com/apache/datafusion-comet/blob/ec9832366bf05b2c645f72c8fabeacfb7af48925/spark/src/main/scala/org/apache/comet/serde/arrays.scala#L663-L686). Reevaluating a stateful expression over different row selections changes its state and results. Both examples pass with ANSI disabled, and both pass with the base serializers while still executing a native projection. I would raise this from P2 to P1 because it regresses an already-supported primitive-array path, requires no malformed data or incompatible-expression flag, and silently changes downstream counts, filters, or persisted results. ANSI is enabled by default in the tested Spark 4.1.3 session. The demonstrated trigger is an **inline nullable, stateful left operand**; these tests do not show corruption of ordinary deterministic array columns. Retaining Spark fallback for affected nondeterministic operands is a narrow safeguard while single-evaluation handling is implemented. **Validation and limits:** both findings were independently reproduced on Spark 4.1.3 / JDK 17 with native and JVM artifacts built from the pinned head and `CometProject` verified. Exact-base controls substitute only `arrays.scala`, `maps.scala`, and `literals.scala` serializers on the current native/JVM runtime; they are not a full base build. The UTF-8 process abort was reproduced with the debug native build and active unsafe-precondition checks. Release-build failure behavior, heap corruption, and exploitability were not tested or established. I recommend retaining safe fallback for these cases before merging #5452; the broader fixes can remain follow-ups here. -- 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]
