wgtmac commented on code in PR #1743:
URL: https://github.com/apache/orc/pull/1743#discussion_r1449872337


##########
java/core/src/java/org/apache/orc/impl/WriterImpl.java:
##########
@@ -182,6 +184,8 @@ public WriterImpl(FileSystem fs,
 
     this.encodingStrategy = opts.getEncodingStrategy();
     this.compressionStrategy = opts.getCompressionStrategy();
+    this.compressionZstdLevel = opts.getCompressionZstdLevel();
+    this.compressionZstdWindowLog = opts.getCompressionZstdWindowLog();

Review Comment:
   Is it possible to work with compressionStrategy? For example, 
compressionStrategy indicates we need to look for external zstd options. BTW, 
are they actually used?



##########
java/core/src/java/org/apache/orc/OrcFile.java:
##########
@@ -934,6 +944,18 @@ public CompressionStrategy getCompressionStrategy() {
       return compressionStrategy;
     }
 
+    public int getCompressionZstdLevel() {

Review Comment:
   IMO, we'd better hide zstd-specific parameters from the public API. At least 
we can encapsulate them into a wrapper class.



##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -72,6 +72,18 @@ public enum OrcConf {
       "Define the compression strategy to use while writing data.\n" +
           "This changes the compression level of higher level compression\n" +
           "codec (like ZLIB)."),
+  COMPRESSION_ZSTD_LEVEL("orc.compression.zstd.level",
+      "hive.exec.orc.compression.zstd.level", 3,
+      "Define the compression level to use with ZStandard codec "
+          + "while writing data."),
+  COMPRESSION_ZSTD_WINDOWLOG("orc.compression.zstd.windowlog",
+      "hive.exec.orc.compression.zstd.windowlog", 0,
+      "Set the maximum allowed back-reference distance for "
+          + "ZStandard codec, expressed as power of 2."),
+  COMPRESSION_ZSTD_LONGMODE("orc.compression.zstd.longmode",

Review Comment:
   Have you tried this before? I'm not sure if it really works here since ORC 
usually does not employ large compression pages.



##########
java/core/src/java/org/apache/orc/OrcConf.java:
##########
@@ -72,6 +72,18 @@ public enum OrcConf {
       "Define the compression strategy to use while writing data.\n" +
           "This changes the compression level of higher level compression\n" +
           "codec (like ZLIB)."),
+  COMPRESSION_ZSTD_LEVEL("orc.compression.zstd.level",
+      "hive.exec.orc.compression.zstd.level", 3,
+      "Define the compression level to use with ZStandard codec "

Review Comment:
   It would be good to provide the valid value range as well.



-- 
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]

Reply via email to