[Impala-ASF-CR] IMPALA-4883: Union Codegen

["Tim Armstrong (Code Review)" <ge...@cloudera.org>]

Tim Armstrong has posted comments on this change.

Change subject: IMPALA-4883: Union Codegen
......................................................................


Patch Set 6: Code-Review+1

(4 comments)

Looks good. I'll let Michael finish his review - he can probably do the +2.

http://gerrit.cloudera.org:8080/#/c/6459/6/be/src/exec/union-node.cc
File be/src/exec/union-node.cc:

Line 98: 
Extra blank lines.


Line 235:     if (ReachedLimit()) {
I don't think this can be taken now, right? Since we don't increment num_rows_returned_ until later.


http://gerrit.cloudera.org:8080/#/c/6459/6/be/src/exec/union-node.h
File be/src/exec/union-node.h:

Line 116:   /// call on the child.
Maybe mention that these GetNext* functions don't apply the limit?


PS6, Line 127:  Increments 'num_rows_returned_' and
             :   /// 'child_row_idx_'
I think this needs an update.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib4107d27582ff5416172810364a6e76d3d93c439
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Michael Ho <kw...@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tb...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-HasComments: Yes