Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11132 )
Change subject: IMPALA-7397: fix DCHECK in impala::AggregationNode::Close ...................................................................... IMPALA-7397: fix DCHECK in impala::AggregationNode::Close As part of IMPALA-110, all of the logic of performing aggregations was refactored out of the aggregation ExecNode and into Aggregators. Each Aggregator manages its own memory, so a DCHECK was added in AggregationNode::Close to ensure that no allocations were made from the regular ExecNode mem pools. This DCHECK is violated if the node was assigned conjuncts that allocate mem in Prepare - even though the conjuncts are evaluated in the Aggregator, we still initialize them in ExecNode::Prepare. This patch solves the problem by creating a copy of the TPlanNode without the conjuncts to pass to ExecNode. In the future, when TAggregator is introduced, we can get rid of this by directly assigning conjuncts to Aggregators. Note that this doesn't affect StreamingAggregationNode, which never has conjuncts assigned to it, but this patch fixes an incorrect DCHECK that enforces this. Testing: - Added a regression test for this case. Change-Id: I65ae00ac23a62ab9f4a7ff06ac9ad5cece80c39d Reviewed-on: http://gerrit.cloudera.org:8080/11132 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- M be/src/exec/aggregation-node.cc M be/src/exec/streaming-aggregation-node.cc M testdata/workloads/functional-query/queries/QueryTest/aggregation.test 3 files changed, 14 insertions(+), 2 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/11132 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I65ae00ac23a62ab9f4a7ff06ac9ad5cece80c39d Gerrit-Change-Number: 11132 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Marshall <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]>
