sunchao commented on code in PR #6132:
URL: https://github.com/apache/datafusion-comet/pull/6132#discussion_r4085254482
##########
native/core/src/execution/operators/explode.rs:
##########
@@ -943,13 +941,22 @@ fn find_longest_length(list_arrays: &[ArrayRef], options:
&UnnestOptions) -> Res
} else {
Scalar::new(Int64Array::from_value(0, 1))
};
+ let expand_empty = options.expand_empty_as_null();
+ let zero = Scalar::new(Int64Array::from_value(0, 1));
+ let one = Scalar::new(Int64Array::from_value(1, 1));
let list_lengths: Vec<ArrayRef> = list_arrays
.iter()
.map(|list_array| {
let mut length_array = length(list_array)?;
// Make sure length arrays have the same type. Int64 is the most
general one.
length_array = cast(&length_array, &DataType::Int64)?;
length_array = zip(&is_not_null(&length_array)?, &length_array,
&null_length)?;
+ if expand_empty {
+ // Bump empty lists to length 1 so they produce a single
NULL-padded output row.
+ // Runs after the NULL substitution above, which has already
set NULL rows to 1,
+ // so they are not matched here.
+ length_array = zip(&eq(&length_array, &zero)?, &one,
&length_array)?;
Review Comment:
Could we apply this `eq`/`zip` once after the rowwise maximum? After NULL
substitution, `max(max(a, 1), max(b, 1)) == max(max(a, b), 1)`, so this
preserves the result while avoiding a duplicate pair for `posexplode_outer`.
Could the release benchmark also cover the positional path, including short
arrays and mixed NULL/empty arrays? The current benchmark constructs only one
`ListUnnest`, so it does not measure this path. A matched local probe that
included the old wrapper/preprojection costs suggested a slowdown, but its
dependencies were built in debug mode. That is motivation to measure the
release path, not evidence of a release-build regression.
--
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]
