westonpace commented on code in PR #38147:
URL: https://github.com/apache/arrow/pull/38147#discussion_r1359474026
##########
cpp/src/arrow/compute/light_array.cc:
##########
@@ -80,16 +80,15 @@ KeyColumnArray KeyColumnArray::Slice(int64_t offset,
int64_t length) const {
KeyColumnArray sliced;
sliced.metadata_ = metadata_;
sliced.length_ = length;
- uint32_t fixed_size =
- !metadata_.is_fixed_length ? sizeof(uint32_t) : metadata_.fixed_length;
+ uint32_t fixed_size = metadata_.fixed_length;
sliced.buffers_[0] =
buffers_[0] ? buffers_[0] + (bit_offset_[0] + offset) / 8 : nullptr;
sliced.mutable_buffers_[0] =
mutable_buffers_[0] ? mutable_buffers_[0] + (bit_offset_[0] + offset) /
8 : nullptr;
sliced.bit_offset_[0] = (bit_offset_[0] + offset) % 8;
- if (fixed_size == 0 && !metadata_.is_null_type) {
+ if (metadata_.fixed_length == 0 && !metadata_.is_null_type) {
sliced.buffers_[1] =
buffers_[1] ? buffers_[1] + (bit_offset_[1] + offset) / 8 : nullptr;
Review Comment:
I think this is correct now. `buffers_[0]` is the validity buffer and is
handled consistently across all types. This if/else if/else branch is handling
the values (for fixed-width types) / offsets (for variable-width types) buffer.
So it's roughly...
```
if (is_bool) {
slice by bit, update bit offset
} else if (!is_null) {
slice by word (`fixed_size`), no bit offset needed
} else // is null {
do nothing, there is no values buffer
}
```
--
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]
