Support compression levels #3847
Conversation
spebern
force-pushed
the
compression-levels
branch
from
55e5c21 to
1531885
Compare
spebern
force-pushed
the
compression-levels
branch
from
dd063bf to
9b7ede6
Compare
| @@ -286,11 +287,11 @@ pub enum Encoding { | |||
| pub enum Compression { | |||
There was a problem hiding this comment.
This is a breaking change and is set in
arrow-rs/parquet/src/file/properties.rs
Line 424 in 9ce0ebb
One way to make this not breaking could be to add a new enum CompressionOptions that can also be set via the WriterPropertiesBuilder.
| @@ -1783,11 +1784,20 @@ mod tests { | |||
| fn test_display_compression() { | |||
| assert_eq!(Compression::UNCOMPRESSED.to_string(), "UNCOMPRESSED"); | |||
| assert_eq!(Compression::SNAPPY.to_string(), "SNAPPY"); | |||
| assert_eq!(Compression::GZIP.to_string(), "GZIP"); | |||
| assert_eq!( | |||
There was a problem hiding this comment.
I am not so sure about whether the level should get displayed.
| @@ -121,6 +121,26 @@ impl CodecOptionsBuilder { | |||
| } | |||
| } | |||
|
|
|||
| /// Defines valid compression levels. | |||
There was a problem hiding this comment.
I just want to mention as I have before that this is copied from parquet2, so the credit should go to those authors of course!
| @@ -284,7 +343,7 @@ mod brotli_codec { | |||
| let mut encoder = brotli::CompressorWriter::new( | |||
| output_buf, | |||
| BROTLI_DEFAULT_BUFFER_SIZE, | |||
There was a problem hiding this comment.
Maybe it makes sense to not only make the level configurable for brotli, but also the buffer and window size.
There was a problem hiding this comment.
I'm not familiar enough with brotli to know how important those parameters are, if someone feels strongly about exposing them I wouldn't object
|
Benchmark runs are scheduled for baseline = dfb8c76 and contender = 7b94b08. 7b94b08 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
* Support zstd compression levels * Support gzip compression levels * Fix tests * Support brotli compression level * Fix tests * Add tests for all supported compression levels
This code has been almost completely copied from
parquet2.Which issue does this PR close?
Closes #3844.
Rationale for this change
What changes are included in this PR?
Are there any user-facing changes?
The compression enum got variants.
Other ways I have thought of
Another way to make this non breaking would be, to include another enum
CompressionOptionsthat has all the codes with their supported arguments, which can be set with theWriterPropertiesBuilder.