wgtmac commented on a change in pull request #713:
URL: https://github.com/apache/orc/pull/713#discussion_r648423627



##########
File path: c++/src/Compression.cc
##########
@@ -1030,6 +1030,47 @@ DIAGNOSTIC_POP
     state = nullptr;
   }
 
+  /**
+   * Snappy block compression
+   */
+  class SnappyCompressionStream: public BlockCompressionStream {
+  public:
+    SnappyCompressionStream(OutputStream * outStream,
+                        int compressionLevel,
+                        uint64_t capacity,
+                        uint64_t blockSize,
+                        MemoryPool& pool)
+                        : BlockCompressionStream(outStream,
+                                                 compressionLevel,
+                                                 capacity,
+                                                 blockSize,
+                                                 pool) {
+    }
+
+    virtual std::string getName() const override {
+      return "SnappyCompressionStream";
+    }
+    
+    virtual ~SnappyCompressionStream() override {
+    }

Review comment:
       // PASS

##########
File path: c++/src/Compression.cc
##########
@@ -1030,6 +1030,47 @@ DIAGNOSTIC_POP
     state = nullptr;
   }
 
+  /**
+   * Snappy block compression
+   */
+  class SnappyCompressionStream: public BlockCompressionStream {
+  public:
+    SnappyCompressionStream(OutputStream * outStream,
+                        int compressionLevel,
+                        uint64_t capacity,
+                        uint64_t blockSize,
+                        MemoryPool& pool)
+                        : BlockCompressionStream(outStream,
+                                                 compressionLevel,
+                                                 capacity,
+                                                 blockSize,
+                                                 pool) {
+    }
+
+    virtual std::string getName() const override {
+      return "SnappyCompressionStream";
+    }
+    
+    virtual ~SnappyCompressionStream() override {
+    }
+
+  protected:
+    virtual uint64_t doBlockCompression() override;
+
+    virtual uint64_t estimateMaxCompressionSize() override {
+      return 
static_cast<uint64_t>(snappy::MaxCompressedLength(static_cast<size_t>(bufferSize)));

Review comment:
       break this long line into two

##########
File path: c++/src/Compression.cc
##########
@@ -1204,7 +1245,12 @@ DIAGNOSTIC_PUSH
         (new Lz4CompressionSteam(
           outStream, level, bufferCapacity, compressionBlockSize, pool));
     }
-    case CompressionKind_SNAPPY:
+    case CompressionKind_SNAPPY: {
+      int level = 0;

Review comment:
       Does snappy support different levels? If true, we should assign 
different levels according to CompressionStrategy




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to