Skip to content

Commit 1820b54

Browse files
schmidt-sebastianschmidt-sebastian
committed
Index-Free: Add the ability to filter by read time
1 parent 8ea9471 commit 1820b54

File tree

7 files changed

+78
-21
lines changed

7 files changed

+78
-21
lines changed

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexedQueryEngine.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import com.google.firebase.firestore.model.DocumentKey;
3030
import com.google.firebase.firestore.model.FieldPath;
3131
import com.google.firebase.firestore.model.MaybeDocument;
32+
import com.google.firebase.firestore.model.SnapshotVersion;
3233
import com.google.firebase.firestore.model.value.ArrayValue;
3334
import com.google.firebase.firestore.model.value.BooleanValue;
3435
import com.google.firebase.firestore.model.value.DoubleValue;
@@ -99,7 +100,7 @@ public IndexedQueryEngine(
99100
@Override
100101
public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(Query query) {
101102
return query.isDocumentQuery()
102-
? localDocuments.getDocumentsMatchingQuery(query)
103+
? localDocuments.getDocumentsMatchingQuery(query, SnapshotVersion.NONE)
103104
: performCollectionQuery(query);
104105
}
105106

@@ -118,7 +119,7 @@ private ImmutableSortedMap<DocumentKey, Document> performCollectionQuery(Query q
118119
"If there are any filters, we should be able to use an index.");
119120
// TODO: Call overlay.getCollectionDocuments(query.getPath()) and filter the
120121
// results (there may still be startAt/endAt bounds that apply).
121-
filteredResults = localDocuments.getDocumentsMatchingQuery(query);
122+
filteredResults = localDocuments.getDocumentsMatchingQuery(query, SnapshotVersion.NONE);
122123
}
123124

124125
return filteredResults;

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalDocumentsView.java

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,22 @@ ImmutableSortedMap<DocumentKey, MaybeDocument> getLocalViewOfDocuments(
131131
// documents in a given collection so that SimpleQueryEngine can do that and then filter in
132132
// memory.
133133

134-
/** Performs a query against the local view of all documents. */
135-
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(Query query) {
134+
/**
135+
* Performs a query against the local view of all documents.
136+
*
137+
* @param query The query to match documents against.
138+
* @param sinceUpdateTime If not set to SnapshotVersion.MIN, return only documents that have been
139+
* modified since this snapshot version (exclusive).
140+
*/
141+
ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
142+
Query query, SnapshotVersion sinceUpdateTime) {
136143
ResourcePath path = query.getPath();
137144
if (query.isDocumentQuery()) {
138145
return getDocumentsMatchingDocumentQuery(path);
139146
} else if (query.isCollectionGroupQuery()) {
140-
return getDocumentsMatchingCollectionGroupQuery(query);
147+
return getDocumentsMatchingCollectionGroupQuery(query, sinceUpdateTime);
141148
} else {
142-
return getDocumentsMatchingCollectionQuery(query);
149+
return getDocumentsMatchingCollectionQuery(query, sinceUpdateTime);
143150
}
144151
}
145152

@@ -156,7 +163,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingDocumentQu
156163
}
157164

