bkietz commented on a change in pull request #11810:
URL: https://github.com/apache/arrow/pull/11810#discussion_r760252620
##########
File path: cpp/src/arrow/util/compression_lz4.cc
##########
@@ -234,14 +244,21 @@ class LZ4Compressor : public Compressor {
LZ4F_compressionContext_t ctx_ = nullptr;
LZ4F_preferences_t prefs_;
bool first_time_;
+
+ private:
+ int compression_level_;
};
// ----------------------------------------------------------------------
// Lz4 frame codec implementation
class Lz4FrameCodec : public Codec {
public:
- Lz4FrameCodec() : prefs_(DefaultPreferences()) {}
+ explicit Lz4FrameCodec(int compression_level)
+ : compression_level_(compression_level == kUseDefaultCompressionLevel
+ ? kLZ4DefaultCompressionLevel
+ : compression_level),
+ prefs_(DefaultPreferences(compression_level_))
{}
Review comment:
Nit: these preferences aren't really default anymore since they have a
non default compression level. Would you rename this, please?
```suggestion
prefs_(PreferencesWithCompressionLevel(compression_level_)) {}
```
##########
File path: python/pyarrow/tests/test_feather.py
##########
@@ -637,7 +637,8 @@ def test_v2_compression_options():
cases = [
# compression, compression_level
('uncompressed', None),
- ('lz4', None),
+ ('lz4', 1),
Review comment:
IIUC, the `None` case should still work so let's keep testing it
```suggestion
('lz4', None),
('lz4', 1),
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
