Support skip_values in ByteArrayColumnValueDecoder #2076
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2076 +/- ##
==========================================
+ Coverage 83.61% 83.70% +0.08%
==========================================
Files 223 224 +1
Lines 58539 58958 +419
==========================================
+ Hits 48947 49350 +403
- Misses 9592 9608 +16
Continue to review full report at Codecov.
|
|
@tustvold PTAL😄 |
| let mut values_skip = 0; | ||
| while values_skip < to_skip { | ||
| if self.index_offset == self.index_buf_len { | ||
| let read = self.decoder.get_batch(self.index_buf.as_mut())?; |
There was a problem hiding this comment.
Possibly something for a follow up PR, but it would be nice if we could avoid decoding values only to dump them on the floor
There was a problem hiding this comment.
Agree! file an issue #2088
| assert_eq!(decoder.skip_values(1).unwrap(), 1); | ||
| assert_eq!(decoder.skip_values(1).unwrap(), 1); | ||
|
|
||
| assert_eq!(decoder.read(&mut output, 0..1).unwrap(), 1); |
There was a problem hiding this comment.
| assert_eq!(decoder.read(&mut output, 0..1).unwrap(), 1); | |
| assert_eq!(decoder.read(&mut output, 1..2).unwrap(), 1); |
The decoder doesn't actually care, as it keeps track of the number of read values, but this is technically more correct
There was a problem hiding this comment.
Your are right! 👍, finally it just need the length.
Why not change
fn read(&mut self, out: &mut Self::Slice, range: Range<usize>) -> Result<usize>;
to
fn read(&mut self, out: &mut Self::Slice, length: usize) -> Result<usize>;
Is there any reason 🤔
There was a problem hiding this comment.
Because for backwards compatibility we need to support decoding to a slice, as opposed to a container that tracks its length.
See ColumnValueDecoderImpl
Co-authored-by: Raphael Taylor-Davies <1781103+tustvold@users.noreply.github.com>
|
Benchmark runs are scheduled for baseline = 32867e3 and contender = 72dada6. 72dada6 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #2072.
Support skip values during decoding, will support bytesArray first which include:
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?