Dan Hecht has posted comments on this change.

Change subject: IMPALA-3645: Free probe expressions' local allocations in 
ConstructBuildSide()
......................................................................


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/3253/2/be/src/exec/partitioned-hash-join-node.cc
File be/src/exec/partitioned-hash-join-node.cc:

Line 627:   // IMPALA-3645 for details.
I think this should be reworded to motivate it differently based on how the 
code works (like your commit message, rather than based on the DCHECK). 
Something like:

// The prepare functions ... Lower()). The probe expr local allocations need to 
be freed now since they don't get freed again until probing. Other exprs' local 
allocations will be freed by ExecNode::FreeLocalAllocations().


-- 
To view, visit http://gerrit.cloudera.org:8080/3253
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2096ca3e2093c5ab0ecc0e7ca4cd1b5f3c1ed1ed
Gerrit-PatchSet: 2
Gerrit-Project: Impala
Gerrit-Branch: cdh5-2.6.0_5.8.0
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to