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 <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-HasComments: Yes
