sunchao commented on code in PR #6761:
URL: https://github.com/apache/datafusion-comet/pull/6761#discussion_r4241239300
##########
native/spark-expr/src/math_funcs/greatest_least.rs:
##########
@@ -51,11 +51,11 @@ impl SparkGreatestLeast {
}
}
- /// Whether arguments of this type need Spark's float ordering.
DataFusion's `greatest` and
- /// `least` already match Spark for every other type, since equal values
are identical there.
+ /// Whether arguments of this type need Spark's ordering, of floats or of
nulls inside nested
+ /// values. DataFusion's `greatest` and `least` already match Spark for
every other type, since
+ /// equal values are identical there.
pub fn handles(data_type: &DataType) -> bool {
- matches!(data_type, DataType::Float32 | DataType::Float64)
- || (data_type.is_nested() && has_float_leaf(data_type))
+ matches!(data_type, DataType::Float32 | DataType::Float64) ||
data_type.is_nested()
Review Comment:
Thanks, the adjacent-constant case is fixed at `5f060225`: my reproduction
now allocates 1,690,016 bytes versus 1,675,260 through the previous DataFusion
route.
[P2] Could we also preserve global constant reduction for float-free nested
arguments? `merge_constant_runs` leaves `constant, column, constant` unchanged,
so both constants still get broadcast and compared across the batch. For native
inputs corresponding to the following query, cumulative allocation per
8,192-row invocation is 2,431,384 bytes at this head versus 1,215,772
previously, with identical output. Replacing `greatest` with `least` gives the
same allocation increase.
```sql
SELECT greatest(
array_repeat(-1, 16),
array_repeat(CAST(id % 21 AS INT), 16),
array_repeat(21, 16))
FROM range(8192)
```
Spark folds the two constant arrays individually and retains their positions
around the column-dependent argument. The first-equal concern matters for
floats, but equal float-free nested values have the same observable value, so
those inputs can still reduce all constants with Spark's ordering before
broadcasting. This part of the earlier regression remains unresolved.
Measured with an instrumented native harness using Arrow 59.3.0 and
DataFusion 55.1.0, comparing the previous DataFusion functions with the
exact-head Comet implementation and asserting equal outputs. These are
cumulative allocation measurements, not release timing measurements.
--
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]