158165
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionGroupQuery(
159-
Query query) {
166+
Query query, SnapshotVersion sinceUpdateTime) {
160167
hardAssert(
161168
query.getPath().isEmpty(),
162169
"Currently we only support collection group queries at the root.");
@@ -169,7 +176,7 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
169176
for (ResourcePath parent : parents) {
170177
Query collectionQuery = query.asCollectionQueryAtPath(parent.append(collectionId));
171178
ImmutableSortedMap<DocumentKey, Document> collectionResults =
172-
getDocumentsMatchingCollectionQuery(collectionQuery);
179+
getDocumentsMatchingCollectionQuery(collectionQuery, sinceUpdateTime);
173180
for (Map.Entry<DocumentKey, Document> docEntry : collectionResults) {
174181
results = results.insert(docEntry.getKey(), docEntry.getValue());
175182
}
@@ -179,9 +186,9 @@ private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollection
179186

180187
/** Queries the remote documents and overlays mutations. */
181188
private ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingCollectionQuery(
182-
Query query) {
189+
Query query, SnapshotVersion sinceUpdateTime) {
183190
ImmutableSortedMap<DocumentKey, Document> results =
184-
remoteDocumentCache.getAllDocumentsMatchingQuery(query);
191+
remoteDocumentCache.getAllDocumentsMatchingQuery(query, sinceUpdateTime);
185192

186193
List<MutationBatch> matchingBatches = mutationQueue.getAllMutationBatchesAffectingQuery(query);
187194

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryRemoteDocumentCache.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ public Map<DocumentKey, MaybeDocument> getAll(Iterable<DocumentKey> keys) {
8282
}
8383

8484
@Override
85-
public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(Query query) {
85+
public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
86+
Query query, SnapshotVersion sinceUpdateTime) {
8687
hardAssert(
8788
!query.isCollectionGroupQuery(),
8889
"CollectionGroup queries should be handled in LocalDocumentsView");
@@ -112,6 +113,11 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(Qu
112113
continue;
113114
}
114115

116+
SnapshotVersion readTime = entry.getValue().second;
117+
if (readTime.compareTo(sinceUpdateTime) <= 0) {
118+
continue;
119+
}
120+
115121
Document doc = (Document) maybeDoc;
116122
if (query.matches(doc)) {
117123
result = result.insert(doc.getKey(), doc);

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/RemoteDocumentCache.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,10 @@ interface RemoteDocumentCache {
7575
* <p>Cached NoDocument entries have no bearing on query results.
7676
*
7777
* @param query The query to match documents against.
78+
* @param sinceUpdateTime If not set to SnapshotVersion.MIN, return only documents that have been
79+
* modified since this snapshot version (exclusive).
7880
* @return The set of matching documents.
7981
*/
80-
ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(Query query);
82+
ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
83+
Query query, SnapshotVersion sinceUpdateTime);
8184
}

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,8 @@ public Map<DocumentKey, MaybeDocument> getAll(Iterable<DocumentKey> documentKeys
131131
}
132132

133133
@Override
134-
public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(Query query) {
134+
public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(
135+
Query query, SnapshotVersion sinceUpdateTime) {
135136
hardAssert(
136137
!query.isCollectionGroupQuery(),
137138
"CollectionGroup queries should be handled in LocalDocumentsView");
@@ -142,6 +143,7 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(Qu
142143

143144
String prefixPath = EncodedPath.encode(prefix);
144145
String prefixSuccessorPath = EncodedPath.prefixSuccessor(prefixPath);
146+
Timestamp updateTime = sinceUpdateTime.getTimestamp();
145147

146148
BackgroundQueue backgroundQueue = new BackgroundQueue();
147149

@@ -150,8 +152,16 @@ public ImmutableSortedMap<DocumentKey, Document> getAllDocumentsMatchingQuery(Qu
150152
new ImmutableSortedMap[] {DocumentCollections.emptyDocumentMap()};
151153

152154
int rowsProcessed =
153-
db.query("SELECT path, contents FROM remote_documents WHERE path >= ? AND path < ?")
154-
.binding(prefixPath, prefixSuccessorPath)
155+
db.query(
156+
"SELECT path, contents FROM remote_documents WHERE path >= ? AND path < ? "
157+
+ "AND (read_time_seconds IS NULL OR read_time_seconds > ? "
158+
+ "OR (read_time_seconds = ? AND read_time_nanos > ?))")
159+
.binding(
160+
prefixPath,
161+
prefixSuccessorPath,
162+
updateTime.getSeconds(),
163+
updateTime.getSeconds(),
164+
updateTime.getNanoseconds())
155165
.forEach(
156166
row -> {
157167
// TODO: Actually implement a single-collection query

‎firebase-firestore/src/main/java/com/google/firebase/firestore/local/SimpleQueryEngine.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.google.firebase.firestore.model.Document;
2020
import com.google.firebase.firestore.model.DocumentKey;
2121
import com.google.firebase.firestore.model.MaybeDocument;
22+
import com.google.firebase.firestore.model.SnapshotVersion;
2223

2324
/**
2425
* A naive implementation of QueryEngine that just loads all the documents in the queried collection
@@ -36,7 +37,7 @@ public SimpleQueryEngine(LocalDocumentsView localDocumentsView) {
3637
public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(Query query) {
3738
// TODO: Once LocalDocumentsView provides a getCollectionDocuments() method, we
3839
// should call that here and then filter the results.
39-
return localDocumentsView.getDocumentsMatchingQuery(query);
40+
return localDocumentsView.getDocumentsMatchingQuery(query, SnapshotVersion.NONE);
4041
}
4142

4243
@Override

‎firebase-firestore/src/test/java/com/google/firebase/firestore/local/RemoteDocumentCacheTestCase.java

Lines changed: 34 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -177,19 +177,48 @@ public void testDocumentsMatchingQuery() {
177177

178178
Query query = Query.atPath(path("b"));
179179
ImmutableSortedMap<DocumentKey, Document> results =
180-
remoteDocumentCache.getAllDocumentsMatchingQuery(query);
180+
remoteDocumentCache.getAllDocumentsMatchingQuery(query, SnapshotVersion.NONE);
181181
List<Document> expected = asList(doc("b/1", 42, docData), doc("b/2", 42, docData));
182182
assertEquals(expected, values(results));
183183
}
184184

185+
@Test
186+
public void testDocumentsMatchingQuerySinceReadTime() {
187+
Map<String, Object> docData = map("data", 2);
188+
addTestDocumentAtPath("b/old", /* updateTime= */ 1, /* readTime= */ 11);
189+
addTestDocumentAtPath("b/current", /* updateTime= */ 2, /* readTime= = */ 12);
190+
addTestDocumentAtPath("b/new", /* updateTime= */ 3, /* readTime= = */ 13);
191+
192+
Query query = Query.atPath(path("b"));
193+
ImmutableSortedMap<DocumentKey, Document> results =
194+
remoteDocumentCache.getAllDocumentsMatchingQuery(query, version(12));
195+
List<Document> expected = asList(doc("b/new", 3, docData));
196+
assertEquals(expected, values(results));
197+
}
198+
199+
@Test
200+
public void testDocumentsMatchingUsesReadTimeNotUpdateTime() {
201+
Map<String, Object> docData = map("data", 2);
202+
addTestDocumentAtPath("b/old", /* updateTime= */ 1, /* readTime= */ 2);
203+
addTestDocumentAtPath("b/new", /* updateTime= */ 2, /* readTime= */ 1);
204+
205+
Query query = Query.atPath(path("b"));
206+
ImmutableSortedMap<DocumentKey, Document> results =
207+
remoteDocumentCache.getAllDocumentsMatchingQuery(query, version(1));
208+
List<Document> expected = asList(doc("b/old", 1, docData));
209+
assertEquals(expected, values(results));
210+
}
211+
185212
private Document addTestDocumentAtPath(String path) {
186-
Document doc = doc(path, 42, map("data", 2));
187-
add(doc, version(42));
213+
return addTestDocumentAtPath(path, 42, 42);
214+
}
215+
216+
private Document addTestDocumentAtPath(String path, int updateTime, int readTime) {
217+
Document doc = doc(path, updateTime, map("data", 2));
218+
add(doc, version(readTime));
188219
return doc;
189220
}
190221

191-
// TODO(mrschmidt): Add a test uses different update and read times and verifies that we correctly
192-
// filter by read time
193222
private void add(MaybeDocument doc, SnapshotVersion readTime) {
194223
persistence.runTransaction("add entry", () -> remoteDocumentCache.add(doc, readTime));
195224
}

0 commit comments

Comments
 (0)