[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/41 @omalley https://issues.apache.org/jira/browse/ORC-127 https://github.com/apache/orc/pull/77 Thanks. --- If your project is set up for it, you can reply to this email and have your repl

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user omalley commented on the issue: https://github.com/apache/orc/pull/41 Deepak, We got a notice of a new problem from Coverity. Can you look at it? ** CID 173749: Uninitialized members (UNINIT_CTOR) /c++/src/Reader.cc: 259 in orc::RowReaderImpl::Ro

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/41 I updated the patch to use a `shared_ptr` for `FileContents`. --- 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 no

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user omalley commented on the issue: https://github.com/apache/orc/pull/41 I think we should have FileContents be a shared ptr so that if the user deallocates the Reader, the RowReader won't reach into deallocated memory. --- If your project is set up for it, you can reply to

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/41 Replaced with REDUNDANT_MOVE. valgrind does not show any memory leaks due to this patch. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as we

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user omalley commented on the issue: https://github.com/apache/orc/pull/41 This is looking good. To compile on MacOS, I needed to replace some of the std::move with REDUNDANT_MOVE. See omalley:pr/41 --- If your project is set up for it, you can reply to this email and have yo

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/41 Included review comments. This patch is huge because I split the monolithic Reader.cc into various files - Reader.hh/Reader.cc: ReaderImpl and RowReaderImpl - Statistics.hh/Statistics.cc:

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user omalley commented on the issue: https://github.com/apache/orc/pull/41 The ReaderOptions needs to split in half too: * ReaderOptions * setTailLocation * setErrorStream * setSerializedFooter * RowReaderOptions * include * range

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user swalkaus commented on the issue: https://github.com/apache/orc/pull/41 I might rename "Reader" something like "FileTailReader" but, in general, the refactoring makes sense. The schema can be read and columns metadata known before column selection is decided. --- If your

[GitHub] orc issue #41: ORC-58: Move code for reading rows from Reader to RowReader

Github user majetideepak commented on the issue: https://github.com/apache/orc/pull/41 @omalley, please let me know your feedback. Thanks! --- 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