sunchao commented on code in PR #6761:
URL: https://github.com/apache/datafusion-comet/pull/6761#discussion_r4241239903
##########
native/spark-expr/src/math_funcs/greatest_least.rs:
##########
@@ -93,6 +93,30 @@ impl SparkGreatestLeast {
}
}
+ /// Replaces each run of adjacent constant arguments with the one constant
the run picks, so
+ /// that a run is compared once rather than broadcast and compared row by
row for each of its
+ /// constants. The result is the first argument that no later argument
ranks strictly before,
+ /// and picking within a run keeps that, so equal floats such as `-0.0`
and `0.0` still resolve
+ /// to the earlier argument. Constants that are not adjacent stay apart
for the same reason.
+ /// `to_array` turns a constant into a one-row array of the result type.
+ fn merge_constant_runs(
+ &self,
+ args: &[ColumnarValue],
+ to_array: impl Fn(&ColumnarValue) -> Result<ArrayRef>,
+ ) -> Result<Vec<ColumnarValue>> {
+ let mut merged: Vec<ColumnarValue> = Vec::with_capacity(args.len());
+ for arg in args {
+ match (merged.last_mut(), arg) {
+ (Some(previous @ ColumnarValue::Scalar(_)),
ColumnarValue::Scalar(_)) => {
+ let picked = self.pick(&to_array(previous)?,
&to_array(arg)?)?;
+ *previous =
ColumnarValue::Scalar(ScalarValue::try_from_array(&picked, 0)?);
Review Comment:
[P2] Could scalar-run reduction retain the selected original scalar instead
of passing it through `pick` and `zip`? For nested values, `zip` copies the
entire winning list on every comparison, even when the winner is unchanged and
the comparison only examines its first element.
For native inputs corresponding to this query, an 8,192-row invocation
allocates 2,052,684 bytes at `5f060225` versus 200,940 through the previous
DataFusion route, with identical results:
```sql
SELECT greatest(
array(CAST(id % 21 AS INT)),
array_repeat(0, 65536),
array(-1), array(-2), array(-3), array(-4),
array(-5), array(-6), array(-7), array(21))
FROM range(8192)
```
The long literal is copied seven times before the final scalar replaces it.
Mirroring the signs and using `least` reproduces the same increase. Comparing
the one-row arrays with Spark's ordering and retaining the selected scalar
preserves first-equal semantics without those copies. A scratch implementation
reduced allocation to 201,324 bytes and returned the same output.
Measured cumulative allocated bytes per invocation with an instrumented
native harness using Arrow 59.3.0 and DataFusion 55.1.0. This is an allocation
regression, not a release timing claim.
--
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]
