diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java index cf8ef713903..c1270ef0d91 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteRemoteDocumentCache.java @@ -155,48 +155,59 @@ public ImmutableSortedMap getAllDocumentsMatchingQuery( (ImmutableSortedMap[]) new ImmutableSortedMap[] {DocumentCollections.emptyDocumentMap()}; + SQLitePersistence.Query sqlQuery; + if (sinceReadTime.equals(SnapshotVersion.NONE)) { + sqlQuery = + db.query("SELECT path, contents FROM remote_documents WHERE path >= ? AND path < ?") + .binding(prefixPath, prefixSuccessorPath); + } else { + // Execute an index-free query and filter by read time. This is safe since all document + // changes to queries that have a lastLimboFreeSnapshotVersion (`sinceReadTime`) have a read + // time set. + sqlQuery = + db.query( + "SELECT path, contents FROM remote_documents WHERE path >= ? AND path < ?" + + "AND (read_time_seconds > ? OR (read_time_seconds = ? AND read_time_nanos > ?))") + .binding( + prefixPath, + prefixSuccessorPath, + readTime.getSeconds(), + readTime.getSeconds(), + readTime.getNanoseconds()); + } + int rowsProcessed = - db.query( - "SELECT path, contents FROM remote_documents WHERE path >= ? AND path < ? " - + "AND (read_time_seconds IS NULL OR read_time_seconds > ? " - + "OR (read_time_seconds = ? AND read_time_nanos > ?))") - .binding( - prefixPath, - prefixSuccessorPath, - readTime.getSeconds(), - readTime.getSeconds(), - readTime.getNanoseconds()) - .forEach( - row -> { - // TODO: Actually implement a single-collection query - // - // The query is actually returning any path that starts with the query path prefix - // which may include documents in subcollections. For example, a query on 'rooms' - // will return rooms/abc/messages/xyx but we shouldn't match it. Fix this by - // discarding rows with document keys more than one segment longer than the query - // path. - ResourcePath path = EncodedPath.decodeResourcePath(row.getString(0)); - if (path.length() != immediateChildrenPathLength) { - return; - } - - byte[] rawDocument = row.getBlob(1); - - // Since scheduling background tasks incurs overhead, we only dispatch to a - // background thread if there are still some documents remaining. - Executor executor = row.isLast() ? Executors.DIRECT_EXECUTOR : backgroundQueue; - executor.execute( - () -> { - MaybeDocument maybeDoc = decodeMaybeDocument(rawDocument); - - if (maybeDoc instanceof Document && query.matches((Document) maybeDoc)) { - synchronized (SQLiteRemoteDocumentCache.this) { - matchingDocuments[0] = - matchingDocuments[0].insert(maybeDoc.getKey(), (Document) maybeDoc); - } - } - }); - }); + sqlQuery.forEach( + row -> { + // TODO: Actually implement a single-collection query + // + // The query is actually returning any path that starts with the query path prefix + // which may include documents in subcollections. For example, a query on 'rooms' + // will return rooms/abc/messages/xyx but we shouldn't match it. Fix this by + // discarding rows with document keys more than one segment longer than the query + // path. + ResourcePath path = EncodedPath.decodeResourcePath(row.getString(0)); + if (path.length() != immediateChildrenPathLength) { + return; + } + + byte[] rawDocument = row.getBlob(1); + + // Since scheduling background tasks incurs overhead, we only dispatch to a + // background thread if there are still some documents remaining. + Executor executor = row.isLast() ? Executors.DIRECT_EXECUTOR : backgroundQueue; + executor.execute( + () -> { + MaybeDocument maybeDoc = decodeMaybeDocument(rawDocument); + + if (maybeDoc instanceof Document && query.matches((Document) maybeDoc)) { + synchronized (SQLiteRemoteDocumentCache.this) { + matchingDocuments[0] = + matchingDocuments[0].insert(maybeDoc.getKey(), (Document) maybeDoc); + } + } + }); + }); statsCollector.recordRowsRead(STATS_TAG, rowsProcessed); diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java index e3fbbeb9c43..8b16cbe9440 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteSchemaTest.java @@ -423,12 +423,17 @@ public void existingDocumentsRemainReadableAfterIndexFreeMigration() { new Object[] {encode(path("coll/new")), 0, 3000, createDummyDocument("coll/new")}); SQLiteRemoteDocumentCache remoteDocumentCache = createRemoteDocumentCache(); + + // Verify that queries with SnapshotVersion.NONE return all results, regardless of whether the + // read time has been set. ImmutableSortedMap results = - remoteDocumentCache.getAllDocumentsMatchingQuery(query("coll"), version(2)); + remoteDocumentCache.getAllDocumentsMatchingQuery(query("coll"), version(0)); + assertResultsContain(results, "coll/existing", "coll/old", "coll/current", "coll/new"); - // Verify that queries return the documents that existed prior to the index-free migration, as - // well as any documents with a newer read time than the one passed in. - assertResultsContain(results, "coll/existing", "coll/new"); + // Queries that filter by read time only return documents that were written after the index-free + // migration. + results = remoteDocumentCache.getAllDocumentsMatchingQuery(query("coll"), version(2)); + assertResultsContain(results, "coll/new"); } private SQLiteRemoteDocumentCache createRemoteDocumentCache() {