-
Notifications
You must be signed in to change notification settings - Fork 60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support to read async #33
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was referenced Aug 2, 2021
update: this now includes reading of both metadata and pages (i.e. data). |
Codecov Report
@@ Coverage Diff @@
## main #33 +/- ##
==========================================
- Coverage 68.79% 67.84% -0.95%
==========================================
Files 61 62 +1
Lines 3326 3427 +101
==========================================
+ Hits 2288 2325 +37
- Misses 1038 1102 +64
Continue to review full report at Codecov.
|
6599090
to
41ca6b3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #32
Backward incompatible changes
Thrift code generation had a disruptive change from 0.13 (which
parquet-format
depends on) to0.15
(latest thrift): thrift enums are no longer generated into rust enums in Camel case, but to structs with CAPITAL cases. Since this library uses enums for compile-time validation of the parameters (e.g. Compression), and because enums are the de facto way in rust of handling these, this PR had to bridge some of the APIs exposed by the code generation.This also allowed to review all dependencies on the
parquet-format
and adjust accordingly. Therefore:Encoding
is now only exposed inencoding::Encoding
CompressionCodec
was renamed toCompression
and is only exposed incompression::Compression
DataPageHeaderExt
to access some attributes from the code-generatedDataPageHeader
fromparquet-format
as this crates's enums (i.e. it performs the conversion automatically so that we do not have to worry about that).Other notes
This replaces
thrift
andparquet-format
byparquet-format-async-temp
that contains a minimal thrift library to handlesync
andasync
, together with the parquet-generated code against it using the compiler version that supportsasync
. That crate is basically apache/thrift#2426 and apache/thrift#2426 in a single crate and exists because thrift has 2 releases every year or so and the PR has been 6 days without feedback, suggesting some delay on that front. Once these land, we migrate back toparquet-format-rs
andthrift
.