chore: Add helpers for computing OffsetBuffer span, length - #25481
Conversation
| for i in 0..num_rows { | ||
| let start = offsets[i].as_usize() - first_offset; | ||
| let end = offsets[i + 1].as_usize() - first_offset; | ||
| for (i, window) in offsets.windows(2).enumerate() { | ||
| let start = window[0].as_usize() - first_offset; | ||
| let end = window[1].as_usize() - first_offset; |
There was a problem hiding this comment.
I found I had to switch to windows(2) to avoid regressing codegen (by introducing bounds checks that were otherwise eliminated). Using windows(2) is arguably more idiomatic, and I checked other locations -- Codex couldn't find another place where using the helpers regressed codegen.
OffsetBuffer span, length
|
If we decide these are a net improvement, I think this would be a candidate to add to arrow-rs -- but I'd argue we should land this in DataFusion first, and then we can switch over to the Arrow versions if/when similar helpers are added there. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25481 +/- ##
==========================================
- Coverage 82.38% 82.38% -0.01%
==========================================
Files 1138 1138
Lines 433733 433696 -37
Branches 433733 433696 -37
==========================================
- Hits 357331 357283 -48
- Misses 54848 54855 +7
- Partials 21554 21558 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| if let (Some(first), Some(last)) = (offsets.first(), offsets.last()) { | ||
| let first = first.as_usize(); | ||
| let last = last.as_usize(); | ||
| let (start, len) = offset_span(list.offsets()); |
There was a problem hiding this comment.
I think this is much easier to read -- thank you @neilconway
|
FYI @rluvaton as I think you are often interested in nested arrays |
Which issue does this PR close?
Rationale for this change
Code that works with Arrow arrays often neglects to account for slicing: that is, all of the array's backing values might not be visible, so code should generally not iterate over values outside the visible range, or assume that the raw length of the backing array is equivalent to the logical length of the array.
To simplify common coding patterns in this area and to encourage correct usage, this PR introduces two helper functions:
offset_spanreturns(first_offset, value_len), the "visible" span described by anOffsetBufferoffset_span_lenreturnsvalue_len, the number of values covered by anOffsetBufferThis replaces typical boilerplate required to handle sliced arrays.
What changes are included in this PR?
estimate_byte_data_sizeinencoding/inner.rs)What is the testing strategy for this PR?
Existing tests pass; new unit tests added.
Are there any user-facing changes?
No. No behavior changes.