Henry Robinson 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);
Done


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 deprec
Done (I'll let John know separately)


Line 490: .
> briefly explain "why". The "what" is pretty clear from the code.  Is it to 
The main reason is to save the cost of applying the filter in the scan - 
particularly for row filters. I've expanded the comment to give list the main 
savings.

I ran a targeted benchmark to measure the cost of building a filter that will 
never be used vs disabling runtime filters altogether. If the probe side is 
100M rows (on a single node), building the filter takes about 3% longer than 
not doing so. See the commit message for details.


Line 496: Produced
> Maybe say "Published" since the filter is always created/produced in the ne
Done


-- 
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

Reply via email to