Sailesh Mukil has posted comments on this change. Change subject: IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans. ......................................................................
Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/6527/5/be/src/exec/kudu-scan-node.cc File be/src/exec/kudu-scan-node.cc: Line 179: if (materialized_row_batches_->AddBatchWithTimeout(move(row_batch), 1000000)) { > I think BlockingPutWithTimeout() actually already supports this fine since Just passing through. I agree with Tim's approach, however, passing a unique_ptr as an rvalue-ref is usually discouraged since the lifetime of the member from the caller side('row_batch' here) completely depends on the implementation of the function it's being passed to, which may not be evident to the caller. That being said, it's that ambiguity that helps us solve this problem and I don't see an easier way to do it, but leaving a comment about this would be good since this is so subtle, and would help from a readability perspective. -- 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: 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: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: anujphadke <[email protected]> Gerrit-HasComments: Yes
