Tim Armstrong has posted comments on this change. Change subject: IMPALA-3905: Add HdfsScanner::GetNext() interface and implementation for Parquet. ......................................................................
Patch Set 5: Code-Review+1 (1 comment) LGTM, I think the comment just needs updating. http://gerrit.cloudera.org:8080/#/c/3801/5/be/src/exec/hdfs-scanner.h File be/src/exec/hdfs-scanner.h: Line 123: /// The memory referenced by the tuples is valid until either Close() is called, Now that I think about it, I think the bit about "Close is called" is wrong for Scanners (although it is true for ExecNodes). I think the second clause should be the only rule. We should be able to close a scanner while its batches are still in the queue. The code looks like it implements that correctly (otherwise why would we bother passing along resources on Close()), just the comment is wrong. -- 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: 5 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-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
