Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15683 )

Change subject: IMPALA-3741 [part 2]: Push runtime bloom filter to Kudu
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/15683/4/be/src/util/bloom-filter.h
File be/src/util/bloom-filter.h:

http://gerrit.cloudera.org:8080/#/c/15683/4/be/src/util/bloom-filter.h@80
PS4, Line 80: ublic:
> unnecessary
It's defined to support the "clone" virtual function in base class.


http://gerrit.cloudera.org:8080/#/c/15683/4/be/src/util/bloom-filter.h@91
PS4, Line 91:   std::shared_ptr<kudu::BlockBloomFilterBufferAllocatorIf> 
Clone() const override {
> I don't understand what this is supposed to do - why wouldn't the cloned al
Yes, we cannot get the buffer pool from runtime filter bank without calling 
context.


http://gerrit.cloudera.org:8080/#/c/15683/4/common/thrift/ImpalaService.thrift
File common/thrift/ImpalaService.thrift:

http://gerrit.cloudera.org:8080/#/c/15683/4/common/thrift/ImpalaService.thrift@521
PS4, Line 521:
> Seems a little confusing, since users don't specify the numbers they use "B
Fixed


http://gerrit.cloudera.org:8080/#/c/15683/4/common/thrift/ImpalaService.thrift@524
PS4, Line 524: R_TYP
> We'll eventually want to apply min-max filters in other places like HDFS sc
Fixed


http://gerrit.cloudera.org:8080/#/c/15683/4/common/thrift/PlanNodes.thrift
File common/thrift/PlanNodes.thrift:

http://gerrit.cloudera.org:8080/#/c/15683/4/common/thrift/PlanNodes.thrift@124
PS4, Line 124: MIN_MAX = 2
> We probably eventually want to add in-list filters as well, and we won't wa
The code for showing the current settings in impala-shell are auto generated. 
Still try to figure out how to show the setting for enabled filter type as 
comma separated list of filter type.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9100076f68ea299ddb6ec8bc027cac7a47f5d754
Gerrit-Change-Number: 15683
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Aman Sinha <[email protected]>
Gerrit-Reviewer: Bankim Bhavsar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 17 Apr 2020 18:57:49 +0000
Gerrit-HasComments: Yes

Reply via email to