pingz-oai commented on code in PR #24684:
URL: https://github.com/apache/datafusion/pull/24684#discussion_r3877756298
##########
datafusion/spark/src/function/map/utils.rs:
##########
@@ -225,31 +246,48 @@ fn map_deduplicate_keys(
);
}
keys_mask_builder.append_value(true);
+ values_mask_builder.append_value(true);
key_to_output_idx.insert(key, value_indices.len());
value_indices.push(abs_value_idx);
new_last_offset += 1;
}
} else {
// The result entry is NULL — no keys/values emitted. Still pad the
- // mask so it stays aligned with `flat_keys`.
+ // masks used by filter so they stay aligned with their flat
arrays.
keys_mask_builder.append_n(num_keys_entries, false);
+ if !needs_value_take {
+ values_mask_builder.append_n(num_values_entries, false);
+ }
}
new_offsets.push(new_last_offset);
cur_keys_offset += num_keys_entries;
cur_values_offset += num_values_entries;
}
let keys_mask = keys_mask_builder.finish();
+ let values_mask = values_mask_builder.finish();
let needed_keys = filter(&flat_keys, &keys_mask)?;
- let value_indices_array = Int32Array::from(value_indices);
- let needed_values = take(&flat_values, &value_indices_array, None)?;
+ let needed_values = if needs_value_take {
+ let value_indices_array = Int32Array::from(value_indices);
+ take(&flat_values, &value_indices_array, None)?
+ } else {
+ // Values lists can be sliced independently of keys lists. Align the
+ // relative mask with their first offset, without allocating a slice
+ // wrapper for the common case where values start at zero.
+ let flat_values = if values_start_offset == 0 {
+ Cow::Borrowed(flat_values)
+ } else {
+ Cow::Owned(flat_values.slice(values_start_offset,
values_mask.len()))
+ };
+ filter(flat_values.as_ref(), &values_mask)?
Review Comment:
[P2] Skip null nested-list children in the filtered values path
When a map value is itself a NULL list with a nonempty child span, this new
`filter()` call can materialize that ignored span. This is separate from the
outer `values_mask_builder.append_n` issue: both outer offsets below are `[0,
1, 2]`, both masks contain just two bits (`[true, false]`), and the retained
first map value is NULL. No duplicate or negative offset selects the `take`
fallback.
With `n = 1 << 30`, both inputs pass Arrow's `validate_full()` and together
use only 172 bytes of buffers. Under the same 64 MiB process address-space
limit, base `4fcaa01c721ba18c10a5ac65446aa48ddf68e9b0` returns `[{7: NULL},
NULL]`, while this head aborts with `memory allocation of 134217728 bytes
failed`. I reproduced this under both `EXCEPTION` and `LAST_WIN`. A small valid
`map_from_arrays` batch can therefore terminate a memory-limited query worker.
Reproduction inside the existing utils test module:
```rust
use arrow::array::{ListArray, NullArray};
let n = 1_i32 << 30;
let offsets = OffsetBuffer::new(vec![0_i32, 1, 2].into());
let keys: ArrayRef = Arc::new(ListArray::new(
Arc::new(Field::new("item", DataType::Int32, false)),
offsets.clone(),
Arc::new(Int32Array::from(vec![7, 8])),
Some(NullBuffer::from(vec![true, false])),
));
let inner: ArrayRef = Arc::new(ListArray::new(
Arc::new(Field::new("item", DataType::Null, true)),
OffsetBuffer::new(vec![0, n, n + 1].into()),
Arc::new(NullArray::new(n as usize + 1)),
Some(NullBuffer::from(vec![false, true])),
));
let values: ArrayRef = Arc::new(ListArray::new(
Arc::new(Field::new("item", inner.data_type().clone(), true)),
offsets,
inner,
None,
));
keys.to_data().validate_full().unwrap();
values.to_data().validate_full().unwrap();
map_from_keys_values_offsets_nulls(
get_list_values(&keys).unwrap(),
get_list_values(&values).unwrap(),
&get_list_offsets(&keys).unwrap(),
&get_list_offsets(&values).unwrap(),
keys.nulls(), values.nulls(), false, // also reproduces with true
).unwrap();
```
In locked Arrow 59.2.0, filtering `List` uses `MutableArrayData`: the
selected NULL list's offsets still extend its full child span, and inherited
null handling allocates a child validity bitmap. The previous `take_list`
instead iterates `output_nulls.valid_indices()` and skips this span. The
independently compiled base/head helper probes used the exact helper bodies,
with a standalone adapter for the unchanged Int32-key and error plumbing. With
a smaller `n = 65_536`, both complete, but the head allocates an 8,192-byte
bitmap and retains 65,536 unused child elements where the base retains zero.
Please retain `take` for this partial-filter case, or otherwise avoid
materializing children of NULL nested values. A guard based only on large
skipped outer-row spans would miss this input.
--
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]
