Add GenericByteArray (#2946) #2947
Conversation
There was a problem hiding this comment.
I think this change looks lovely 😍 -- though I think we should leave it open for a while in case other contributors (such as @viirya and @HaoYang670 ) would like to comment
| @@ -0,0 +1,206 @@ | |||
| // Licensed to the Apache Software Foundation (ASF) under one | |||
| value_offsets: RawPtrBox<OffsetSize>, | ||
| value_data: RawPtrBox<u8>, | ||
| } | ||
| pub type GenericStringArray<OffsetSize> = GenericByteArray<GenericStringType<OffsetSize>>; |
There was a problem hiding this comment.
This is a very nice cleanup to remove duplication 🏆
| pub fn value_length(&self, i: usize) -> T::Offset { | ||
| let offsets = self.value_offsets(); | ||
| offsets[i + 1] - offsets[i] | ||
| } |
There was a problem hiding this comment.
Slice indexing is checked by default, so it will panic in such a case. We could add an unchecked variant, but at that point the caller might as well just call value_offsets and do this manually
|
I intend to merge this this evening, i.e. in about 6 hours |
|
Removed api-change label as this is not a breaking change |
|
Benchmark runs are scheduled for baseline = 73416f8 and contender = b6f08a8. b6f08a8 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
alamb
added
api-change
Which issue does this PR close?
Closes #2946
Rationale for this change
See ticket
What changes are included in this PR?
Defines a
GenericByteArrayand migrates some methods across, subsequent PRs can then work on eliminating duplicationAre there any user-facing changes?
No 🎉