Michael Ho has posted comments on this change. Change subject: IMPALA-3286: prefetching for PartitionedAggregationNode ......................................................................
Patch Set 11: (10 comments) http://gerrit.cloudera.org:8080/#/c/3070/11/be/src/exec/partitioned-aggregation-node.cc File be/src/exec/partitioned-aggregation-node.cc: Line 278: LlvmCodeGen* codegen; : RETURN_IF_ERROR(state->GetCodegen(&codegen)); Not needed in this function anymore ? Line 282: codegen_enabled = codegen_status.ok(); This line and line 285 can be probably be merged. Line 343: // There is grouping, so we will do partitioned aggregation. This line can be moved to line 339 instead as this should not be specific to the codegen case. Line 1412: hash_partitions_.clear(); May be we should reset hash_tbls_[] here too ? http://gerrit.cloudera.org:8080/#/c/3070/11/be/src/exec/partitioned-aggregation-node.h File be/src/exec/partitioned-aggregation-node.h: Line 232: nit: extra space. Same below. Line 400: hash_tbls_ nit: 'hash_tbls_' Line 499: stores the results Can you please mention the null bool and hash values are also cached ? Line 500: 'ht_ctx'. the expression values cache in 'ht_ctx'. Line 501: nit: extra space. Line 581: HashTable* hash_tbl Can you please add a comment about this new parameter ? -- To view, visit http://gerrit.cloudera.org:8080/3070 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7726454efb416d61080c4e11db0ee7ada18c149b Gerrit-PatchSet: 11 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Tim Armstrong <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
