parthchandra opened a new pull request, #7191:
URL: https://github.com/apache/datafusion-comet/pull/7191
## Which issue does this PR close?
Closes #7024.
## Rationale for this change
A single `min`/`max` over a struct input sends the whole aggregate operator
back to Spark. A common pattern is `max(struct(order_col, payload_1, payload_2,
...))`, used to pick the payload of the row with the largest ordering value.
DataFusion already has a struct min/max accumulator. Only the Scala-side gate
blocked it.
## What changes are included in this PR?
- `AggSerde.minMaxSupportLevel` now accepts `StructType`:
- **Compatible** when every field, at any depth, is a type native min/max
supports, no field is float or double, and no field is nullable.
- **Incompatible** (opt-in with
`spark.comet.expression.Min/Max.allowIncompatible`) when some field is
nullable. DataFusion skips a null field when comparing two structs, while Spark
orders null as the smallest value.
- **Unsupported** for float/double fields, non-default collations, and
other unsupported leaf types such as arrays and `timestamp_ntz`.
- The native planner casts a struct input to Spark's declared type with
Comet's `Cast` instead of DataFusion's `CastExpr`. Native expressions such as
`coalesce` can report nullable fields where Spark declares non-nullable ones,
and `CastExpr` rejects that narrowing. The aggregate keeps Spark's declared
type, so its state matches what shuffle and scans hand to the final stage.
- Window min/max over structs still falls back.
## How are these changes tested?
New SQL file tests that compare against Spark:
- `min_max_struct.sql`:
- grouped, ungrouped and empty inputs
- nested structs and whole-struct nulls
- boolean, integral, decimal(38), date, timestamp, string (CJK) and binary
fields
- ties decided by later fields
- PartialMerge via `count(DISTINCT)`
- Spark partial to native final
- fallback reasons for float, array and `timestamp_ntz` fields
- Spark's analysis error for map fields
- `min_max_struct_nullable.sql`: the opt-in path. The known null-field
difference is recorded as an ignored query.
- `min_max_struct_timezone.sql`: four session time zones, DST transitions,
and values near the ends of the range.
- `min_max_struct_no_comet_shuffle.sql`: Comet shuffle disabled.
Plus a Rust unit test that the struct cast keeps values when it relabels
field nullability.
--
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]
