Github user xndai commented on a diff in the pull request:

    https://github.com/apache/orc/pull/126#discussion_r118048556
  
    --- Diff: c++/src/ByteRLE.cc ---
    @@ -27,6 +27,272 @@
     namespace orc {
     
       const size_t MINIMUM_REPEAT = 3;
    +  const size_t MAXIMUM_REPEAT = 127 + MINIMUM_REPEAT;
    +  const size_t MAX_LITERAL_SIZE = 128;
    +
    +  ByteRleEncoder::~ByteRleEncoder() {
    +    // PASS
    +  }
    +
    +  class ByteRleEncoderImpl : public ByteRleEncoder {
    +  public:
    +    ByteRleEncoderImpl(std::unique_ptr<BufferedOutputStream> output);
    +    virtual ~ByteRleEncoderImpl();
    +
    +    /**
    +     * Encode the next batch of values
    +     * @param data to be encoded
    +     * @param numValues the number of values to be encoded
    +     * @param notNull If the pointer is null, all values are read. If the
    +     *    pointer is not null, positions that are false are skipped.
    +     */
    +    virtual void add(const char* data, uint64_t numValues,
    +                      const char* notNull) override;
    +
    +    /**
    +     * Get size of buffer used so far.
    +     */
    +    virtual uint64_t getBufferSize() const override;
    +
    +    /**
    +     * Flushing underlying BufferedOutputStream
    +    */
    +    virtual uint64_t flush() override;
    +
    +    virtual void recordPosition(PositionRecorder* recorder) const override;
    +
    +  protected:
    +    std::unique_ptr<BufferedOutputStream> outputStream;
    +    char * literals;
    +    int numLiterals;
    +    bool repeat;
    +    int tailRunLength;
    +    int bufferPosition;
    +    int bufferLength;
    +    char * buffer;
    +
    +    void writeByte(char c);
    +    void writeValues();
    +    void write(char c);
    +  };
    +
    +  ByteRleEncoderImpl::ByteRleEncoderImpl(
    +                                std::unique_ptr<BufferedOutputStream> 
output)
    +                                  : outputStream(std::move(output)) {
    +    literals = new char[MAX_LITERAL_SIZE];
    --- End diff --
    
    thanks for catching that. I add a free() call. Regarding to RAII, I think 
it's a bit overkilled here. The array is guaranteed to be allocated in 
constructor and freed in destructor. The logic is quite clear. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to