unikdahal commented on code in PR #5414:
URL: https://github.com/apache/datafusion-comet/pull/5414#discussion_r3838162016
##########
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:
Fixed both. Same root cause, two spots: ScalarValue::partial_cmp_struct
flattens a struct to its leaf columns to compare, so a zero-field struct loses
its own validity bit and a NULL element ties with a non-null {} element.
Guarded array_max/array_min on element type, and RANGE frame ordering on the
ORDER BY key type (the existing offset checks only covered explicit-offset
bounds UNBOUNDED/CURRENT ROW skipped them entirely). Also checked sort_array
and plain ORDER BY both use arrow's row-format comparator instead of
ScalarValue::partial_cmp, which encodes struct-level validity independent of
field count, so they're not affected by this one
--
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]
