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

    https://github.com/apache/orc/pull/126#discussion_r117869507
  
    --- 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 --
    
    are you freeing this anywhere? RAII is a preferred technique for these 
instances.
    Reference: http://en.cppreference.com/w/cpp/language/raii
    add a `std::vector<char> literals_buf` to the fields above. 
`literals_buf.resize(MAX_LITERAL_SIZE)` and point `literals = 
literals_buf.data()` instead to avoid using a smart pointer.



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