Alex Behm 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 des Done 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 Proc In that case, I think it's cleaner to declare the intent in the c'tor because batch() and Close() can be called before even calling ProcessSplit(), e.g., if Open() fails. Added the c'tor param. Line 139: RowBatch* batch() const { return batch_; } > is this valid in GetNext mode? if not, add the above mentioned flags. Added comment and DCHECK. 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 Done -- 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
