Dan Hecht has posted comments on this change. Change subject: IMPALA-3077: Enable runtime filters when PHJ spills ......................................................................
Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/2783/5/be/src/exec/partitioned-hash-join-node-ir.cc File be/src/exec/partitioned-hash-join-node-ir.cc: Line 276: if (build_filters) { DCHECK_EQ(ht_ctx_->level(), 0); http://gerrit.cloudera.org:8080/#/c/2783/5/be/src/exec/partitioned-hash-join-node.cc File be/src/exec/partitioned-hash-join-node.cc: Line 40: Deprecated Please add a quick note to the commit message explaining why this is deprecated. Also, is this flag documented, and if so please let John know. Line 490: . briefly explain "why". The "what" is pretty clear from the code. Is it to save the work on the probe scan side? save transmission costs? both? Also, as mentioned earlier, have you checked the CPU cost of always doing the filter hash/insertions? Moving the code to IR won't make it run any faster (eventually we need to unroll it and propagate the constants) so I thought part of reason we disabled was to save the CPU cost of building the filter. Is that not the case? Line 496: Produced Maybe say "Published" since the filter is always created/produced in the new code. -- To view, visit http://gerrit.cloudera.org:8080/2783 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I59a2d9ee03ccea6b674392584e4c7f272233571e Gerrit-PatchSet: 5 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
