Alex Behm has posted comments on this change.

Change subject: IMPALA-4285/IMPALA-4286: Fixes for Parquet scanner with MT_DOP 
> 0.
......................................................................


Patch Set 4:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/4767/4//COMMIT_MSG
Commit Message:

Line 17: HdfsScanNodeMt::GetNext().
> why do we have this check in HdfsScanNodeBase::Open() in the first place?
Removed the short-circuit in Open() instead.


http://gerrit.cloudera.org:8080/#/c/4767/4/be/src/exec/hdfs-scan-node-mt.cc
File be/src/exec/hdfs-scan-node-mt.cc:

PS4, Line 71: ,
> did you mean to say more or make it a period?
I removed the short-circuit in Open() instead.


Line 72:   if (file_descs_.empty()) {
> this is okay, but why can't we just delete the equivalent check in HdfsScan
No, removed it.


http://gerrit.cloudera.org:8080/#/c/4767/4/be/src/exec/hdfs-scanner.cc
File be/src/exec/hdfs-scanner.cc:

PS4, Line 230: row
> "row" seems misleading
Not relevant anymore, I also removed the conjunct eval. Tests passed.


PS4, Line 230: an
> and
Done


Line 236:   // Set remaining tuples in row.
> misleading comment
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4767
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I79c0f6fd2aeb4bc6fa5f87219a485194fef2db1b
Gerrit-PatchSet: 4
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-HasComments: Yes

Reply via email to