viirya commented on code in PR #6761:
URL: https://github.com/apache/datafusion-comet/pull/6761#discussion_r4253232449
##########
native/spark-expr/src/math_funcs/greatest_least.rs:
##########
@@ -185,17 +240,18 @@ impl ScalarUDFImpl for SparkGreatestLeast {
// Spark gives every argument the same type up to nullability, but
Arrow also compares
// field names and nullability, which the row selection below needs to
match.
let data_type = args.return_field.data_type();
- let to_array = |arg: &ColumnarValue| -> Result<ArrayRef> {
+ let to_array = |arg: &ColumnarValue, rows: usize| -> Result<ArrayRef> {
let array = arg.to_array(rows)?;
Ok(if array.data_type() == data_type {
array
} else {
cast(&array, data_type)?
})
};
- let mut result = to_array(&args.args[0])?;
- for arg in &args.args[1..] {
- result = self.pick(&result, &to_array(arg)?)?;
+ let args = self.merge_constants(&args.args, data_type, |arg|
to_array(arg, 1))?;
+ let mut result = to_array(&args[0], rows)?;
Review Comment:
Agreed. eabb46575 moves the reduced constant to the front for types without
a float leaf, since the order of their arguments does not matter, so the
columns are compared against it rather than against an intermediate result of
earlier columns. Types with a float leaf keep every argument in place. Your
trailing-constant input now allocates 268,188 bytes versus 268,060 through
DataFusion's functions (16,979,868 at fb630b60a), with identical output.
`constants_merge_in_spark_order` now checks that the merged constant goes first
for a list type and that float arguments keep their order.
--
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]
