Skip to content

Commit 6eecb2f

Browse files
authored
Use a prefix scan when fetching documents matching a query. (firebase#488)
Minor optimization (which is already present in the ts code).
1 parent a401798 commit 6eecb2f

File tree

2 files changed

+11
-6
lines changed

2 files changed

+11
-6
lines changed

Firestore/Example/Tests/Local/FSTRemoteDocumentCacheTests.m

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,8 @@ - (void)testRemoveNonExistentDocument {
105105
- (void)testDocumentsMatchingQuery {
106106
if (!self.remoteDocumentCache) return;
107107

108+
// TODO(rsgowman): This just verifies that we do a prefix scan against the
109+
// query path. We'll need more tests once we add index support.
108110
[self setTestDocumentAtPath:@"a/1"];
109111
[self setTestDocumentAtPath:@"b/1"];
110112
[self setTestDocumentAtPath:@"b/2"];
@@ -114,12 +116,10 @@ - (void)testDocumentsMatchingQuery {
114116
FSTDocumentDictionary *results = [self.remoteDocumentCache documentsMatchingQuery:query];
115117
NSArray *expected =
116118
@[ FSTTestDoc(@"b/1", kVersion, _kDocData, NO), FSTTestDoc(@"b/2", kVersion, _kDocData, NO) ];
119+
XCTAssertEqual([results count], [expected count]);
117120
for (FSTDocument *doc in expected) {
118121
XCTAssertEqualObjects([results objectForKey:doc.key], doc);
119122
}
120-
121-
// TODO(mikelehen): Perhaps guard against extra documents in the result set once our
122-
// implementations are smarter.
123123
}
124124

125125
#pragma mark - Helpers

Firestore/Source/Local/FSTLevelDBRemoteDocumentCache.mm

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,15 @@
2121
#include <string>
2222

2323
#import "Firestore/Protos/objc/firestore/local/MaybeDocument.pbobjc.h"
24+
#import "Firestore/Source/Core/FSTQuery.h"
2425
#import "Firestore/Source/Local/FSTLevelDBKey.h"
2526
#import "Firestore/Source/Local/FSTLocalSerializer.h"
2627
#import "Firestore/Source/Local/FSTWriteGroup.h"
2728
#import "Firestore/Source/Model/FSTDocument.h"
2829
#import "Firestore/Source/Model/FSTDocumentDictionary.h"
2930
#import "Firestore/Source/Model/FSTDocumentKey.h"
3031
#import "Firestore/Source/Model/FSTDocumentSet.h"
32+
#import "Firestore/Source/Model/FSTPath.h"
3133
#import "Firestore/Source/Util/FSTAssert.h"
3234

3335
#include "Firestore/Port/ordered_code.h"
@@ -102,18 +104,21 @@ - (nullable FSTMaybeDocument *)entryForKey:(FSTDocumentKey *)documentKey {
102104
}
103105

104106
- (FSTDocumentDictionary *)documentsMatchingQuery:(FSTQuery *)query {
105-
// TODO(mikelehen): PERF: At least filter to the documents that match the path of the query.
106107
FSTDocumentDictionary *results = [FSTDocumentDictionary documentDictionary];
107108

108-
std::string startKey = [FSTLevelDBRemoteDocumentKey keyPrefix];
109+
// Documents are ordered by key, so we can use a prefix scan to narrow down
110+
// the documents we need to match the query against.
111+
std::string startKey = [FSTLevelDBRemoteDocumentKey keyPrefixWithResourcePath:query.path];
109112
std::unique_ptr<Iterator> it(_db->NewIterator(StandardReadOptions()));
110113
it->Seek(startKey);
111114

112115
FSTLevelDBRemoteDocumentKey *currentKey = [[FSTLevelDBRemoteDocumentKey alloc] init];
113116
for (; it->Valid() && [currentKey decodeKey:it->key()]; it->Next()) {
114117
FSTMaybeDocument *maybeDoc =
115118
[self decodedMaybeDocument:it->value() withKey:currentKey.documentKey];
116-
if ([maybeDoc isKindOfClass:[FSTDocument class]]) {
119+
if (![query.path isPrefixOfPath:maybeDoc.key.path]) {
120+
break;
121+
} else if ([maybeDoc isKindOfClass:[FSTDocument class]]) {
117122
results = [results dictionaryBySettingObject:(FSTDocument *)maybeDoc forKey:maybeDoc.key];
118123
}
119124
}

0 commit comments

Comments
 (0)