Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/15034 )
Change subject: KUDU-2483 Integrate BlockBloomFilter with ColumnPredicate on server side ...................................................................... Patch Set 4: (4 comments) http://gerrit.cloudera.org:8080/#/c/15034/4/src/kudu/common/column_predicate.h File src/kudu/common/column_predicate.h: http://gerrit.cloudera.org:8080/#/c/15034/4/src/kudu/common/column_predicate.h@152 PS4, Line 152: std::vector<BlockBloomFilterInner>&& bfs, > Here and below you don't need the rvalue reference; could just pass std::ve I understand but using rvalue reference as type for bfs forces caller to pass rvalue(using std::move or directly) and avoid the costly copy of a vector by mistake. Hence this is deliberate. http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h File src/kudu/util/block_bloom_filter.h: http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h@78 PS3, Line 78: // Initialize the internal data structures using the supplied arguments. : // Useful for de-serializing the BlockBloomFilter. : Status Init(int log_space_bytes, const void* src_data, size_t src_len, bool always_false); > Good point. Thanks for the tip. Done. http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.h@121 PS3, Line 121: // Returns amount of space used in log2 bytes. : int GetLogSpaceBytes() const { : return log_num_buckets_ + kLogBucketByteSize; : } > Same as above. Done http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.cc File src/kudu/util/block_bloom_filter.cc: http://gerrit.cloudera.org:8080/#/c/15034/3/src/kudu/util/block_bloom_filter.cc@108 PS3, Line 108: > Ths Init variant calls InitInternal (memset on L91) and then calls memcpy ( Intent was to move the memset() outside of InitInternal() but missed removing that line from InitInternal(). Fixed. -- To view, visit http://gerrit.cloudera.org:8080/15034 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7ecfd67e9c5fbe459c5b4aed91e0be2a194d433a Gerrit-Change-Number: 15034 Gerrit-PatchSet: 4 Gerrit-Owner: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Bankim Bhavsar <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu <[email protected]> Gerrit-Reviewer: ZhangYao <[email protected]> Gerrit-Reviewer: helifu <[email protected]> Gerrit-Comment-Date: Fri, 17 Jan 2020 00:08:51 +0000 Gerrit-HasComments: Yes
