impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3905: HdfsScanner::GetNext() for Avro, RC, and Seq scans.
Date Thu, 29 Jun 2017 05:20:59 GMT
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 <alex.behm@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Henry Robinson <henry@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Sailesh Mukil <sailesh@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: anujphadke <aphadke@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message