viirya commented on code in PR #6761:
URL: https://github.com/apache/datafusion-comet/pull/6761#discussion_r4235579131
##########
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:
Agreed, the expanded routing broadcast every constant. 5f0602251 merges each
run of adjacent constant arguments into one before the row-wise picks,
comparing them in argument order with the same Spark ordering. That keeps which
of equal arguments is returned, e.g. `-0.0` before `0.0`, so non-adjacent
constants stay separate. With your input (an 8,192-row `ListArray<Int32>` and
eight constant arrays), cumulative allocation is now about 1.69 MB for both
`greatest` and `least`, against 12.94 MB / 10.18 MB at ad5447e4a and 1.68 MB
through DataFusion's functions, with identical results. The same change also
covers nested float arguments, which already took this path.
`adjacent_constants_merge_in_argument_order` covers the merging, float ties,
and nested nulls.
--
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]
