Marcel Kornacker has posted comments on this change. Change subject: IMPALA-3905: Add HdfsScanner::GetNext() interface and implementation for Parquet. ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/3801/1/be/src/exec/hdfs-scanner.h File be/src/exec/hdfs-scanner.h: Line 119: /// the batch is reset or destroyed, or a batch returned from a subsequent GetNext() more succinctly: or this or any subsequently returned batch is reset or destroyed. Line 137: virtual void Close(RowBatch* row_batch, bool add_to_queue); on second thought, add_to_queue is a bit odd (what if you never called ProcessSplit?). why not have ProcessSplit set an internal flag add_to_queue? (and have GetNext dcheck that it's not set; or have two flags to make sure that you only ever call one or the other) Line 139: RowBatch* batch() const { return batch_; } is this valid in GetNext mode? if not, add the above mentioned flags. http://gerrit.cloudera.org:8080/#/c/3801/1/be/src/exec/hdfs-sequence-scanner.h File be/src/exec/hdfs-sequence-scanner.h: Line 165: fix all trailing spaces -- To view, visit http://gerrit.cloudera.org:8080/3801 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iab50770bac05afcda4d3404fb4f53a0104931eb0 Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
