Support peek_next_page() and skip_next_page in serialized_reader.#2044
Support peek_next_page() and skip_next_page in serialized_reader.#2044tustvold merged 6 commits intoapache:masterfrom
Conversation
tustvold
left a comment
There was a problem hiding this comment.
Looking really good only some minor nits.
| // total row count. | ||
| num_rows: i64, | ||
|
|
||
| //Page offset index. |
There was a problem hiding this comment.
Perhaps we could have something like
enum SerializedPages<T: Read>
/// Read entire chunk
Chunk {
buf: T,
total_num_values: i64,
},
Pages {
index: Vec<PageLocation>,
seen_num_data_pages: usize,
has_dictionary_page_to_read: bool,
page_bufs: VecDeque<T>
}
To make it clear what is present in what mode
There was a problem hiding this comment.
Good advice! i will try in this mode.👍
Co-authored-by: Raphael Taylor-Davies <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2044 +/- ##
==========================================
+ Coverage 83.54% 83.57% +0.02%
==========================================
Files 222 223 +1
Lines 58186 58357 +171
==========================================
+ Hits 48612 48769 +157
- Misses 9574 9588 +14
Continue to review full report at Codecov.
|
|
|
||
| // Column chunk type. | ||
| physical_type: Type, | ||
| // //Page offset index. |
There was a problem hiding this comment.
Do you want to remove these lines?
|
Benchmark runs are scheduled for baseline = 9116297 and contender = 5e520bb. 5e520bb is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Thanks ❤️ |
Which issue does this PR close?
Closes #2043.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?