You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@parquet.apache.org by GitBox <gi...@apache.org> on 2023/01/14 13:14:33 UTC

[GitHub] [parquet-mr] yabola commented on a diff in pull request #1020: PARQUET-2226 Support merge bloom filters

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


##########
parquet-column/src/main/java/org/apache/parquet/column/values/bloomfilter/BloomFilter.java:
##########
@@ -176,4 +176,10 @@ public String toString() {
    * @return compress algorithm that the bloom filter apply
    */
   Compression getCompression();
+
+  /**
+   * Combines this Bloom filter with another Bloom filter by performing a bitwise OR of the underlying data
+   * @param otherBloomFilter The Bloom filter to combine this Bloom filter with.
+   */
+  void putAll(BloomFilter otherBloomFilter) throws IOException;

Review Comment:
   > IMHO, user needs to know if two BFs are compatible to merge before calling this function. So an utility function to test compatibility of two BFs is also required. WDYT?
   
   I had added some check before merging bloomFilter. I see that guava does not seem to have additional utility function.If necessary, I can add an utility function.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@parquet.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org