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 <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Taras Bobrovytsky <tbobrovyt...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-HasComments: Yes