Skip to content

Commit 160b742

Browse files
committed
resolve comments
1 parent 37d15ab commit 160b742

File tree

6 files changed

+10
-12
lines changed

6 files changed

+10
-12
lines changed

packages/firestore/src/local/local_store_impl.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -601,7 +601,7 @@ export function localStoreApplyRemoteEventToLocalCache(
601601
let newTargetData = oldTargetData.withSequenceNumber(
602602
txn.currentSequenceNumber
603603
);
604-
if (remoteEvent.targetMismatches.get(targetId) != null) {
604+
if (remoteEvent.targetMismatches.get(targetId) !== null) {
605605
newTargetData = newTargetData
606606
.withResumeToken(
607607
ByteString.EMPTY_BYTE_STRING,

packages/firestore/src/local/target_data.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ export const enum TargetPurpose {
2626
Listen,
2727

2828
/**
29-
* The query target was used to refill a query after an existence filter mismatch.
29+
* The query target was used to refill a query after an existence filter
30+
* mismatch.
3031
*/
3132
ExistenceFilterMismatch,
3233

3334
/**
34-
* The query target was used if the query is the result of a false positive in the bloom filter.
35+
* The query target was used if the query is the result of a false positive in
36+
* the bloom filter.
3537
*/
3638
ExistenceFilterMismatchBloom,
3739

packages/firestore/src/remote/bloom_filter.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ export class BloomFilter {
7575
this.bitCountInInteger = Integer.fromNumber(this.bitCount);
7676
}
7777

78-
isEmpty(): boolean {
79-
return this.bitCount === 0;
80-
}
81-
8278
// Calculate the ith hash value based on the hashed 64bit integers,
8379
// and calculate its corresponding bit index in the bitmap to be checked.
8480
private getBitIndex(num1: Integer, num2: Integer, hashIndex: number): number {

packages/firestore/src/remote/remote_store.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -611,10 +611,7 @@ function raiseWatchSnapshot(
611611
// Mark the target we send as being on behalf of an existence filter
612612
// mismatch, but don't actually retain that in listenTargets. This ensures
613613
// that we flag the first re-listen this way without impacting future
614-
// listens of this target (that might happen e.g. on reconnect). The target
615-
// purpose will be `ExistenceFilterMismatchBloom` if there is a bloom filter
616-
// but it yield false positive result, otherwise, it will be set to
617-
// `ExistenceFilterMismatch`.
614+
// listens of this target (that might happen e.g. on reconnect).
618615
const requestTargetData = new TargetData(
619616
targetData.target,
620617
targetId,

packages/firestore/src/remote/watch_change.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,7 @@ export class WatchChangeAggregator {
499499
return BloomFilterApplicationStatus.Skipped;
500500
}
501501

502-
if (bloomFilter.isEmpty()) {
502+
if (bloomFilter.bitCount === 0) {
503503
return BloomFilterApplicationStatus.Skipped;
504504
}
505505

packages/firestore/test/unit/remote/remote_event.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,9 @@ describe('RemoteEvent', () => {
431431
expectTargetChangeEquals(event.targetChanges.get(1)!, expected);
432432
});
433433

434+
// TODO(Mila): Add test cases for existence filter with bloom filter, one will
435+
// skip the re-query, one will yield false positive result and clears target
436+
// mapping. b/272564458
434437
it('existence filters clears target mapping', () => {
435438
const targets = listens(1, 2);
436439

0 commit comments

Comments
 (0)