pingz-oai commented on code in PR #24684:
URL: https://github.com/apache/datafusion/pull/24684#discussion_r3877674174
##########
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);
Review Comment:
[P2] Avoid allocating a bitmap for large skipped value spans
When `map_from_arrays` receives a NULL typed key-list row, its values row
may be much wider and must be ignored. This new append allocates a bit for
every ignored value, even for a `List<Null>` whose child has no payload buffer.
For example, the valid two-row input below uses only 92 bytes of Arrow buffers.
Base `4fcaa01c721ba18c10a5ac65446aa48ddf68e9b0` returns `[NULL, {7: NULL}]`,
but this head attempts a 134,217,728-byte allocation here. Under the same 64
MiB process address-space limit, base succeeds and head aborts with `memory
allocation of 134217728 bytes failed`, under both `EXCEPTION` and `LAST_WIN`. A
small valid Arrow input can therefore terminate a memory-limited query worker.
Reproduction inside the existing utils test module (both input arrays pass
`validate_full()`):
```rust
use arrow::array::{ListArray, NullArray};
let n = 1_i32 << 30;
let keys: ArrayRef = Arc::new(ListArray::new(
Arc::new(Field::new("item", DataType::Int32, false)),
OffsetBuffer::new(vec![0, 0, 1].into()),
Arc::new(Int32Array::from(vec![7])),
Some(NullBuffer::from(vec![false, true])),
));
let values: 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)),
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,
).unwrap();
```
The exact base/head helper comparison used the locked Arrow 59.2.0
dependencies. Previously the skipped row added no value indices, so `take`
needed space only for the one surviving entry. All offsets here are
non-negative, so the new negative-offset fallback does not help. Please retain
`take` for disproportionately large skipped value spans, or otherwise avoid
materializing their all-false bitmap.
--
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]
