Michael Ho has posted comments on this change.

Change subject: IMPALA-3154: Check for UDF errors in in the Kudu scanner
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/2764/1/be/src/exec/kudu-scanner.cc
File be/src/exec/kudu-scanner.cc:

Line 185: ExprContext::FreeLocalAllocations(conjunct_ctxs_);
Not this change but why do we FreeLocalAllocations() per range instead of 
per-row batch. This seems to be inconsistent with other scanners.


Line 250: state_->GetQueryStatus();
Would be good to follow the pattern of other scanners of doing UDF status check 
at the same location as FreeLocalAllocations(). Please see 
HdfsScanner::CommitRows() as reference. In fact, is there any chance kudu 
scanner may utilize HdfsScanner::CommitRows() so as to avoid duplicated logic 
here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I04c8f03860d2044bae7ed87560687af63e016ce0
Gerrit-PatchSet: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Casey Ching <[email protected]>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-HasComments: Yes

Reply via email to