impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Henry Robinson (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.
Date Wed, 21 Jun 2017 18:33:11 GMT
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)
File be/src/exec/

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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie18f57b0d3fe0052a8ccd361b6a5fcdf979d0669
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Henry Robinson <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-Reviewer: Sailesh Mukil <>
Gerrit-Reviewer: Tim Armstrong <>
Gerrit-Reviewer: anujphadke <>
Gerrit-HasComments: Yes

View raw message