Repository: incubator-impala Updated Branches: refs/heads/master 67b63f37e -> 8a77fff9a
IMPALA-5551: Fix AggregationNode::Close() when Prepare() fails A recent change moved the initialization of output_tuple_desc_ to the constructor of AggregationNode. This changes the order in which tuple_pool_ and output_tuple_desc_ are initialized. The code in AggregationNode::Close() made the assumption that tuple_pool_ cannot be NULL (although without a DCHECK) if output_tuple_desc_ is not NULL. This no longer holds in the new code. This change makes AggregationNode::Close() checks tuple_pool_ to see if it's NULL before using it. Change-Id: Ia0f8cce7cf6e3183d3a5e145c2fcfa50f36c77e0 Reviewed-on: http://gerrit.cloudera.org:8080/7254 Reviewed-by: Tim Armstrong <[email protected]> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/384e864f Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/384e864f Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/384e864f Branch: refs/heads/master Commit: 384e864fa19cc004e66c4139508cf44a41e0479c Parents: 67b63f3 Author: Michael Ho <[email protected]> Authored: Wed Jun 21 09:01:41 2017 -0700 Committer: Impala Public Jenkins <[email protected]> Committed: Mon Jun 26 23:14:07 2017 +0000 ---------------------------------------------------------------------- be/src/exec/aggregation-node.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/384e864f/be/src/exec/aggregation-node.cc ---------------------------------------------------------------------- diff --git a/be/src/exec/aggregation-node.cc b/be/src/exec/aggregation-node.cc index 17820ff..13f7dd3 100644 --- a/be/src/exec/aggregation-node.cc +++ b/be/src/exec/aggregation-node.cc @@ -107,6 +107,7 @@ Status AggregationNode::Init(const TPlanNode& tnode, RuntimeState* state) { } Status AggregationNode::Prepare(RuntimeState* state) { + DCHECK(output_iterator_.AtEnd()); SCOPED_TIMER(runtime_profile_->total_time_counter()); RETURN_IF_ERROR(ExecNode::Prepare(state)); @@ -268,7 +269,8 @@ void AggregationNode::Close(RuntimeState* state) { // but we don't actually need the result, so allocate a single dummy tuple to avoid // accumulating memory. Tuple* dummy_dst = nullptr; - if (needs_finalize_ && output_tuple_desc_ != nullptr) { + // 'tuple_pool_' can be NULL if Prepare() failed. + if (needs_finalize_ && tuple_pool_.get() != nullptr) { dummy_dst = Tuple::Create(output_tuple_desc_->byte_size(), tuple_pool_.get()); } while (!output_iterator_.AtEnd()) {
