Skye Wanderman-Milne has posted comments on this change. Change subject: IMPALA-2548: Codegen Tuple::MaterializeExprs() and use in TopN node ......................................................................
Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/1901/9/be/src/runtime/tuple.h File be/src/runtime/tuple.h: Line 151: DCHECK_EQ(materialize_expr_ctxs.size(), desc.slots().size()); : StringValue** non_null_string_values_array = NULL; : if (collect_string_vals) { : non_null_string_values->clear(); : non_null_string_values->reserve(desc.string_slots().size()); : non_null_string_values_array = non_null_string_values->data(); : } : MaterializeExprsNoPool<collect_string_vals>(row, desc, materialize_expr_ctxs.data(), : non_null_string_values_array, total_string); > This function is inlined in the IR, and we look for this MaterializeExprsNo Just realized with the templates you would reintroduce the problem where both versions of the function have the same signature, so it would be harder to notice if you did mess up the replacement. Personally I think that's still better than duplicating this much code (you will eventually notice if you swap the replacement because you'll get memory corruption), but I could see it either way. -- 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: 9 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Skye Wanderman-Milne <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Skye Wanderman-Milne <[email protected]> Gerrit-HasComments: Yes
