-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Rename ColumnOptions
to ParquetColumnOptions
#11512
Conversation
@metesynnada or @metegenez I wonder if you have any thoughts about this change |
I dont see an issue here, ColumnOptions only belong to Parquet anyway. It is a good clarification. |
@@ -1552,7 +1552,7 @@ config_namespace_with_hashmap! { | |||
/// Options controlling parquet format for individual columns. | |||
/// | |||
/// See [`ParquetOptions`] for more details | |||
pub struct ColumnOptions { | |||
pub struct ParquetColumnOptions { |
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.
We could additionally (later) move the Parquet functionality to Parquet-related modules/files.
Perhaps just for organizational purposes.
Thanks @findepi and @jonahgao and @metegenez |
Which issue does this PR close?
Part of #11367
Rationale for this change
While reviewing #11444 from @wiedld I was somewhat confused about ColumnOptions were and that they were parquet specific
What changes are included in this PR?
ColumnOptions
toParquetColumnOptions
rust structsAre these changes tested?
Are there any user-facing changes?