sunchao commented on code in PR #6533:
URL: https://github.com/apache/datafusion-comet/pull/6533#discussion_r4163306181
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -376,13 +376,40 @@ object CometArrayExcept
return None
case None =>
}
- val leftArrayExprProto = exprToProtoInternal(expr.left, inputs, binding)
- val rightArrayExprProto = exprToProtoInternal(expr.right, inputs, binding)
+
+ // Fix for issue #3646: Normalize element type nullability to avoid
DataFusion type mismatch.
+ // DataFusion's array_except requires both arrays to have the same element
nullability.
+ // Without this normalization, we get errors like:
+ // "array_except received incompatible types: List(Int32), List(non-null
Int32)"
+ val leftType = expr.left.dataType.asInstanceOf[ArrayType]
+ val rightType = expr.right.dataType.asInstanceOf[ArrayType]
+
+ val normalizedLeft = if (!leftType.containsNull && rightType.containsNull)
{
+ // Left is non-null but right is nullable, cast left to nullable
+ org.apache.spark.sql.catalyst.expressions.Cast(
+ expr.left,
+ ArrayType(leftType.elementType, containsNull = true))
Review Comment:
[P2] Preserve recursive nullability when normalizing nested arrays. With
`spark.comet.expression.ArrayExcept.allowIncompatible=true`, consider `SELECT
array_except(array(array(id)), array(IF(id = 0, array(2L), NULL))) FROM
range(2)`. Spark returns `[[0]]` and `[[1]]`. Both native arguments previously
had type `List(List(Int64))` because `CometCreateArray` widens nested element
nullability. This new cast uses Catalyst's non-nullable inner element type,
narrowing the left argument to `List(List(non-null Int64))` while leaving the
right argument unchanged. DataFusion consequently rejects previously working
input with `array_except received incompatible types`. The symmetric branch has
the same problem. Could both arguments be normalized to a common recursively
nullable type, preserving the existing widening, with regression tests for both
operand orders?
Evidence: Spark 3.5.9 executed the SQL successfully and its optimized plan
retained left type ArrayType(ArrayType(LongType,false),false) and right type
ArrayType(ArrayType(LongType,false),true). A Spark 4.1.3 Catalyst probe
confirmed those types and results. A disposable native test used the current
source and locked dependencies: DataFusion MakeArray produced both
List(List(Int64)) inputs, and ArrayExcept succeeded before the added cast.
Applying Comet spark_cast to the PR's target type caused the exact
incompatible-types error in both legacy and ANSI modes and both operand orders.
Command: `CARGO_TARGET_DIR=/tmp/pr6518-cargo-target cargo test --manifest-path
native/Cargo.toml -p datafusion-comet-spark-expr --test review6533 --locked
--offline -- --nocapture`. The test passed its regression assertions. Source
and logs are preserved under `/tmp/comet6533-review/`. This was an isolated
kernel reproduction, not an end-to-end Comet SQL run.
--
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]
