llama90 commented on code in PR #38147:
URL: https://github.com/apache/arrow/pull/38147#discussion_r1358505770
##########
cpp/src/arrow/compute/light_array.cc:
##########
@@ -80,27 +80,32 @@ 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) {
+ ARROW_DCHECK(is_bool_type() || is_other_fixed_width_types())
+ << "Expected BOOL type or OTHER FIXED WIDTH type but got a different
type.";
sliced.buffers_[1] =
buffers_[1] ? buffers_[1] + (bit_offset_[1] + offset) / 8 : nullptr;
sliced.mutable_buffers_[1] = mutable_buffers_[1]
? mutable_buffers_[1] + (bit_offset_[1] +
offset) / 8
: nullptr;
sliced.bit_offset_[1] = (bit_offset_[1] + offset) % 8;
- } else {
+ } else if (metadata_.fixed_length > 0) {
+ ARROW_DCHECK(is_dictionary_type() || is_binary_type() ||
is_large_binary_type())
Review Comment:
That's right.
If you're worried about why variable and fixed lengths are handled with the
same logic, then the answer is as follows.
As I understand it, Arrow maintains a separate buffer for storing values for
variable lengths.
Because of this, we only need to calculate the offset well for the address
pointing to the value, so I don't think fixed or variable length is an
important consideration when calculating the offset. This is why I didn't think
it was a problem to calculate it with the same logic.
If I'm misunderstanding, I'd be grateful to hear your concerns, I'm still a
newbie to Arrow.
--
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]
