Skip to content

Commit fdff9cd

Browse files
schmidt-sebastianschmidt-sebastian
committed
Re-work executeQuery()
1 parent 2f1ce77 commit fdff9cd

File tree

10 files changed

+145
-165
lines changed

10 files changed

+145
-165
lines changed

‎packages/firestore/src/core/firestore_client.ts

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,6 @@ import { LocalStore } from '../local/local_store';
2222
import { MemoryPersistence } from '../local/memory_persistence';
2323
import { Persistence } from '../local/persistence';
2424
import { SimpleQueryEngine } from '../local/simple_query_engine';
25-
import {
26-
DocumentKeySet,
27-
documentKeySet,
28-
DocumentMap
29-
} from '../model/collections';
3025
import { Document, MaybeDocument, NoDocument } from '../model/document';
3126
import { DocumentKey } from '../model/document_key';
3227
import { Mutation } from '../model/mutation';
@@ -45,7 +40,7 @@ import {
4540
QueryListener
4641
} from './event_manager';
4742
import { SyncEngine } from './sync_engine';
48-
import { View, ViewDocumentChanges } from './view';
43+
import { View } from './view';
4944

5045
import {
5146
LruGarbageCollector,
@@ -584,22 +579,18 @@ export class FirestoreClient {
584579

585580
getDocumentsFromLocalCache(query: Query): Promise<ViewSnapshot> {
586581
this.verifyNotTerminated();
587-
return this.asyncQueue
588-
.enqueue(() => {
589-
return this.localStore.executeQuery(query);
590-
})
591-
.then((docs: DocumentMap) => {
592-
const remoteKeys: DocumentKeySet = documentKeySet();
593-
594-
const view = new View(query, remoteKeys);
595-
const viewDocChanges: ViewDocumentChanges = view.computeDocChanges(
596-
docs
597-
);
598-
return view.applyChanges(
599-
viewDocChanges,
600-
/* updateLimboDocuments= */ false
601-
).snapshot!;
602-
});
582+
return this.asyncQueue.enqueue(async () => {
583+
const queryResult = await this.localStore.executeQuery(
584+
query,
585+
/* usePreviousResults= */ true
586+
);
587+
const view = new View(query, queryResult.remoteKeys);
588+
const viewDocChanges = view.computeDocChanges(queryResult.documents);
589+
return view.applyChanges(
590+
viewDocChanges,
591+
/* updateLimboDocuments= */ false
592+
).snapshot!;
593+
});
603594
}
604595

605596
write(mutations: Mutation[]): Promise<void> {

‎packages/firestore/src/core/sync_engine.ts

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -243,16 +243,12 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
243243
current: boolean
244244
): Promise<ViewSnapshot> {
245245
const query = queryData.query;
246-
const remoteKeys = await this.localStore.remoteDocumentKeys(
247-
queryData.targetId
246+
const queryResult = await this.localStore.executeQuery(
247+
query,
248+
/* usePreviousResults= */ true
248249
);
249-
const docs = await this.localStore.executeQuery(query, {
250-
queryData,
251-
remoteKeys
252-
});
253-
254-
const view = new View(query, remoteKeys);
255-
const viewDocChanges = view.computeDocChanges(docs);
250+
const view = new View(query, queryResult.remoteKeys);
251+
const viewDocChanges = view.computeDocChanges(queryResult.documents);
256252
const synthesizedTargetChange = TargetChange.createSynthesizedTargetChangeForCurrentChange(
257253
queryData.targetId,
258254
current && this.onlineState !== OnlineState.Offline
@@ -286,14 +282,11 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
286282
queryData: QueryData,
287283
view: View
288284
): Promise<ViewChange> {
289-
const remoteKeys = await this.localStore.remoteDocumentKeys(
290-
queryData.targetId
285+
const queryResult = await this.localStore.executeQuery(
286+
queryData.query,
287+
/* usePreviousResults= */ true
291288
);
292-
const docs = await this.localStore.executeQuery(queryData.query, {
293-
queryData,
294-
remoteKeys
295-
});
296-
const viewSnapshot = view.synchronizeWithPersistedState(docs, remoteKeys);
289+
const viewSnapshot = view.synchronizeWithPersistedState(queryResult);
297290
if (this.isPrimary) {
298291
this.updateTrackedLimbos(queryData.targetId, viewSnapshot.limboChanges);
299292
}
@@ -791,9 +784,12 @@ export class SyncEngine implements RemoteSyncer, SharedClientStateSyncer {
791784
// to re-run the query against the local store to make sure we
792785
// didn't lose any good docs that had been past the limit.
793786
return this.localStore
794-
.executeQuery(queryView.query, { needsRefill: true })
795-
.then(docs => {
796-
return queryView.view.computeDocChanges(docs, viewDocChanges);
787+
.executeQuery(queryView.query, /* usePreviousResults= */ false)
788+
.then(({ documents }) => {
789+
return queryView.view.computeDocChanges(
790+
documents,
791+
viewDocChanges
792+
);
797793
});
798794
})
799795
.then((viewDocChanges: ViewDocumentChanges) => {

‎packages/firestore/src/core/view.ts

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { QueryResult } from '../local/local_store';
1819
import {
1920
documentKeySet,
2021
DocumentKeySet,
@@ -433,21 +434,18 @@ export class View {
433434
* of `syncedDocuments` since secondary clients update their query views
434435
* based purely on synthesized RemoteEvents.
435436
*
436-
* @param localDocs - The documents that match the query according to the
437-
* LocalStore.
438-
* @param remoteKeys - The keys of the documents that match the query
439-
* according to the backend.
437+
* @param queryResult.documents - The documents that match the query according
438+
* to the LocalStore.
439+
* @param queryResult.remoteKeys - The keys of the documents that match the
440+
* query according to the backend.
440441
*
441442
* @return The ViewChange that resulted from this synchronization.
442443
*/
443444
// PORTING NOTE: Multi-tab only.
444-
synchronizeWithPersistedState(
445-
localDocs: MaybeDocumentMap,
446-
remoteKeys: DocumentKeySet
447-
): ViewChange {
448-
this._syncedDocuments = remoteKeys;
445+
synchronizeWithPersistedState(queryResult: QueryResult): ViewChange {
446+
this._syncedDocuments = queryResult.remoteKeys;
449447
this.limboDocuments = documentKeySet();
450-
const docChanges = this.computeDocChanges(localDocs);
448+
const docChanges = this.computeDocChanges(queryResult.documents);
451449
return this.applyChanges(docChanges, /*updateLimboDocuments=*/ true);
452450
}
453451

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ import { QueryEngine } from './query_engine';
1919
import { LocalDocumentsView } from './local_documents_view';
2020
import { PersistenceTransaction } from './persistence';
2121
import { PersistencePromise } from './persistence_promise';
22-
import { QueryData } from './query_data';
2322
import { Query } from '../core/query';
2423
import { SnapshotVersion } from '../core/snapshot_version';
2524
import {
@@ -62,7 +61,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
6261
getDocumentsMatchingQuery(
6362
transaction: PersistenceTransaction,
6463
query: Query,
65-
queryData: QueryData | null,
64+
lastLimboFreeSnapshotVersion: SnapshotVersion,
6665
remoteKeys: DocumentKeySet
6766
): PersistencePromise<DocumentMap> {
6867
assert(
@@ -79,10 +78,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
7978

8079
// Queries that have never seen a snapshot without limbo free documents
8180
// should also be run as a full collection scan.
82-
if (
83-
queryData === null ||
84-
queryData.lastLimboFreeSnapshotVersion.isEqual(SnapshotVersion.MIN)
85-
) {
81+
if (lastLimboFreeSnapshotVersion.isEqual(SnapshotVersion.MIN)) {
8682
return this.executeFullCollectionScan(transaction, query);
8783
}
8884

@@ -95,7 +91,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
9591
this.needsRefill(
9692
previousResults,
9793
remoteKeys,
98-
queryData.lastLimboFreeSnapshotVersion
94+
lastLimboFreeSnapshotVersion
9995
)
10096
) {
10197
return this.executeFullCollectionScan(transaction, query);
@@ -105,7 +101,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
105101
debug(
106102
'IndexFreeQueryEngine',
107103
'Re-using previous result from %s to execute query: %s',
108-
queryData.lastLimboFreeSnapshotVersion.toString(),
104+
lastLimboFreeSnapshotVersion.toString(),
109105
query.toString()
110106
);
111107
}
@@ -115,7 +111,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
115111
return this.localDocumentsView!.getDocumentsMatchingQuery(
116112
transaction,
117113
query,
118-
queryData.lastLimboFreeSnapshotVersion
114+
lastLimboFreeSnapshotVersion
119115
).next(updatedResults => {
120116
// We merge `previousResults` into `updateResults`, since
121117
// `updateResults` is already a DocumentMap. If a document is

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

Lines changed: 40 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ export interface UserChangeResult {
6969
readonly addedBatchIds: BatchId[];
7070
}
7171

72+
/** The result of executing a query against the local store. */
73+
export interface QueryResult {
74+
readonly documents: DocumentMap;
75+
readonly remoteKeys: DocumentKeySet;
76+
}
77+
7278
/**
7379
* Local storage in the Firestore client. Coordinates persistence components
7480
* like the mutation queue and remote document cache to present a
@@ -782,13 +788,22 @@ export class LocalStore {
782788
transaction: PersistenceTransaction,
783789
query: Query
784790
): PersistencePromise<QueryData | null> {
785-
return this.queryCache.getQueryData(transaction, query).next(queryData => {
786-
if (queryData == null) {
787-
return null;
791+
let queryData: QueryData | null = null;
792+
793+
// Look up the query data in our local map first, as this avoid a potential
794+
// lookup in IndexedDb.
795+
objUtils.forEachNumber(
796+
this.queryDataByTarget,
797+
(targetId, cachedQueryData) => {
798+
if (cachedQueryData.query.isEqual(query)) {
799+
queryData = cachedQueryData;
800+
}
788801
}
789-
const cachedQueryData = this.queryDataByTarget[queryData!.targetId];
790-
return cachedQueryData || queryData;
791-
});
802+
);
803+
804+
return queryData
805+
? PersistencePromise.resolve<QueryData | null>(queryData)
806+
: this.queryCache.getQueryData(transaction, query);
792807
}
793808

794809
/**
@@ -833,49 +848,23 @@ export class LocalStore {
833848
* Runs the specified query against the local store and returns the results,
834849
* potentially taking advantage of query data from previous executions (such
835850
* as the set of remote keys).
851+
*
852+
* @param usePreviousResults Whether results from previous executions can
853+
* be used to optimize this query execution.
836854
*/
837-
executeQuery(query: Query): Promise<DocumentMap>;
838-
/**
839-
* Performs a full collection scan for the provided query and returns the
840-
* results. Does not take into account metadata from prior executions.
841-
*/
842-
executeQuery(
843-
query: Query,
844-
options: { needsRefill: true }
845-
): Promise<DocumentMap>;
846-
/**
847-
* Runs the specified query against the local store and returns the results,
848-
* potentially taking advantage of the provided query data and the set of
849-
* remote document keys.
850-
*/
851-
executeQuery(
852-
query: Query,
853-
options: { queryData: QueryData; remoteKeys: DocumentKeySet }
854-
): Promise<DocumentMap>;
855855
executeQuery(
856856
query: Query,
857-
options: {
858-
needsRefill?: boolean;
859-
queryData?: QueryData;
860-
remoteKeys?: DocumentKeySet;
861-
} = {}
862-
): Promise<DocumentMap> {
863-
const allowLookup = options.needsRefill !== false;
864-
865-
let queryData = options.queryData;
866-
let remoteKeys = options.remoteKeys;
857+
usePreviousResults: boolean
858+
): Promise<QueryResult> {
859+
let lastLimboFreeSnapshotVersion = SnapshotVersion.MIN;
860+
let remoteKeys = documentKeySet();
867861

868862
return this.persistence.runTransaction('Execute query', 'readonly', txn => {
869-
return PersistencePromise.resolve()
870-
.next(() => {
871-
if (allowLookup && !queryData) {
872-
return this.getQueryData(txn, query).next(result => {
873-
queryData = result || undefined;
874-
});
875-
}
876-
})
877-
.next(() => {
878-
if (allowLookup && queryData && !remoteKeys) {
863+
return this.getQueryData(txn, query)
864+
.next(queryData => {
865+
if (queryData) {
866+
lastLimboFreeSnapshotVersion =
867+
queryData.lastLimboFreeSnapshotVersion;
879868
return this.queryCache
880869
.getMatchingKeysForTargetId(txn, queryData.targetId)
881870
.next(result => {
@@ -887,9 +876,14 @@ export class LocalStore {
887876
return this.queryEngine.getDocumentsMatchingQuery(
888877
txn,
889878
query,
890-
queryData || null,
891-
remoteKeys || documentKeySet()
879+
usePreviousResults
880+
? lastLimboFreeSnapshotVersion
881+
: SnapshotVersion.MIN,
882+
usePreviousResults ? remoteKeys : documentKeySet()
892883
);
884+
})
885+
.next(documents => {
886+
return { documents, remoteKeys };
893887
});
894888
});
895889
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { SnapshotVersion } from '../core/snapshot_version';
1819
import { Query } from '../core/query';
1920
import { DocumentKeySet, DocumentMap } from '../model/collections';
2021
import { LocalDocumentsView } from './local_documents_view';
2122
import { PersistenceTransaction } from './persistence';
2223
import { PersistencePromise } from './persistence_promise';
23-
import { QueryData } from './query_data';
2424

2525
/**
2626
* Represents a query engine capable of performing queries over the local
@@ -34,7 +34,7 @@ export interface QueryEngine {
3434
getDocumentsMatchingQuery(
3535
transaction: PersistenceTransaction,
3636
query: Query,
37-
queryData: QueryData | null,
37+
lastLimboFreeSnapshotVersion: SnapshotVersion,
3838
remoteKeys: DocumentKeySet
3939
): PersistencePromise<DocumentMap>;
4040
}

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import { LocalDocumentsView } from './local_documents_view';
2323
import { QueryEngine } from './query_engine';
2424
import { PersistenceTransaction } from './persistence';
2525
import { PersistencePromise } from './persistence_promise';
26-
import { QueryData } from './query_data';
2726

2827
/**
2928
* A naive implementation of QueryEngine that just loads all the documents in
@@ -40,7 +39,7 @@ export class SimpleQueryEngine implements QueryEngine {
4039
getDocumentsMatchingQuery(
4140
transaction: PersistenceTransaction,
4241
query: Query,
43-
queryData: QueryData | null,
42+
lastLimboFreeSnapshotVersion: SnapshotVersion,
4443
remoteKeys: DocumentKeySet
4544
): PersistencePromise<DocumentMap> {
4645
assert(

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,14 @@
1515
* limitations under the License.
1616
*/
1717

18+
import { SnapshotVersion } from '../../../src/core/snapshot_version';
1819
import { QueryEngine } from '../../../src/local/query_engine';
1920
import { LocalDocumentsView } from '../../../src/local/local_documents_view';
2021
import { PersistenceTransaction } from '../../../src/local/persistence';
2122
import { Query } from '../../../src/core/query';
2223
import { PersistencePromise } from '../../../src/local/persistence_promise';
2324
import { RemoteDocumentCache } from '../../../src/local/remote_document_cache';
2425
import { MutationQueue } from '../../../src/local/mutation_queue';
25-
import { QueryData } from '../../../src/local/query_data';
2626
import { DocumentKeySet, DocumentMap } from '../../../src/model/collections';
2727

2828
/**
@@ -52,13 +52,13 @@ export class CountingQueryEngine implements QueryEngine {
5252
getDocumentsMatchingQuery(
5353
transaction: PersistenceTransaction,
5454
query: Query,
55-
queryData: QueryData | null,
55+
lastLimboFreeSnapshotVersion: SnapshotVersion,
5656
remoteKeys: DocumentKeySet
5757
): PersistencePromise<DocumentMap> {
5858
return this.queryEngine.getDocumentsMatchingQuery(
5959
transaction,
6060
query,
61-
queryData,
61+
lastLimboFreeSnapshotVersion,
6262
remoteKeys
6363
);
6464
}

0 commit comments

Comments
 (0)