0lai0 opened a new pull request, #5395:
URL: https://github.com/apache/datafusion-comet/pull/5395
## Which issue does this PR close?
Closes #5266.
## Rationale for this change
Follow-up to #5233. Arrow's `length` kernel does not accept `MapArray`, so
Map still went through a per-row `Int32Builder` loop over
`MapArray::value_length(i)`. Each `value_length` is already O(1) from the
offset buffer; the cost was the per-row `is_null` branch plus builder appends,
which also blocked autovectorizing the offset diffs.
## What changes are included in this PR?
- `spark_size_map` differences `MapArray::offsets()` with `windows(2)` in
one shot. `offsets()` returns the sliced buffer, so sliced inputs stay correct.
- Extract `rewrite_nulls_to_minus_one` so List and Map share the null → `-1`
rewrite (`into_parts` + `set_indices`). The list-like `null_count() == 0` fast
path is unchanged: return the `length` kernel output as-is and skip the
downcast / `Int32Array::clone` that the helper would otherwise pay.
- Criterion shapes for Map with and without nulls (`benches/array_size.rs`).
- Unit tests for no-null Map and sliced Map (mirrors the sliced-List test
from #5233).
- Docs table: Map array path ~6-13x; `length` does not support Map.
The with-nulls Map path still materializes `Vec<i32>` then clones again
inside `rewrite_nulls_to_minus_one`. Writing `-1` during the first collect
would drop that second allocation; left as a follow-up because the path is
already ~6.5x faster than the loop in #5233.
## How are these changes tested?
- Unit tests in `native/spark-expr/src/array_funcs/size.rs`:
`test_spark_size_map_no_nulls`, `test_spark_size_sliced_map_array`
(slice(2, 3) → `[3, -1, 1]`).
- Existing SQL coverage in `sql-tests/expressions/array/size.sql` (column
map, literal map fallback, `cast(NULL as map)`).
- Criterion (`benches/array_size.rs`), 8192 rows, M-series Mac, arrow
58.4.0. Same session, only `size.rs` swapped, bench file held fixed:
| shape | before | after | speedup |
| -------------------------- | ------- | ------ | ------- |
| spark_size: map, no nulls | 5.85 µs | 452 ns | ~13x |
| spark_size: map (10% null) | 8.22 µs | 1.27 µs | ~6.5x |
List / LargeList moved by less than noise, as expected (only the Map path
changed).
--
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]
