-
Notifications
You must be signed in to change notification settings - Fork 918
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
apply bloomFilter while handling existence filter mismatch #6897
apply bloomFilter while handling existence filter mismatch #6897
Conversation
|
Size Report 1Affected Products
Test Logs |
Size Analysis Report 1This report is too large (991,528 characters) to be displayed here in a GitHub comment. Please use the below link to see the full report on Google Cloud Storage.Test Logs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this! I'm especially impressed with the quality of the spec test cases that you wrote.
packages/firestore/test/unit/specs/existence_filter_spec.test.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh also, please add a spec test case where the bloom filter contains documents with special characters in the document name.
…ter-in-existence-filter-mismatch
packages/firestore/test/unit/specs/existence_filter_spec.test.ts
Outdated
Show resolved
Hide resolved
packages/firestore/test/unit/specs/existence_filter_spec.test.ts
Outdated
Show resolved
Hide resolved
packages/firestore/test/unit/specs/existence_filter_spec.test.ts
Outdated
Show resolved
Hide resolved
packages/firestore/test/unit/specs/existence_filter_spec.test.ts
Outdated
Show resolved
Hide resolved
…ter-in-existence-filter-mismatch
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge it!
-- When bloomFilter yields expected result, skip full re-query
-- When bloomFilter contains false positive result, trigger re-query
newTextEncoder
andnewTextDecoder
out of platform/serializer file into platform/text_serializer fileCircular dependency: BloomFilter uses
newTextEncoder
from /platform/serializer -> /remote/watch_change usesBloomFilter
-> /platform/serializer usesJsonProtoSerializer
from /remote/serializerDo we need to use platform based
newTextEncoder
in checking the membership of a key in BloomFilter? if a key is consist of non-special characters, like 'ÀÒ∑', md5 hash will return different results with and without utf-8 encoding.