Dan Hecht has posted comments on this change. Change subject: IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans. ......................................................................
Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/6527/7/be/src/exec/base-sequence-scanner.h File be/src/exec/base-sequence-scanner.h: PS7, Line 40: nit: extra space http://gerrit.cloudera.org:8080/#/c/6527/7/be/src/exec/hdfs-scanner.cc File be/src/exec/hdfs-scanner.cc: PS7, Line 134: final_batch this looked leaked... the function comment claims it's added to the queue. also, what other class implements this virtual function? http://gerrit.cloudera.org:8080/#/c/6527/7/be/src/exec/kudu-scan-node.cc File be/src/exec/kudu-scan-node.cc: Line 183: // Make sure that we still own the RowBatch if BlockingPutWithTimeout() timed out. how does that work? the move() on line 180 doesn't move in the case of timeout? does this path get exercised? -- To view, visit http://gerrit.cloudera.org:8080/6527 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ie18f57b0d3fe0052a8ccd361b6a5fcdf979d0669 Gerrit-PatchSet: 7 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Alex Behm <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: anujphadke <[email protected]> Gerrit-HasComments: Yes
