Dan Hecht has posted comments on this change.

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


Patch Set 16:

(1 comment)

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

Line 91:   bool const_todo_;
> Minor naming thing, I don't feel too strongly but 'todo' seems unusual. Alt
Rather than having these variables, how about just saving the index of the 
first materialize child.  Then, these can be simple functions, e.g. 
HasMorePassthrough(), HasMoreMaterialized(), that is derived from existing 
state. that way, there's less dynamic state that needs to be updated and 
reasoned about.


-- 
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: 16
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