Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
Message-Id: <201605111912.u4BJCIPg001616@ip-10-146-233-104.ec2.internal>
Date: Wed, 11 May 2016 19:12:18 +0000
From: "Tim Armstrong (Code Review)" <gerrit@cloudera.org>
To: Skye Wanderman-Milne <skye@cloudera.com>, impala-cr@cloudera.com,
        dev@impala.incubator.apache.org
CC: Dan Hecht <dhecht@cloudera.com>
Reply-To: tarmstrong@cloudera.com
Subject: =?UTF-8?Q?[Impala-CR](cdh5-trunk)_IMPALA-3311:_fix_string_data_coming_out_of_aggs_in_subplans=0A?=
In-Reply-To: <gerrit.1462223464000.Iada891504c261ba54f4eb8c9d7e4e5223668d7b9@gerrit.cloudera.org>
References: <gerrit.1462223464000.Iada891504c261ba54f4eb8c9d7e4e5223668d7b9@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.10-rc0
archived-at: Wed, 11 May 2016 19:12:23 -0000

Tim Armstrong has posted comments on this change.

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


Patch Set 1:

(2 comments)

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

Line 370:   for (int i = 0; i < aggregate_evaluators_.size(); ++i) {
> I moved the string copying code to its own function. Lemme know if you thin
It would be good to factor this whole loop out, e.g. into HandleLocalStringAllocations() (or some other name, that's a little clunky).

I think then we can bail out of the function early if (!needs_finalize_ && !needs_serialize_).


Line 374:     if (IsInSubplan()) {
> I'm having trouble writing a good query that really isolates this effect an
Thanks for doing this, it's good to confirm that there's a real effect. I also commented on the query file itself.


-- 
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: 1
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes