aherbert commented on code in PR #358:
URL:
https://github.com/apache/commons-collections/pull/358#discussion_r1023145652
##########
src/main/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilter.java:
##########
@@ -114,10 +114,6 @@ public boolean merge(final BitMapProducer bitMapProducer) {
// idx[0] will be limit+1 so decrement it
idx[0]--;
final int idxLimit = BitMap.getLongIndex(shape.getNumberOfBits());
- if (idxLimit < idx[0]) {
Review Comment:
I assume you are removing this as it is unreachable code, i.e. any idxLimit
< idx[0] would generate an index out of bounds exception in `bitMap[idx[0]++]`.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/AbstractBitMapProducerTest.java:
##########
@@ -103,6 +103,18 @@ public final void testForEachBitMapPair() {
Arrays.fill(count, 0);
createProducer().forEachBitMapPair(createEmptyProducer(), lbp);
assertEquals(count[2], count[1]);
+
+ // test where both producer does not process all records and function
Review Comment:
IIUC the producer `shortProducer` will process all records (since it always
returns true). Do you mean:
```
// test where the created producer does not process all records because the
predicate function
// returns false before the processing is completed.
```
##########
src/main/java/org/apache/commons/collections4/bloomfilter/BloomFilter.java:
##########
@@ -302,7 +302,7 @@ default int estimateIntersection(final BloomFilter other) {
if (Double.isInfinite(eUnion)) {
throw new IllegalArgumentException("The estimated N for the
union of the filters is infinite");
}
- // all estimated values are small values greater than 0 but less
that number of bits
+ // maximum estimate value using integer valuess is: 46144189292
Review Comment:
I do not think this comment change helps. I think the original comment is
valid, i.e. since all values are positive and in the range of an integer then
this will not overflow a long.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
Review Comment:
`for (int i = 0; i < limit; i++)`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
Review Comment:
`for (int i = 0; i < ary.length - 1; i++)`
Note the change to `ary.length - 1` here.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
+ assertEquals(0xFFFFFFFFFFFFFFFFL, ary[i]);
+ }
+ int lastBits = (Integer.MAX_VALUE-1) % 64;
Review Comment:
`(Integer.MAX_VALUE - 1)`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
+ assertEquals(0xFFFFFFFFFFFFFFFFL, ary[i]);
+ }
+ int lastBits = (Integer.MAX_VALUE-1) % 64;
+ long last = 0L;
+ for (int i=0; i<lastBits; i++) {
Review Comment:
`for (int i = 0; i < lastBits; i++) {`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
Review Comment:
Whitespace `() -> filter`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBitMapProducerTest.java:
##########
@@ -87,4 +87,23 @@ public void testFromBitMapArray() {
final long[] ary =
BitMapProducer.fromBitMapArray(expected).asBitMapArray();
assertArrayEquals(expected, ary);
}
+
+ @Test
+ public void testAsBitMapArrayLargeArray() {
+ final long[] expected = generateLongArray(32);
+ BitMapProducer producer = new BitMapProducer() {
+
Review Comment:
Remove empty line
##########
src/test/java/org/apache/commons/collections4/bloomfilter/AbstractBitMapProducerTest.java:
##########
@@ -103,6 +103,18 @@ public final void testForEachBitMapPair() {
Arrays.fill(count, 0);
createProducer().forEachBitMapPair(createEmptyProducer(), lbp);
assertEquals(count[2], count[1]);
+
+ // test where both producer does not process all records and function
+ // returns false before the processing is completed.
+ int[] limit = new int[1];
+ final LongBiPredicate shortFunc = (x, y) -> {
+ limit[0]++;
+ return limit[0]<2;
+ };
+ final BitMapProducer shortProducer = (l) -> {
Review Comment:
No need for parentheses around `l`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
+ assertEquals(0xFFFFFFFFFFFFFFFFL, ary[i]);
+ }
+ int lastBits = (Integer.MAX_VALUE-1) % 64;
+ long last = 0L;
+ for (int i=0; i<lastBits; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ assertEquals(last, ary[ary.length-1]);
+ }
+
+ @Test
+ public void testEstimateLargeN() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
Review Comment:
`limit` should be Integer.MAX_VALUE. There are a lot of whitespace issues
here. See the comments for the method `testLargeBitmapAsArray`.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
Review Comment:
`limit > 64`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
+ assertEquals(0xFFFFFFFFFFFFFFFFL, ary[i]);
+ }
+ int lastBits = (Integer.MAX_VALUE-1) % 64;
+ long last = 0L;
+ for (int i=0; i<lastBits; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ assertEquals(last, ary[ary.length-1]);
+ }
+
+ @Test
+ public void testEstimateLargeN() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ assertEquals(Integer.MAX_VALUE, bf1.estimateN());
+ }
+
+ @Test
+ public void testIntersectionLimit() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
Review Comment:
`limit` should be Integer.MAX_VALUE. There are a lot of whitespace issues
here. See the comments for the method `testLargeBitmapAsArray`.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
Review Comment:
I do not know why we need this test. It is just checking that the `asArray`
function works for a maximum length filter? If this is the purpose then can we
just use a filter with `bitIndex = Integer.MAX_VALUE` set to 1, e.g. by using a
hasher with 1 index to populate the filter.
Is this code simply to test a fully populated max length filter, e.g. it was
debugging code?
##########
src/test/java/org/apache/commons/collections4/bloomfilter/AbstractBitMapProducerTest.java:
##########
@@ -103,6 +103,18 @@ public final void testForEachBitMapPair() {
Arrays.fill(count, 0);
createProducer().forEachBitMapPair(createEmptyProducer(), lbp);
assertEquals(count[2], count[1]);
+
+ // test where both producer does not process all records and function
+ // returns false before the processing is completed.
+ int[] limit = new int[1];
+ final LongBiPredicate shortFunc = (x, y) -> {
+ limit[0]++;
+ return limit[0]<2;
Review Comment:
whitespace `limit[0] < 2`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
Review Comment:
`Integer.MAX_VALUE - 1`
Q. Should the limit be Integer.MAX_VALUE? The filter can handle this index.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
+ assertEquals(0xFFFFFFFFFFFFFFFFL, ary[i]);
+ }
+ int lastBits = (Integer.MAX_VALUE-1) % 64;
+ long last = 0L;
+ for (int i=0; i<lastBits; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ assertEquals(last, ary[ary.length-1]);
Review Comment:
`ary[ary.length - 1]`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
+ long[] ary = bf1.asBitMapArray();
+ for (int i=0; i<ary.length-2; i++) {
+ assertEquals(0xFFFFFFFFFFFFFFFFL, ary[i]);
+ }
+ int lastBits = (Integer.MAX_VALUE-1) % 64;
+ long last = 0L;
+ for (int i=0; i<lastBits; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ assertEquals(last, ary[ary.length-1]);
+ }
+
+ @Test
+ public void testEstimateLargeN() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
+ // the actual result of the calculation is: 46144189292
Review Comment:
The actual result is not relevant. It could be obtained from
`Shape.estimateN` to verify the result is actually above Integer.MAX_VALUE.
##########
src/test/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilterTest.java:
##########
@@ -24,4 +29,15 @@ public class SimpleBloomFilterTest extends
AbstractBloomFilterTest<SimpleBloomFi
protected SimpleBloomFilter createEmptyFilter(final Shape shape) {
return new SimpleBloomFilter(shape);
}
+
+ @Test
+ public void testMergeShortBitMapProducer() {
+ SimpleBloomFilter filter = createEmptyFilter(getTestShape());
+ // create a producer that returns too few values
+ BitMapProducer producer = (p) -> {
Review Comment:
No parentheses around `p`
##########
src/test/java/org/apache/commons/collections4/bloomfilter/SimpleBloomFilterTest.java:
##########
@@ -24,4 +29,15 @@ public class SimpleBloomFilterTest extends
AbstractBloomFilterTest<SimpleBloomFi
protected SimpleBloomFilter createEmptyFilter(final Shape shape) {
return new SimpleBloomFilter(shape);
}
+
+ @Test
+ public void testMergeShortBitMapProducer() {
+ SimpleBloomFilter filter = createEmptyFilter(getTestShape());
+ // create a producer that returns too few values
Review Comment:
// create a producer that returns no values
##########
src/test/java/org/apache/commons/collections4/bloomfilter/DefaultBloomFilterTest.java:
##########
@@ -67,6 +68,110 @@ public void testHasherBasedMergeWithDifferingSparseness() {
.forEachBitMapPair(bf1, (x, y) -> x == y));
}
+ @Test
+ public void testEstimateNWithBrokenCardinality() {
+ // build a filter
+ BloomFilter filter1 = TestingHashers.populateEntireFilter(new
BrokenCardinality(getTestShape()));
+ assertThrows(IllegalArgumentException.class, ()->filter1.estimateN());
+ }
+
+ @Test
+ public void testLargeBitmapAsArray() {
+ Shape s = Shape.fromKM(1, Integer.MAX_VALUE);
+ // create a very large filter with Integer.MAX_VALUE-1 bit set.
+ BloomFilter bf1 = new SimpleBloomFilter(s);
+ bf1.merge(new BitMapProducer() {
+ @Override
+ public boolean forEachBitMap(LongPredicate predicate) {
+ int limit = Integer.MAX_VALUE-1;
+ while (limit>64) {
+ predicate.test(0xFFFFFFFFFFFFFFFFL);
+ limit -= 64;
+ }
+ long last = 0L;
+ for (int i=0; i<limit; i++) {
+ last |= BitMap.getLongBit(i);
+ }
+ predicate.test(last);
+ return true;
+ }} );
Review Comment:
```
}
});
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
