[ 
https://issues.apache.org/jira/browse/PARQUET-2226?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17677157#comment-17677157
 ] 

ASF GitHub Bot commented on PARQUET-2226:
-----------------------------------------

wgtmac commented on code in PR #1020:
URL: https://github.com/apache/parquet-mr/pull/1020#discussion_r1070853151


##########
parquet-column/src/main/java/org/apache/parquet/column/values/bloomfilter/BlockSplitBloomFilter.java:
##########
@@ -394,4 +395,24 @@ public long hash(float value) {
   public long hash(Binary value) {
     return hashFunction.hashBytes(value.getBytes());
   }
+
+  @Override
+  public void merge(BloomFilter otherBloomFilter) throws IOException {
+    Preconditions.checkArgument(otherBloomFilter != null, "Cannot merge a null 
BloomFilter");
+    Preconditions.checkArgument((getAlgorithm() == 
otherBloomFilter.getAlgorithm()),
+      "BloomFilters must have the same algorithm (%s != %s)",
+        getAlgorithm(), otherBloomFilter.getAlgorithm());
+    Preconditions.checkArgument((getHashStrategy() == 
otherBloomFilter.getHashStrategy()),
+      "BloomFilters must have the same hashStrategy (%s != %s)",
+        getHashStrategy(), otherBloomFilter.getHashStrategy());
+    Preconditions.checkArgument((getBitsetSize() == 
otherBloomFilter.getBitsetSize()),

Review Comment:
   Maybe we should put this check to the 1st place to make it fail fast. 
Usually parameters above do not change.



##########
parquet-column/src/main/java/org/apache/parquet/column/values/bloomfilter/BlockSplitBloomFilter.java:
##########
@@ -394,4 +395,24 @@ public long hash(float value) {
   public long hash(Binary value) {
     return hashFunction.hashBytes(value.getBytes());
   }
+
+  @Override
+  public void merge(BloomFilter otherBloomFilter) throws IOException {

Review Comment:
   I still think it is helpful to extract the check logic to a separate method 
like `boolean canMergeFrom(BloomFilter other)` because user has to check these 
parameters anyway before calling the `merge` method. Then `merge` 
implementation can simply call `canMergeFrom`.





> Support merge Bloom Filter
> --------------------------
>
>                 Key: PARQUET-2226
>                 URL: https://issues.apache.org/jira/browse/PARQUET-2226
>             Project: Parquet
>          Issue Type: Improvement
>            Reporter: Mars
>            Priority: Major
>
> We need to collect Parquet's bloom filter of multiple files, and then 
> synthesize a more comprehensive bloom filter for common use. 
> Guava supports similar api operations
> https://guava.dev/releases/31.0.1-jre/api/docs/src-html/com/google/common/hash/BloomFilter.html#line.252



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to