Skip to content

Commit 2fef555

Browse files
schmidt-sebastianschmidt-sebastian
authored
Index-Free: Filter document queries by read time (#2159)
1 parent d32cbbe commit 2fef555

8 files changed

+134
-26
lines changed

‎packages/firestore/src/local/indexeddb_remote_document_cache.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ import { PersistenceTransaction } from './persistence';
4646
import { PersistencePromise } from './persistence_promise';
4747
import { RemoteDocumentCache } from './remote_document_cache';
4848
import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer';
49-
import { SimpleDb, SimpleDbStore, SimpleDbTransaction } from './simple_db';
49+
import {
50+
IterateOptions,
51+
SimpleDb,
52+
SimpleDbStore,
53+
SimpleDbTransaction
54+
} from './simple_db';
5055
import { ObjectMap } from '../util/obj_map';
5156

5257
export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
@@ -257,7 +262,8 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
257262

258263
getDocumentsMatchingQuery(
259264
transaction: PersistenceTransaction,
260-
query: Query
265+
query: Query,
266+
sinceReadTime: SnapshotVersion
261267
): PersistencePromise<DocumentMap> {
262268
assert(
263269
!query.isCollectionGroupQuery(),
@@ -267,12 +273,27 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
267273

268274
const immediateChildrenPathLength = query.path.length + 1;
269275

270-
// Documents are ordered by key, so we can use a prefix scan to narrow down
271-
// the documents we need to match the query against.
272-
const startKey = query.path.toArray();
273-
const range = IDBKeyRange.lowerBound(startKey);
276+
const iterationOptions: IterateOptions = {};
277+
if (sinceReadTime.isEqual(SnapshotVersion.MIN)) {
278+
// Documents are ordered by key, so we can use a prefix scan to narrow
279+
// down the documents we need to match the query against.
280+
const startKey = query.path.toArray();
281+
iterationOptions.range = IDBKeyRange.lowerBound(startKey);
282+
} else {
283+
// Execute an index-free query and filter by read time. This is safe
284+
// since all document changes to queries that have a
285+
// lastLimboFreeSnapshotVersion (`sinceReadTime`) have a read time set.
286+
const collectionKey = query.path.toArray();
287+
const readTimeKey = this.serializer.toDbTimestampKey(sinceReadTime);
288+
iterationOptions.range = IDBKeyRange.lowerBound(
289+
[collectionKey, readTimeKey],
290+
/* open= */ true
291+
);
292+
iterationOptions.index = DbRemoteDocument.collectionReadTimeIndex;
293+
}
294+
274295
return remoteDocumentsStore(transaction)
275-
.iterate({ range }, (key, dbRemoteDoc, control) => {
296+
.iterate(iterationOptions, (key, dbRemoteDoc, control) => {
276297
// The query is actually returning any path that starts with the query
277298
// path prefix which may include documents in subcollections. For
278299
// example, a query on 'rooms' will return rooms/abc/messages/xyx but we
@@ -440,6 +461,10 @@ export class IndexedDbRemoteDocumentCache implements RemoteDocumentCache {
440461
`Cannot modify a document that wasn't read (for ${key})`
441462
);
442463
if (maybeDocument) {
464+
assert(
465+
!this.readTime.isEqual(SnapshotVersion.MIN),
466+
'Cannot add a document with a read time of zero'
467+
);
443468
const doc = this.documentCache.serializer.toDbRemoteDocument(
444469
maybeDocument,
445470
this.readTime

‎packages/firestore/src/local/local_documents_view.ts

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,17 +143,33 @@ export class LocalDocumentsView {
143143
});
144144
}
145145

146-
/** Performs a query against the local view of all documents. */
146+
/**
147+
* Performs a query against the local view of all documents.
148+
*
149+
* @param transaction The persistence transaction.
150+
* @param query The query to match documents against.
151+
* @param sinceReadTime If not set to SnapshotVersion.MIN, return only
152+
* documents that have been read since this snapshot version (exclusive).
153+
*/
147154
getDocumentsMatchingQuery(
148155
transaction: PersistenceTransaction,
149-
query: Query
156+
query: Query,
157+
sinceReadTime: SnapshotVersion
150158
): PersistencePromise<DocumentMap> {
151159
if (query.isDocumentQuery()) {
152160
return this.getDocumentsMatchingDocumentQuery(transaction, query.path);
153161
} else if (query.isCollectionGroupQuery()) {
154-
return this.getDocumentsMatchingCollectionGroupQuery(transaction, query);
162+
return this.getDocumentsMatchingCollectionGroupQuery(
163+
transaction,
164+
query,
165+
sinceReadTime
166+
);
155167
} else {
156-
return this.getDocumentsMatchingCollectionQuery(transaction, query);
168+
return this.getDocumentsMatchingCollectionQuery(
169+
transaction,
170+
query,
171+
sinceReadTime
172+
);
157173
}
158174
}
159175

@@ -175,7 +191,8 @@ export class LocalDocumentsView {
175191

176192
private getDocumentsMatchingCollectionGroupQuery(
177193
transaction: PersistenceTransaction,
178-
query: Query
194+
query: Query,
195+
sinceReadTime: SnapshotVersion
179196
): PersistencePromise<DocumentMap> {
180197
assert(
181198
query.path.isEmpty(),
@@ -194,7 +211,8 @@ export class LocalDocumentsView {
194211
);
195212
return this.getDocumentsMatchingCollectionQuery(
196213
transaction,
197-
collectionQuery
214+
collectionQuery,
215+
sinceReadTime
198216
).next(r => {
199217
r.forEach((key, doc) => {
200218
results = results.insert(key, doc);
@@ -206,13 +224,14 @@ export class LocalDocumentsView {
206224

207225
private getDocumentsMatchingCollectionQuery(
208226
transaction: PersistenceTransaction,
209-
query: Query
227+
query: Query,
228+
sinceReadTime: SnapshotVersion
210229
): PersistencePromise<DocumentMap> {
211230
// Query the remote documents and overlay mutations.
212231
let results: DocumentMap;
213232
let mutationBatches: MutationBatch[];
214233
return this.remoteDocumentCache
215-
.getDocumentsMatchingQuery(transaction, query)
234+
.getDocumentsMatchingQuery(transaction, query, sinceReadTime)
216235
.next(queryResults => {
217236
results = queryResults;
218237
return this.mutationQueue.getAllMutationBatchesAffectingQuery(

‎packages/firestore/src/local/local_store.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,11 @@ export class LocalStore {
819819
*/
820820
executeQuery(query: Query): Promise<DocumentMap> {
821821
return this.persistence.runTransaction('Execute query', 'readonly', txn => {
822-
return this.localDocuments.getDocumentsMatchingQuery(txn, query);
822+
return this.localDocuments.getDocumentsMatchingQuery(
823+
txn,
824+
query,
825+
SnapshotVersion.MIN
826+
);
823827
});
824828
}
825829

‎packages/firestore/src/local/memory_remote_document_cache.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache {
8383
doc: MaybeDocument,
8484
readTime: SnapshotVersion
8585
): PersistencePromise<void> {
86+
assert(
87+
!readTime.isEqual(SnapshotVersion.MIN),
88+
'Cannot add a document with a read time of zero'
89+
);
90+
8691
const key = doc.key;
8792
const entry = this.docs.get(key);
8893
const previousSize = entry ? entry.size : 0;
@@ -140,7 +145,8 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache {
140145

141146
getDocumentsMatchingQuery(
142147
transaction: PersistenceTransaction,
143-
query: Query
148+
query: Query,
149+
sinceReadTime: SnapshotVersion
144150
): PersistencePromise<DocumentMap> {
145151
assert(
146152
!query.isCollectionGroupQuery(),
@@ -155,11 +161,14 @@ export class MemoryRemoteDocumentCache implements RemoteDocumentCache {
155161
while (iterator.hasNext()) {
156162
const {
157163
key,
158-
value: { maybeDocument }
164+
value: { maybeDocument, readTime }
159165
} = iterator.getNext();
160166
if (!query.path.isPrefixOf(key.path)) {
161167
break;
162168
}
169+
if (readTime.compareTo(sinceReadTime) <= 0) {
170+
continue;
171+
}
163172
if (maybeDocument instanceof Document && query.matches(maybeDocument)) {
164173
results = results.insert(maybeDocument.key, maybeDocument);
165174
}

‎packages/firestore/src/local/remote_document_cache.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import { DocumentKey } from '../model/document_key';
2828
import { PersistenceTransaction } from './persistence';
2929
import { PersistencePromise } from './persistence_promise';
3030
import { RemoteDocumentChangeBuffer } from './remote_document_change_buffer';
31+
import { SnapshotVersion } from '../core/snapshot_version';
3132

3233
/**
3334
* Represents cached documents received from the remote backend.
@@ -71,11 +72,14 @@ export interface RemoteDocumentCache {
7172
* Cached NoDocument entries have no bearing on query results.
7273
*
7374
* @param query The query to match documents against.
75+
* @param sinceReadTime If not set to SnapshotVersion.MIN, return only
76+
* documents that have been read since this snapshot version (exclusive).
7477
* @return The set of matching documents.
7578
*/
7679
getDocumentsMatchingQuery(
7780
transaction: PersistenceTransaction,
78-
query: Query
81+
query: Query,
82+
sinceReadTime: SnapshotVersion
7983
): PersistencePromise<DocumentMap>;
8084

8185
/**

‎packages/firestore/test/unit/local/local_store.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -759,15 +759,15 @@ function genericLocalStoreTests(
759759
expectLocalStore()
760760
.afterAllocatingQuery(query)
761761
.toReturnTargetId(2)
762-
.after(docAddedRemoteEvent(doc('foo/bar', 0, { foo: 'old' }), [2]))
762+
.after(docAddedRemoteEvent(doc('foo/bar', 1, { foo: 'old' }), [2]))
763763
.after(patchMutation('foo/bar', { foo: 'bar' }))
764764
// Release the query so that our target count goes back to 0 and we are considered
765765
// up-to-date.
766766
.afterReleasingQuery(query)
767767
.after(setMutation('foo/bah', { foo: 'bah' }))
768768
.after(deleteMutation('foo/baz'))
769769
.toContain(
770-
doc('foo/bar', 0, { foo: 'bar' }, { hasLocalMutations: true })
770+
doc('foo/bar', 1, { foo: 'bar' }, { hasLocalMutations: true })
771771
)
772772
.toContain(
773773
doc('foo/bah', 0, { foo: 'bah' }, { hasLocalMutations: true })
@@ -800,15 +800,15 @@ function genericLocalStoreTests(
800800
expectLocalStore()
801801
.afterAllocatingQuery(query)
802802
.toReturnTargetId(2)
803-
.after(docAddedRemoteEvent(doc('foo/bar', 0, { foo: 'old' }), [2]))
803+
.after(docAddedRemoteEvent(doc('foo/bar', 1, { foo: 'old' }), [2]))
804804
.after(patchMutation('foo/bar', { foo: 'bar' }))
805805
// Release the query so that our target count goes back to 0 and we are considered
806806
// up-to-date.
807807
.afterReleasingQuery(query)
808808
.after(setMutation('foo/bah', { foo: 'bah' }))
809809
.after(deleteMutation('foo/baz'))
810810
.toContain(
811-
doc('foo/bar', 0, { foo: 'bar' }, { hasLocalMutations: true })
811+
doc('foo/bar', 1, { foo: 'bar' }, { hasLocalMutations: true })
812812
)
813813
.toContain(
814814
doc('foo/bah', 0, { foo: 'bah' }, { hasLocalMutations: true })

‎packages/firestore/test/unit/local/remote_document_cache.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import { expect } from 'chai';
1919
import { Query } from '../../../src/core/query';
20+
import { SnapshotVersion } from '../../../src/core/snapshot_version';
2021
import { IndexedDbPersistence } from '../../../src/local/indexeddb_persistence';
2122
import { MaybeDocument } from '../../../src/model/document';
2223
import {
@@ -312,14 +313,57 @@ function genericRemoteDocumentCacheTests(
312313
);
313314

314315
const query = new Query(path('b'));
315-
const matchingDocs = await cache.getDocumentsMatchingQuery(query);
316+
const matchingDocs = await cache.getDocumentsMatchingQuery(
317+
query,
318+
SnapshotVersion.MIN
319+
);
316320

317321
assertMatches(
318322
[doc('b/1', VERSION, DOC_DATA), doc('b/2', VERSION, DOC_DATA)],
319323
matchingDocs
320324
);
321325
});
322326

327+
it('can get documents matching query by read time', async () => {
328+
await cache.addEntries(
329+
[doc('b/old', 1, DOC_DATA)],
330+
/* readTime= */ version(11)
331+
);
332+
await cache.addEntries(
333+
[doc('b/current', 2, DOC_DATA)],
334+
/* readTime= */ version(12)
335+
);
336+
await cache.addEntries(
337+
[doc('b/new', 3, DOC_DATA)],
338+
/* readTime= */ version(13)
339+
);
340+
341+
const query = new Query(path('b'));
342+
const matchingDocs = await cache.getDocumentsMatchingQuery(
343+
query,
344+
/* sinceReadTime= */ version(12)
345+
);
346+
assertMatches([doc('b/new', 3, DOC_DATA)], matchingDocs);
347+
});
348+
349+
it('query matching uses read time rather than update time', async () => {
350+
await cache.addEntries(
351+
[doc('b/old', 1, DOC_DATA)],
352+
/* readTime= */ version(2)
353+
);
354+
await cache.addEntries(
355+
[doc('b/new', 2, DOC_DATA)],
356+
/* readTime= */ version(1)
357+
);
358+
359+
const query = new Query(path('b'));
360+
const matchingDocs = await cache.getDocumentsMatchingQuery(
361+
query,
362+
/* sinceReadTime= */ version(1)
363+
);
364+
assertMatches([doc('b/old', 1, DOC_DATA)], matchingDocs);
365+
});
366+
323367
it('can get changes', async () => {
324368
await cache.addEntries(
325369
[

‎packages/firestore/test/unit/local/test_remote_document_cache.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,15 @@ export class TestRemoteDocumentCache {
107107
});
108108
}
109109

110-
getDocumentsMatchingQuery(query: Query): Promise<DocumentMap> {
110+
getDocumentsMatchingQuery(
111+
query: Query,
112+
sinceReadTime: SnapshotVersion
113+
): Promise<DocumentMap> {
111114
return this.persistence.runTransaction(
112115
'getDocumentsMatchingQuery',
113116
'readonly',
114117
txn => {
115-
return this.cache.getDocumentsMatchingQuery(txn, query);
118+
return this.cache.getDocumentsMatchingQuery(txn, query, sinceReadTime);
116119
}
117120
);
118121
}

0 commit comments

Comments
 (0)