-
Notifications
You must be signed in to change notification settings - Fork 891
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
[Java] Added Java bindings for Parquet options for binary read #11410
[Java] Added Java bindings for Parquet options for binary read #11410
Conversation
7fe4ae8
to
a009c02
Compare
Codecov Report
@@ Coverage Diff @@
## branch-22.08 #11410 +/- ##
===============================================
Coverage ? 86.47%
===============================================
Files ? 144
Lines ? 22856
Branches ? 0
===============================================
Hits ? 19765
Misses ? 3091
Partials ? 0 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason this functionality was added to the base ColumnFilterOptions class instead of ParquetOptions? I think we are only adding this for parquet?
The reason I did it in ColumnFilter was to avoid the code duplication of having to create all the methods from the Builder. But I do see this as a problem. |
rerun tests |
@jbrennan333 I think I have addressed your concerns PTAL |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update @razajafri!
This looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
c++ looks good!
@bdice can we merge this? |
Description
This PR adds Java bindings for adding binary option for
ParquetOptions
Checklist