sunchao commented on code in PR #5452:
URL: https://github.com/apache/datafusion-comet/pull/5452#discussion_r3867906876
##########
spark/src/main/scala/org/apache/comet/serde/arrays.scala:
##########
@@ -472,27 +472,20 @@ object CometCreateArray extends
CometExpressionSerde[CreateArray] {
// DataFusion's `make_array` asserts strict element-type equality in
// `MutableArrayData::with_capacities` and panics on a mismatch. Spark's
CreateArray is more
- // permissive: its type coercion compares element types with `sameType`,
which ignores
- // nullability, so children that share a surface type but differ only in
nested field
- // nullability get no unifying cast. DataFusion tolerates container
nullability differences
- // (an `ArrayType.containsNull` / `MapType.valueContainsNull` mismatch is
coerced), but NOT a
- // struct field's nullability -- `array(struct(a not null), struct(a
nullable))` panics inside
- // `make_array_inner`. Decline only those cases (i.e. children that still
differ after
- // normalizing container nullability) so Spark's evaluator handles them.
- //
- // TODO: remove this decline once apache/datafusion#22366 lands; the
upstream fix widens the
- // element type via nullability-OR-merge and casts each child before
MutableArrayData.
- val normalizedTypes = children.map(c =>
normalizeContainerNullability(c.dataType))
- if (normalizedTypes.distinct.size > 1) {
- withFallbackReason(
- expr,
- "CreateArray children have mismatched data types: " +
- children.map(_.dataType).distinct.mkString(", "))
- return None
+ // permissive: its coercion compares element types with `sameType`
(nullability ignored), so
+ // children that share a surface type but differ in nullability reach here
as distinct types.
+ // Comet's native runtime types are also frequently MORE nullable than
Spark's Catalyst types
+ // (`map_entries` forces the entry `value` field nullable, list elements
are nullable, ...), so
+ // casting to Spark's declared element type does not reliably unify them.
Cast every child to a
+ // deeply-nullable element type instead (every array/map/struct field
nullable at all nesting
+ // levels; the cast only widens metadata and never changes values), so
`make_array` always sees
+ // identical Arrow types. A child whose cast is unsupported declines below.
+ val elementType =
deepNullable(expr.dataType.asInstanceOf[ArrayType].elementType)
Review Comment:
**[P2] Keep widened array types consistent with downstream consumers**
`deepNullable` changes the Arrow type produced by this array without
reconciling independently serialized arguments and declared result types of its
consumers. For Parquet INT `id` values `1,2,3`:
```sql
SELECT array_insert(
array(map(1, coalesce(id, 0))),
2,
map(2, coalesce(id, 0)))
FROM t
```
Both Catalyst maps have non-nullable values. Only the array element is
widened here, so the inserted map remains non-nullable and native `ArrayInsert`
rejects their unequal types (`Type mismatch in ArrayInsert`). This fails with
normal folding on Spark 4.0.4/JDK 17; Spark, dispatcher-disabled execution, and
exact prior/base array-serializer controls all return the correct rows (the
prior/base controls also execute this projection natively). The same metadata
drift causes `slice` declared-return-type errors and `IF` schema errors on
all-false batches. Please keep consumer arguments/result types consistent with
this widening, or retain fallback where that cannot be satisfied.
--
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]
