sunchao commented on code in PR #5414:
URL: https://github.com/apache/datafusion-comet/pull/5414#discussion_r3837170725
##########
spark/src/main/scala/org/apache/comet/DataTypeSupport.scala:
##########
@@ -54,8 +54,9 @@ trait DataTypeSupport {
CalendarIntervalType =>
true
case StructType(fields) =>
- fields.nonEmpty && fields.forall(f =>
- isTypeSupported(f.dataType, f.name, fallbackReasons))
+ // A struct's `fields` can be empty -- e.g. Iceberg's `_partition`
metadata column is
+ // exactly that on an unpartitioned table. It's still a value Comet
can represent.
+ fields.forall(f => isTypeSupported(f.dataType, f.name,
fallbackReasons))
Review Comment:
[P2] Guard nested-empty values in native scalar-ordering paths
With native local-table scan/shuffle enabled, let `df` contain `id: INT` and
`a: ARRAY<STRUCT<marker:STRUCT<>>>`, with `a = [Row(null), Row(Row())]`.
`df.repartition(2, df("id")).selectExpr("array_max(a)")` stays on Spark at the
pinned base, but the exact-head planner converts it to `CometProject ->
CometNativeShuffle -> CometLocalTableScan`. Spark 3.5.2/4.0.4 returns the
element whose marker is `{}`; the pinned DataFusion 54.1 UDF instead returns
the element whose marker is NULL.
`ScalarValue::partial_cmp_struct` recursively flattens structs, so the
zero-field child contributes neither fields nor validity and those two values
compare equal. The same comparator also misidentifies RANGE-window peers: with
`s: struct<e:struct<>>` and a second integer order key, `COUNT(*) OVER (ORDER
BY s, k RANGE BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW)` counts `{e:NULL}`
and `{e:{}}` together. The new compaction, grouping-reconstruction, and
default-cast guards do not cover these ordering paths. Please keep the affected
array extrema and RANGE order keys containing empty structs on Spark until
native comparison preserves nested validity.
--
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]
