Marcel Kornacker has posted comments on this change. Change subject: IMPALA-2548: Codegen Tuple::MaterializeExprs() and use in TopN node ......................................................................
Patch Set 10: (2 comments) 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_, > We don't need two different bodies for the codegen'd MaterializeExprs() fun you need to describe that somewhere in the code. right now, i'm wondering why this function needs template parameters at all. http://gerrit.cloudera.org:8080/#/c/1901/10/be/src/exec/topn-node.cc File be/src/exec/topn-node.cc: Line 79: sort_exec_exprs_.sort_tuple_slot_expr_ctxs(), NULL, &materialize_exprs_no_pool_fn)); what's the problem with having just a single symbol? -- 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 <s...@cloudera.com> Gerrit-Reviewer: Marcel Kornacker <mar...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Skye Wanderman-Milne <s...@cloudera.com> Gerrit-HasComments: Yes
