impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Hecht (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3586: Implement union passthrough
Date Thu, 09 Mar 2017 04:56:42 GMT
Dan Hecht has posted comments on this change.

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


Patch Set 11:

> No, we do not break out of the loop, we return right away (see line
 > 149 in the original union-node.cc). The next getnext call can
 > return 0 rows.

I'm talking about the case where we don't hit the limit. We normally won't take the return
at line 149 (unless we happen to fill the row batch at the same time as finishing the children,
but that's not the usual case). Instead, if we still have room after evaluating the children
(and constant exprs), we'll get all the way to 192.  So, the normal non-limit case is inconsistently
handled.  I agree it's not a functional bug, though.

It'd still be nice to exercise the multiple child row-batch path, if we're not already. Hopefully
we already are (row-batch-size might be a exec option dimension in pytest), but I just thought
it'd be good to check that.

-- 
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: 11
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Taras Bobrovytsky <tbobrovytsky@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: No

Mime
View raw message