Dan Hecht has posted comments on this change.

Change subject: IMPALA-3586: Implement union passthrough
......................................................................


Patch Set 10: Code-Review+2

(2 comments)

Do any of the tests exercise multiple child row-batches (to exercise that child 
Close() on next GetNext() call logic more)? If not, please add coverage for 
that (maybe by setting the row batch size to something smaller).

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

Line 124:   DCHECK(child_idx_ >= children_.size() || 
!IsChildPassThrough(child_idx_));
this dcheck isn't helpful given the inverse is used as the if-condition only 
two lines above


PS10, Line 228: this
Remove this and clean up the child Close() logic as part of...


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia8f6d5062724ba5b78174c3227a7a796d10d8416
Gerrit-PatchSet: 10
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: Alex Behm <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Marcel Kornacker <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to