impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2548: Codegen Tuple::MaterializeExprs() and use in TopN node
Date Fri, 18 Mar 2016 18:24:22 GMT
Michael Ho has posted comments on this change.

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


Patch Set 9:

(3 comments)

Some more comments. I think we are close.

http://gerrit.cloudera.org:8080/#/c/1901/9/be/src/runtime/tuple.cc
File be/src/runtime/tuple.cc:

Line 202: if (collect_string_vals) *total_string = 0;
Can this be moved to the body if (collect_string_vals) stmt of the caller in the header file
so the relevant parameters are initialized together ?


Line 326:   //     StringValue** non_null_string_values, int* total_string)
Please consider also adding the signature of the NoPool version.


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);
Why cannot this simply be  MaterializeExprs<collect_string_vals>(row, desc, materialize_expr_ctxs,
NULL, non_null_string_values, total_string); ?


-- 
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 <skye@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message