Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5173: crash with hash join feeding directly into nlj
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/6568/2/be/src/exec/nested-loop-join-builder.cc
File be/src/exec/nested-loop-join-builder.cc:

Line 55:     // data in that cases. TODO: remove workaround when IMPALA-4179 is 
fixed
> this is so confusing.  here you say we can't acquire ownership of these, ye
Yeah when the block gets attached the batch has control over deletion, but the 
accounting isn't updated. Maybe there's a better way of expressing this, e.g.
"Transferring ownership doesn't correctly update memory accounting - see 
IMPALA-4179"

And yeah, this is the only place where we check needs_deep_copy() in this way.

It's a mess I agree but I don't think we can do a proper fix yet.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I0c04952e591d17e5ff7e994884be4c4c899ae192
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Dan Hecht <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to