Yao Xu has posted comments on this change. ( http://gerrit.cloudera.org:8080/13703 )
Change subject: KUDU-2483 (part 1/3) Add bloom filter predicate for Java API ...................................................................... Patch Set 1: (3 comments) I will first answer some implementation questions. If there are no problem, I will update this patch later. :) http://gerrit.cloudera.org:8080/#/c/13703/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/13703/1//COMMIT_MSG@13 PS1, Line 13: and fixed some bugs in bloom filter of Java. > Are there unit tests for BloomFilter? Could you add some? Currently, we verified by scanning with BloomFilter predicate. 'm not sure how to do this test better, because I have to compare the results of Java BloomFilter and C++ BloomFilter. http://gerrit.cloudera.org:8080/#/c/13703/1//COMMIT_MSG@16 PS1, Line 16: the DECIMAL type. I will support it in later patches. > What is the challenge with supporting DECIMAL? I think I need to look at the encoding of DECIMAL type to make sure it's the same as the service-side hashcode. http://gerrit.cloudera.org:8080/#/c/13703/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java File java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java: http://gerrit.cloudera.org:8080/#/c/13703/1/java/kudu-client/src/main/java/org/apache/kudu/util/BloomFilter.java@65 PS1, Line 65: private final byte[] bitSet; > I am curious why you changed to a byte array? > > Should this be called `bytes` now? `bitSet seems wrong. The key point of the BloomFilter Predicate is to ensure that the BloomFilter on the client-side and server-side must be the same. However, I found that the length of the bytes array returned by the BitSet.toByteArray is calculated based on the actual set position. This causes us to fail to generate the same BloomFilter on the server-side. Maybe we can solve this problem with BitSet.valueOf, but I think it might be better to use a byte array. Maybe this member variable is better called bitmap? -- To view, visit http://gerrit.cloudera.org:8080/13703 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If96ec164e86091f8fb3b7f92b3623ca3e728edfd Gerrit-Change-Number: 13703 Gerrit-PatchSet: 1 Gerrit-Owner: Yao Xu <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Yao Xu <[email protected]> Gerrit-Comment-Date: Sat, 22 Jun 2019 04:33:17 +0000 Gerrit-HasComments: Yes
