Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
Message-Id: <201604131801.u3DI19lN031823@ip-10-146-233-104.ec2.internal>
Date: Wed, 13 Apr 2016 18:01:08 +0000
From: "Skye Wanderman-Milne (Code Review)" <gerrit@cloudera.org>
To: impala-cr@cloudera.com, dev@impala.incubator.apache.org
CC: Marcel Kornacker <marcel@cloudera.com>, Michael Ho <kwho@cloudera.com>
Reply-To: skye@cloudera.com
Subject: 
 =?UTF-8?Q?[Impala-CR](cdh5-trunk)_IMPALA-2548:_Codegen_Tuple::MaterializeExprs()_and_use_in_TopN_node=0A?=
In-Reply-To: 
 <gerrit.1453770925000.Ib422a8d50303c21c6a228675157bf867e8619444@gerrit.cloudera.org>
References: 
 <gerrit.1453770925000.Ib422a8d50303c21c6a228675157bf867e8619444@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

Skye Wanderman-Milne has posted comments on this change.

Change subject: IMPALA-2548: Codegen Tuple::MaterializeExprs() and use in TopN node
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/1901/10/be/src/exec/topn-node-ir.cc
File be/src/exec/topn-node-ir.cc:

Line 32:     insert_tuple->MaterializeExprs<false, false>(input_row, *materialized_tuple_desc_,
> i get that, but why is that needed? why do you need two different bodies, i
We don't need two different bodies for the codegen'd MaterializeExprs() function, just two different symbols. However, we do need two different bodies for the inlined MaterializeExprs() wrapper functions that deal with the vector operations, since they need to call the correct inner function.

To see what I mean, I originally had made two different functions, MaterializeExprs() and MaterializeExprsNoPool(). However, this caused a lot of copy/paste code as it required making two versions of the inlined wrapper methods, see http://gerrit.cloudera.org:8080/#/c/1901/9/be/src/runtime/tuple.h. There's some discussion between me and Michael on this at L159.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib422a8d50303c21c6a228675157bf867e8619444
Gerrit-PatchSet: 10
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-HasComments: Yes