You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2024/06/10 19:59:29 UTC
[PR] Improve Parquet reader/writer properties docs [arrow-rs]
alamb opened a new pull request, #5863:
URL: https://github.com/apache/arrow-rs/pull/5863
# Which issue does this PR close?
Related to #5770
# Rationale for this change
I had to go poking around in the code to determine what the default values for the writer properties were. I think it would be helpful to have this information in the docs.
# What changes are included in this PR?
This PR adds the default values to the docs for the writer properties.
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking change` label.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve Parquet reader/writer properties docs [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold commented on code in PR #5863:
URL: https://github.com/apache/arrow-rs/pull/5863#discussion_r1634807527
##########
parquet/src/file/properties.rs:
##########
@@ -657,8 +709,17 @@ impl WriterPropertiesBuilder {
self
}
- /// Sets the max length of min/max value fields in statistics. Must be greater than 0.
- /// If set to `None` - there's no effective limit.
+ /// Sets the max length of min/max value fields in row group level
+ /// [`Statistics`] (defaults to `None`).
+ ///
+ /// # Notes
+ /// Row group level [`Statistics`] are written when [`Self::set_statistics_enabled`] is
+ /// set to [`EnabledStatistics::Chunk`] or [`EnabledStatistics::Page`].
+ ///
+ /// * If `Some`, Must be greater than 0, otherwise will panic
+ /// * If `None` - there's no effective limit.
Review Comment:
```suggestion
/// * If `Some`, must be greater than 0, otherwise will panic
/// * If `None`, there's no effective limit.
```
##########
parquet/src/file/properties.rs:
##########
@@ -615,39 +653,53 @@ impl WriterPropertiesBuilder {
self
}
- /// Sets max size for statistics for a column.
- /// Takes precedence over globally defined settings.
+ /// Sets max size for statistics for a specific column.
+ ///
+ /// Takes precedence over [`Self::set_max_statistics_size`].
pub fn set_column_max_statistics_size(mut self, col: ColumnPath, value: usize) -> Self {
self.get_mut_props(col).set_max_statistics_size(value);
self
}
- /// Sets whether a bloom filter should be created for a specific column.
- /// The behavior is similar to [`set_bloom_filter_enabled`](Self::set_bloom_filter_enabled).
- /// Takes precedence over globally defined settings.
+ /// Sets whether a bloom filter should be written for a specific column.
+ ///
+ /// Takes precedence over [`Self::set_bloom_filter_enabled`].
pub fn set_column_bloom_filter_enabled(mut self, col: ColumnPath, value: bool) -> Self {
self.get_mut_props(col).set_bloom_filter_enabled(value);
self
}
/// Sets the false positive probability for bloom filter for a specific column.
- /// The behavior is similar to [`set_bloom_filter_fpp`](Self::set_bloom_filter_fpp) but will
- /// override the default.
+ ///
+ /// Takes precedence over [`Self::set_bloom_filter_fpp`].
pub fn set_column_bloom_filter_fpp(mut self, col: ColumnPath, value: f64) -> Self {
self.get_mut_props(col).set_bloom_filter_fpp(value);
self
}
/// Sets the number of distinct values for bloom filter for a specific column.
- /// The behavior is similar to [`set_bloom_filter_ndv`](Self::set_bloom_filter_ndv) but will
- /// override the default.
+ ///
+ /// Takes precedence over [`Self::set_bloom_filter_ndv`].
pub fn set_column_bloom_filter_ndv(mut self, col: ColumnPath, value: u64) -> Self {
self.get_mut_props(col).set_bloom_filter_ndv(value);
self
}
- /// Sets the max length of min/max value fields in the column index. Must be greater than 0.
- /// If set to `None` - there's no effective limit.
+ /// Sets the max length of min/max value fields when writing the column
+ /// [`Index`] (defaults to `None`).
+ ///
+ /// This can be used to prevent columns with very long values (hundreds of
+ /// bytes long) from causing the parquet metadata to become huge.
+ ///
+ /// # Notes
+ ///
+ /// The column [`Index`] is written when [`Self::set_statistics_enabled`] is
+ /// set to [`EnabledStatistics::Page`].
+ ///
+ /// * If `Some`, Must be greater than 0, otherwise will panic
+ /// * If `None` - there's no effective limit.
Review Comment:
```suggestion
/// * If `Some`, must be greater than 0, otherwise will panic
/// * If `None`, there's no effective limit.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve Parquet reader/writer properties docs [arrow-rs]
Posted by "progval (via GitHub)" <gi...@apache.org>.
progval commented on code in PR #5863:
URL: https://github.com/apache/arrow-rs/pull/5863#discussion_r1634462067
##########
parquet/src/file/properties.rs:
##########
@@ -299,7 +299,7 @@ impl WriterProperties {
.unwrap_or(DEFAULT_DICTIONARY_ENABLED)
}
- /// Returns `true` if statistics are enabled for a column.
+ /// Returns the type of statistics written for a column.
Review Comment:
```suggestion
/// Returns which statistics are written for a column.
```
seems more accurate, as there are two different types of statistics and they are not mutually exclusive
##########
parquet/src/file/properties.rs:
##########
@@ -575,37 +608,42 @@ impl WriterPropertiesBuilder {
self.column_properties.entry(col).or_default()
}
- /// Sets encoding for a column.
- /// Takes precedence over globally defined settings.
+ /// Sets encoding for a specific column.
+ ///
+ /// Takes precedence over [`Self::set_encoding`].
///
/// If dictionary is not enabled, this is treated as a primary encoding for this
/// column. In case when dictionary is enabled for this column, either through
/// global defaults or explicitly, this value is considered to be a fallback
/// encoding for this column.
///
- /// Panics if user tries to set dictionary encoding here, regardless of dictionary
+ /// # Panics
+ /// If user tries to set dictionary encoding here, regardless of dictionary
/// encoding flag being set.
pub fn set_column_encoding(mut self, col: ColumnPath, value: Encoding) -> Self {
self.get_mut_props(col).set_encoding(value);
self
}
- /// Sets compression codec for a column.
- /// Takes precedence over globally defined settings.
+ /// Sets compression codec for a specific column.
+ ///
+ /// Takes precedence over [`Self::set_compression`].
pub fn set_column_compression(mut self, col: ColumnPath, value: Compression) -> Self {
self.get_mut_props(col).set_compression(value);
self
}
- /// Sets flag to enable/disable dictionary encoding for a column.
- /// Takes precedence over globally defined settings.
+ /// Sets flag to enable/disable dictionary encoding for a specific column.
+ ///
+ /// Takes precedence over [`Self::set_dictionary_enabled`].
pub fn set_column_dictionary_enabled(mut self, col: ColumnPath, value: bool) -> Self {
self.get_mut_props(col).set_dictionary_enabled(value);
self
}
- /// Sets flag to enable/disable statistics for a column.
- /// Takes precedence over globally defined settings.
+ /// Sets flag to enable/disable statistics for a specific column.
+ ///
+ /// Takes precedence over [`Self::set_statistics_enabled`].
Review Comment:
```suggestion
/// Sets statistics level for a specific column.
///
/// Takes precedence over [`Self::set_statistics_enabled`].
```
to be consistent with `set_statistics_enabled`'s phrasing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve Parquet reader/writer properties docs [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5863:
URL: https://github.com/apache/arrow-rs/pull/5863#issuecomment-2159209043
MSRV failure is due to https://github.com/apache/arrow-rs/issues/5864
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve Parquet reader/writer properties docs [arrow-rs]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #5863:
URL: https://github.com/apache/arrow-rs/pull/5863#issuecomment-2160650013
Thank you @progval
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Improve Parquet reader/writer properties docs [arrow-rs]
Posted by "tustvold (via GitHub)" <gi...@apache.org>.
tustvold merged PR #5863:
URL: https://github.com/apache/arrow-rs/pull/5863
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org