pitrou commented on a change in pull request #11810:
URL: https://github.com/apache/arrow/pull/11810#discussion_r761130558
##########
File path: cpp/src/arrow/util/compression_lz4.cc
##########
@@ -41,6 +41,9 @@ namespace util {
namespace {
+constexpr int kLZ4MinCompressionLevel = 1;
+constexpr int kLZ4DefaultCompressionLevel = 1;
Review comment:
The same constant is already defined in the `.h`, why redefine it here?
##########
File path: ci/docker/linux-apt-jni.dockerfile
##########
@@ -83,5 +82,6 @@ ENV ARROW_BUILD_TESTS=OFF \
CC=gcc \
CXX=g++ \
ORC_SOURCE=BUNDLED \
+ LZ4_SOURCE=BUNDLED \
Review comment:
Nit: keep those entries in alphabetical order.
##########
File path: cpp/src/arrow/util/compression_internal.h
##########
@@ -65,7 +65,10 @@ std::unique_ptr<Codec> MakeLz4RawCodec();
std::unique_ptr<Codec> MakeLz4HadoopRawCodec();
// Lz4 frame format codec.
-std::unique_ptr<Codec> MakeLz4FrameCodec();
+constexpr int kLZ4DefaultCompressionLevel = 1;
Review comment:
Nit: "Lz4" (not capitalized)
##########
File path: cpp/src/arrow/util/compression_lz4.cc
##########
@@ -300,9 +316,14 @@ class Lz4FrameCodec : public Codec {
}
Compression::type compression_type() const override { return
Compression::LZ4_FRAME; }
- int minimum_compression_level() const override { return
kUseDefaultCompressionLevel; }
- int maximum_compression_level() const override { return
kUseDefaultCompressionLevel; }
- int default_compression_level() const override { return
kUseDefaultCompressionLevel; }
+ int minimum_compression_level() const override { return
kLZ4MinCompressionLevel; }
+ int maximum_compression_level() const override { return
LZ4F_compressionLevel_max(); }
+ int default_compression_level() const override { return
kLZ4DefaultCompressionLevel; }
+
+ int compression_level() const override { return compression_level_; }
+
+ private:
+ const int compression_level_;
Review comment:
Just put this in the `protected` section below?
--
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-unsubscr...@arrow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
