Henry Robinson has posted comments on this change. Change subject: IMPALA-3007: Adjust Bloom Filter size according to NDV estimate ......................................................................
Patch Set 5: (3 comments) http://gerrit.cloudera.org:8080/#/c/2812/5/be/src/runtime/runtime-filter.h File be/src/runtime/runtime-filter.h: Line 100: int64_t GetFilterSizeForNdv(int64_t ndv); > does this need to be public? also, this can be a static function for Runtim Doesn't need to be public, but relies on default_log_filter_size_ so can't be static. Line 155: filter_size_(filter_size) { > if this is computed from filter_desc_ (as indicated in l212), then why pass Although the comment below was accurate, it suggested the computation was done in this class, so I removed it. http://gerrit.cloudera.org:8080/#/c/2812/5/testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test File testdata/workloads/functional-query/queries/QueryTest/runtime_filters.test: Line 344: row_regex: .*All 1 Runtime Filter\(s\) Published.* > this (s) thing is a bit annoying, why don't you alter the string based on t Done -- To view, visit http://gerrit.cloudera.org:8080/2812 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I1fe37b8d4cfb3c52bb8e8cf0ca55e92665b87803 Gerrit-PatchSet: 5 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Henry Robinson <[email protected]> Gerrit-Reviewer: Henry Robinson <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Mostafa Mokhtar <[email protected]> Gerrit-HasComments: Yes
