sunchao commented on code in PR #5452:
URL: https://github.com/apache/datafusion-comet/pull/5452#discussion_r3848123213
##########
spark/src/main/scala/org/apache/comet/serde/literals.scala:
##########
@@ -215,4 +225,104 @@ object CometLiteral extends CometExpressionSerde[Literal]
with Logging {
}
listLiteralBuilder
}
+
+ /**
+ * Rebuild a folded complex Literal as an equivalent tree of `CreateArray` /
`CreateMap` over
+ * primitive-typed Literals, or `None` when the shape cannot be rebuilt. The
native `Literal`
+ * proto carries scalars and nested `ListLiteral`s but no map values, so a
Literal whose type
+ * contains a `MapType` has to be expanded before serialization. Teaching
the proto to transport
+ * maps directly would remove the need for this rewrite and for the declines
below:
+ * https://github.com/apache/datafusion-comet/issues/1937
+ *
+ * Declined shapes:
+ * - Null values and empty top-level containers: a synthesized `Create*`
with no children
+ * cannot recover the original element type. Empty `ArrayType` literals
still serialize via
+ * `makeListLiteral`, which keeps the type.
+ * - `StructType` at any depth. Native `CreateNamedStruct` builds a 1-row
`StructArray`
+ * whenever all of its children are scalars (`values_to_arrays`), which
collides with the
+ * surrounding batch's row count. Its proto message also carries no
type, so Spark's
+ * declared field nullability cannot survive the wire either way.
+ * - Map key types whose Spark equality semantics native lookup cannot
honor, see
+ * [[hasUnsafeMapKeyType]].
+ * - Folded maps with duplicate keys. The rebuilt `CreateMap` evaluates
through
+ * `ArrayBasedMapBuilder`, which throws under
`MAP_KEY_DEDUP_POLICY=EXCEPTION`, where the
+ * original literal had already folded cleanly.
+ */
+ private def expandComplexLiteral(expr: Literal): Option[Expression] = {
+ if (expr.value == null) return None
+ expr.dataType match {
+ case ArrayType(et, _) if needsExpansion(et) =>
+ val arr = expr.value.asInstanceOf[ArrayData]
+ if (arr.numElements() == 0) {
+ None
+ } else {
+ val elements = (0 until arr.numElements()).map(i =>
asNullable(literalAt(arr, i, et)))
+ Some(CreateArray(elements, useStringTypeWhenEmpty = false))
+ }
+ case MapType(kt, vt, _) =>
+ val mapData = expr.value.asInstanceOf[MapData]
+ val keys = mapData.keyArray()
+ if (mapData.numElements() == 0 || hasUnsafeMapKeyType(kt) ||
+ hasDuplicateMapKeys(keys, kt)) {
+ None
+ } else {
+ val values = mapData.valueArray()
+ val children = (0 until keys.numElements()).flatMap(i =>
+ Seq(literalAt(keys, i, kt), asNullable(literalAt(values, i, vt))))
Review Comment:
[P2] Preserve map-value nullability when expanding folded literals
Could reconstruction retain the literal's declared `valueContainsNull`? With
normal constant folding and a Parquet table containing `id` values 1, 2, 3:
```sql
SELECT id, array(map(1, 2), map(2, coalesce(id, 0))) AS a
FROM t
```
Both original array children have `MapType(IntegerType, IntegerType,
false)`, so the existing array type check passes. Wrapping the folded map's
value in `KnownNullable` changes only that reconstructed map to
`valueContainsNull=true`. The dynamic sibling stays `false`, and native
`make_array` panics with `Arrays with inconsistent types passed to
MutableArrayData`. Unlike the earlier non-folded constructor example, this
rewrite introduces the mismatch after checking two identical input types.
I reproduced this at `31f28b2e` on Spark 3.5.9 and 4.1.3 with native
`CometProject` assertions. Replacing only the literal serializer with the exact
base version restores Spark projection and the correct three-row result. Please
preserve the declared map-value nullability and add a normal-folding regression
test for this mixed literal/dynamic case.
--
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]
