-
Notifications
You must be signed in to change notification settings - Fork 1.5k
GH-3213: Add the configuration for ByteStreamSplit encoding #3214
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
Conversation
| } | ||
|
|
||
| public SELF withByteStreamSplitEncoding(String columnPath, boolean enableByteStreamSplit) { | ||
| encodingPropsBuilder.withByteStreamSplitEncoding(columnPath, enableByteStreamSplit); |
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 adding this! Could you please check if there is any test case covering it? If not, it would be good to add one to make sure it takes effect.
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.
Added the corresponding test. Thanks for your reminder
wgtmac
left a comment
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.
+1 Thanks @joeyutong!
|
Could you help rebase to re-trigger the failed CIs? I'll merge after all CIs are passed. |
|
@wgtmac Sorry for the late response. Would you still be available to take another look when you have a moment? |
|
Thanks @joeyutong! |
Rationale for this change
Add the configuration for ByteStreamSplit encoding for a specific column when building
ParquetWriterWhat changes are included in this PR?
A new config method
Are these changes tested?
No
Are there any user-facing changes?
Yes when building
ParquetWritere.g.Closes #3213