ziting-openai commented on code in PR #24684:
URL: https://github.com/apache/datafusion/pull/24684#discussion_r3885075970
##########
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:
Rechecked `dd83e70097aa9d3b68e3590379e60bca3929c8f3` against base
`4fcaa01c721ba18c10a5ac65446aa48ddf68e9b0` using Arrow 59.2.0. This P2 still
reproduces in a bounded probe: with a skipped outer values span of `1 << 20`,
the largest allocation grows from 224 B on base to 262,144 B on head under both
duplicate-key policies, although both return the same two-row result with one
retained value. No OOM experiment was needed.
The harness compiles the exact helper bodies with only Int32-key ScalarValue
and error-plumbing adapters; it is not a full DataFusion/Spark integration run.
The new mask still scales with ignored input rather than retained output.
_[Posted by Codex on behalf of ziting-openai using the spark-pr-review-memo
skill.]_
##########
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:
Rechecked the nested-null case on `dd83e70097aa9d3b68e3590379e60bca3929c8f3`
with pinned Arrow 59.2.0. It still reproduces independently of the
skipped-outer-span finding: for inner offsets `[0, 65536, 65537]`, inner
validity `[false, true]`, outer offsets `[0, 1, 2]`, and key-row validity
`[true, false]`, base retains zero inner child elements (largest allocation 224
B), while head retains 65,536 unused elements (largest allocation 8,192 B).
Both duplicate-key policies show the same result.
This bounded test uses the exact base/head helper bodies with Int32-key and
error-plumbing adapters. Arrow filter takes the MutableArrayData path for the
partial List selection, while take skips the null child span. The
negative-offset guard and Cow slice do not address this case. No huge
allocation or full-engine run was performed.
_[Posted by Codex on behalf of ziting-openai using the spark-pr-review-memo
skill.]_
--
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]
