Henry Robinson 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)) {
> Just passing through.
You could add an output parameter to BlockingPutWithTimeout() (rather than 
change the return type). Or the row batch queue should be of shared_ptr<>. I 
don't think we should rely on when move() is called in the blocking queue (and 
I'm not sure that analysis is quite right, as you would need to move() the row 
batch into AddBatchWithTimeout() to make it an rvalue-ref; i.e. before any 
timeout).

Sailesh - Where did you see discouraging using unique_ptr<> as an rvalue-ref? 
The idea of rvalues is that you move() into them, which means you are 
explicitly relinquishing ownership of the moved-from object.


-- 
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: 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

Reply via email to