-
Notifications
You must be signed in to change notification settings - Fork 3.9k
GH-48168: [C++][Parquet] Fix setting column-specific options when writing an encrypted Dataset #48170
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
Merged
+257
−2
Merged
GH-48168: [C++][Parquet] Fix setting column-specific options when writing an encrypted Dataset #48170
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
6b0b5e6
Add Python unit test to reproduce statistics being lost
adamreeve 63c2821
Add C++ test
adamreeve a9fd162
Part fix
adamreeve e6ad2c4
Fix handling of other column-specific properties
adamreeve 00a13ef
Remove C++ dataset test
adamreeve 0552b66
Remove use of C++20 designated initializers feature
adamreeve a7fc11e
Move initialization logic out of header
adamreeve 6871d86
Merge remote-tracking branch 'upstream/main' into dataset-stats-lost
adamreeve 06a1df1
Merge remote-tracking branch 'upstream/main' into dataset-stats-lost
adamreeve 9cd689b
Add export macro to WriterProperties::Builder
adamreeve File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 file contains hidden or 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 file contains hidden or 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
Oops, something went wrong.
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.
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.
I just noticed that
WriterPropertiesandWriterProperties::Builderuse different forms to represent column properties. Is it possible to simplify this by using the same form?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.
I think the builder needs to work this way so that different column-specific properties can be changed as well as changing default column properties without the order of operations affecting the result. I don't think the
WriterPropertiescan be changed to use the Builder's representation without breaking the API asColumnPropertiesis part of the public API and returned as a reference:arrow/cpp/src/parquet/properties.h
Line 911 in e6ad2c4