wgtmac commented on code in PR #1020:
URL: https://github.com/apache/parquet-mr/pull/1020#discussion_r1070233210
##########
parquet-column/src/main/java/org/apache/parquet/column/values/bloomfilter/BlockSplitBloomFilter.java:
##########
@@ -394,4 +395,21 @@ public long hash(float value) {
public long hash(Binary value) {
return hashFunction.hashBytes(value.getBytes());
}
+
+ @Override
+ public void putAll(BloomFilter otherBloomFilter) throws IOException {
Review Comment:
Could you add some tests to verify if the merged BF is as expected?
##########
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?
##########
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:
Rename to `merge`?
--
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-unsubscr...@parquet.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
