sunchao commented on code in PR #5194:
URL: https://github.com/apache/datafusion-comet/pull/5194#discussion_r4104285895
##########
native/spark-expr/src/array_funcs/arrays_overlap.rs:
##########
@@ -435,26 +471,12 @@ fn arrays_overlap_list_generic<OffsetSize:
OffsetSizeTrait>(
(&right_values, &left_values)
};
- let comparator = if needs_comparator(probe.data_type()) {
- Some(make_comparator(
- probe.as_ref(),
- search.as_ref(),
- SortOptions::default(),
- )?)
- } else {
- None
- };
-
for pi in 0..probe.len() {
if probe.is_null(pi) {
has_null = true;
continue;
}
- let (found, null_eq) = if let Some(comparator) = &comparator {
- find_in_array_nested(pi, search, comparator.as_ref())
- } else {
- find_in_array_flat(probe, pi, search)?
- };
+ let (found, null_eq) = find_in_array_flat(probe, pi, search)?;
Review Comment:
[P2] Could we preserve nested comparator handling in this fallback? The
caller still routes unequal Arrow child types here, including types that differ
only in nullability. For a Parquet table `t(i INT)`, `SELECT
arrays_overlap(array_repeat(named_struct('x',1,'y',i),1),
array(named_struct('x',1,'y',i))) FROM t` should return `true`. `array_repeat`
preserves the non-nullable `x` field, while `CometCreateArray` widens it to
nullable. The previous `make_comparator` handled this difference, but the
unconditional `find_in_array_flat` now raises `Nested comparison ... (hint: use
make_comparator instead)`, aborting a previously working query. Keep comparator
support in the fallback or dispatch compatible nested types before the strict
metadata-equality guard, and cover this mixed-constructor case.
Evidence: Using exact base and head implementations with locked Arrow
59.3.0, disposable Rust probes compared identical List and Struct values whose
child nullability differed. Both returned `true` on the base and Arrow errors
on the head. A third probe using `CreateNamedStruct`, Spark's native
`array_repeat`, Comet's `spark_cast`, and `make_array` produced the SQL input
types and reproduced the same regression. Spark 4.1.3 returned `[true, true,
true]` for `i = 1, 2, NULL`. Reproduction source and output are retained at
`/tmp/comet-5194-reproduction.rs` and `/tmp/comet-5194-repro.log`.
--
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]
