addthis / stream-lib

Stream summarizer and cardinality estimator.
Apache License 2.0
2.26k stars 556 forks source link

Add support for intersection to BloomFilter. #134

Open b4hand opened 7 years ago

b4hand commented 7 years ago

I realize that intersection will have the false positive rate of the maximum of the false positive rates of the intersected bloom filters, but this is a useful feature. You already provide support for unions via the merge method.

One concerning thing about the existing union support is that it doesn't check the size of the underlying bitset. This should probably be added to the checks for compatibility purposes before performing a merge. Similarly, it would be nice to add that check for intersection as well.

I'm willing to add code for isCompatible or similar to confirm compatibility before doing merges or intersection if you will allow that change.

b4hand commented 7 years ago

Build is failing for the same reason as #133 which is more evidence that it is unrelated to this code.

b4hand commented 7 years ago

Looking at the build history #108 has the same build failure as well, so it seems highly unlikely that the build failure is due to this code.

b4hand commented 7 years ago

I rebased to the latest master which drops the openjdk7 build, so the build now passes.

abramsm commented 7 years ago

OK to close in favor of https://github.com/addthis/stream-lib/pull/136?