Dan Hecht has posted comments on this change.

Change subject: IMPALA-3311: fix string data coming out of aggs in subplans
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/2929/6/be/src/exec/partitioned-aggregation-node.cc
File be/src/exec/partitioned-aggregation-node.cc:

Line 375:   // FunctionContextImpl::AllocateLocal(), and are ultimately backed 
by 'agg_fn_pool_'.)
the comment for agg_fn_ctxs_ says the allocations come from here only in the 
non-grouping case.


Line 380: NYI
Not yet implemented implies that we should implement it, so is this really a 
NYI, or just a normal invariant of the code?


http://gerrit.cloudera.org:8080/#/c/2929/6/be/src/exec/partitioned-aggregation-node.h
File be/src/exec/partitioned-aggregation-node.h:

Line 397: won't be cleared
this is a bit ambiguous, eventually it will get cleared. Maybe: has a lifetime 
at least as long as the batch's tuples? or something similar?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada891504c261ba54f4eb8c9d7e4e5223668d7b9
Gerrit-PatchSet: 6
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Skye Wanderman-Milne <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to