Support peek_next_page and skip_next_page in InMemoryPageReader #2407
Conversation
|
@tustvold ptal 😊 |
| if let Some(x) = self.page_iter.peek() { | ||
| match x { | ||
| Page::DataPage { .. } => { | ||
| unreachable!() |
There was a problem hiding this comment.
I found all test pages read by InMemoryPageReader are using DataPagev2.
and DataPagev1 not have the 'num_rows' info.
There was a problem hiding this comment.
I my mind , i think most application(like spark) still use v1 page(which not supported DeltaBitPack). Is iox using datapagev2?
There was a problem hiding this comment.
Yeah, most applications have yet to update to parquet v2 despite it being released almost a decade ago now 😆
IOx isn't currently, but may in future. DeltaBitPack is very good for storing sorted timestamps as are common for our workload. Although their are some questionable decisions in the DeltaBitPack encoding scheme that make it more expensive than it really should be... Perhaps that is why most applications haven't bothered to upgrade 😅
That was why I was surprised to see this isn't being exercised 😅
I can't remember if column index support requires v2 🤔
DataPagev1 not have the 'num_rows' info.
Just use num_values (which also includes nulls) it is a best effort number that is only relied upon if there aren't any levels (in which case it is the row count)
There was a problem hiding this comment.
Cool ! thanks for your info.
There was a problem hiding this comment.
I can't remember if column index support requires v2 🤔
supported both v1 and v2
index info stored in ColumnChunk
|
Benchmark runs are scheduled for baseline = d11b388 and contender = ee2818e. ee2818e 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 #2406.
Related #2393
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?