diff --git a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java index f04bd00fbec8..b80736f7b0d6 100644 --- a/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java +++ b/firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java @@ -108,6 +108,9 @@ public void testBadIndexDoesNotCrashClient() { + "}")); } + /** + * After Auto Index Creation is enabled, through public API there is no way to state of indexes sitting inside SDK. So this test only checks the API of auto index creation. + */ @Test public void testAutoIndexCreationSetSuccessfully() { // Use persistent disk cache (default) @@ -128,19 +131,21 @@ public void testAutoIndexCreationSetSuccessfully() { assertEquals(1, results.size()); assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation()); - - results = waitFor(collection.whereEqualTo("match", true).get()); + results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE)); assertEquals(1, results.size()); assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation()); - - results = waitFor(collection.whereEqualTo("match", true).get()); + results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE)); assertEquals(1, results.size()); assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().deleteAllIndexes()); + results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE)); assertEquals(1, results.size()); } + /** + * After Auto Index Creation is enabled, through public API there is no way to state of indexes sitting inside SDK. So this test only checks the API of auto index creation. + */ @Test public void testAutoIndexCreationSetSuccessfullyUsingDefault() { // Use persistent disk cache (default) @@ -156,16 +161,15 @@ public void testAutoIndexCreationSetSuccessfullyUsingDefault() { assertEquals(1, results.size()); assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation()); - - results = waitFor(collection.whereEqualTo("match", true).get()); + results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE)); assertEquals(1, results.size()); assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation()); - - results = waitFor(collection.whereEqualTo("match", true).get()); + results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE)); assertEquals(1, results.size()); assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().deleteAllIndexes()); + results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE)); assertEquals(1, results.size()); } diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java index 9c9a707e2d06..e5754092578c 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java @@ -120,7 +120,6 @@ public ImmutableSortedMap getDocumentsMatchingQuery( * Decides whether SDK should create a full matched field index for this query based on query * context and query result size. */ - // TODO(csi): Auto experiment data. private void createCacheIndexes(Query query, QueryContext context, int resultSize) { if (context.getDocumentReadCount() < indexAutoCreationMinCollectionSize) { Logger.debug( diff --git a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java index 7480bf7524a9..3afd2cb4be42 100644 --- a/firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java +++ b/firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java @@ -219,6 +219,9 @@ public FieldIndex buildTargetIndex() { } } + // Note: We do not explicitly check `inequalityFilter` but rather rely on the target defining an + // appropriate `orderBys` to ensure that the required index segment is added. The query engine + // would reject a query with an inequality filter that lacks the required order-by clause. for (OrderBy orderBy : orderBys) { // Stop adding more segments if we see a order-by on key. Typically this is the default // implicit order-by which is covered in the index_entry table as a separate column. diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java index ef4c97d662d8..b57e48543ccf 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java @@ -24,6 +24,7 @@ import static com.google.firebase.firestore.testutil.TestUtil.key; import static com.google.firebase.firestore.testutil.TestUtil.keyMap; import static com.google.firebase.firestore.testutil.TestUtil.map; +import static com.google.firebase.firestore.testutil.TestUtil.orFilters; import static com.google.firebase.firestore.testutil.TestUtil.orderBy; import static com.google.firebase.firestore.testutil.TestUtil.query; import static com.google.firebase.firestore.testutil.TestUtil.setMutation; @@ -398,19 +399,50 @@ public void testCanAutoCreateIndexes() { assertQueryReturned("coll/a", "coll/e", "coll/f"); } + @Test + public void testCanAutoCreateIndexesWorksWithOrQuery() { + Query query = query("coll").filter(orFilters(filter("a", "==", 3), filter("b", "==", true))).orderBy(orderBy("time", "desc")); + int targetId = allocateQuery(query); + + setIndexAutoCreationEnabled(true); + setMinCollectionSizeToAutoCreateIndex(0); + setRelativeIndexReadCostPerDocument(2); + + applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("time", 0, "b", true)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("time", 2, "b", false)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("time", 7, "a", 5, "b", false)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("time", 3, "a", true)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("time", 1, "a", 3, "b", true)), targetId)); + + // First time query runs without indexes. + // Based on current heuristic, collection document counts (5) > 2 * resultSize (2). + // Full matched index should be created. + executeQuery(query); + assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); + assertQueryReturned("coll/e", "coll/a"); + + backfillIndexes(); + + applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("time", 7, "a", 3, "b", false)), targetId)); + + executeQuery(query); + assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 1); + assertQueryReturned("coll/f", "coll/e", "coll/a"); + } + @Test public void testDoesNotAutoCreateIndexesForSmallCollections() { - Query query = query("coll").filter(filter("count", ">=", 3)); + Query query = query("coll").filter(filter("foo", "==", 9)).filter(filter("count", ">=", 3)); int targetId = allocateQuery(query); setIndexAutoCreationEnabled(true); setRelativeIndexReadCostPerDocument(2); - applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("count", 5)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("count", 1)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("count", 0)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 1)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("count", 3)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("foo", 9, "count", 5)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("foo", 8, "count", 6)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("foo", 9, "count", 0)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 4)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("foo", 9, "count", 3)), targetId)); // SDK will not create indexes since collection size is too small. executeQuery(query); @@ -419,7 +451,7 @@ public void testDoesNotAutoCreateIndexesForSmallCollections() { backfillIndexes(); - applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("count", 4)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("foo", 9, "count", 4)), targetId)); executeQuery(query); assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 3); @@ -460,18 +492,18 @@ public void testDoesNotAutoCreateIndexesWhenIndexLookUpIsExpensive() { @Test public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() { - Query query = query("coll").filter(filter("matches", "==", "foo")); + Query query = query("coll").filter(filter("matches", "==", "foo")).filter(filter("count", ">", 10)); int targetId = allocateQuery(query); setIndexAutoCreationEnabled(true); setMinCollectionSizeToAutoCreateIndex(0); setRelativeIndexReadCostPerDocument(2); - applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo")), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "")), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "bar")), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo")), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo", "count", 11)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "foo", "count", 9)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "foo")), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7, "count", 11)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo", "count", 21)), targetId)); // First time query is running without indexes. // Based on current heuristic, collection document counts (5) > 2 * resultSize (2). @@ -483,7 +515,7 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() { setBackfillerMaxDocumentsToProcess(2); backfillIndexes(); - applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo")), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo" , "count", 15)), targetId)); executeQuery(query); assertRemoteDocumentsRead(/* byKey= */ 1, /* byCollection= */ 2); @@ -492,7 +524,7 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() { @Test public void testIndexCreatedByIndexAutoCreationExistsAfterTurnOffAutoCreation() { - Query query = query("coll").filter(filter("value", "not-in", Collections.singletonList(3))); + Query query = query("coll").filter(filter("value", "not-in", Collections.singletonList(3))).orderBy(orderBy("value", "asc")); int targetId = allocateQuery(query); setIndexAutoCreationEnabled(true); @@ -564,35 +596,37 @@ public void testDisableIndexAutoCreationWorks() { executeQuery(query2); assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); + assertQueryReturned("foo/a", "foo/e"); backfillIndexes(); // Run the query in second time, test index won't be created executeQuery(query2); assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); + assertQueryReturned("foo/a", "foo/e"); } @Test public void testDeleteAllIndexesWorksWithIndexAutoCreation() { - Query query = query("coll").filter(filter("value", "==", "match")); + Query query = query("coll").filter(filter("value", "==", "match")).orderBy(orderBy("time", "asc")); int targetId = allocateQuery(query); setIndexAutoCreationEnabled(true); setMinCollectionSizeToAutoCreateIndex(0); setRelativeIndexReadCostPerDocument(2); - applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("value", "match")), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("value", Double.NaN)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("value", null)), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("value", "mismatch")), targetId)); - applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("value", "match")), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("time", 1, "value", "match")), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("time", 2, "value", Double.NaN)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("time", 7, "value", null)), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("time", 3, "value", "mismatch")), targetId)); + applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("time", 0, "value", "match")), targetId)); // First time query is running without indexes. // Based on current heuristic, collection document counts (5) > 2 * resultSize (2). // Full matched index should be created. executeQuery(query); assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); - assertQueryReturned("coll/a", "coll/e"); + assertQueryReturned("coll/e", "coll/a"); setIndexAutoCreationEnabled(false); @@ -600,13 +634,13 @@ public void testDeleteAllIndexesWorksWithIndexAutoCreation() { executeQuery(query); assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 0); - assertQueryReturned("coll/a", "coll/e"); + assertQueryReturned("coll/e", "coll/a"); deleteAllIndexes(); executeQuery(query); assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2); - assertQueryReturned("coll/a", "coll/e"); + assertQueryReturned("coll/e", "coll/a"); } @Test diff --git a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java index 2b28b0e28845..22b5d7380b15 100644 --- a/firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java +++ b/firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java @@ -664,7 +664,7 @@ public void testBuildTargetIndexWithQueriesWithArrayContains() { } @Test - public void testBuildTargetIndexWithQueriesWithOrderBy() { + public void testBuildTargetIndexWithQueriesWithOrderBys() { for (Query query : queriesWithOrderBys) { validateBuildTargetIndexCreateFullMatchIndex(query); }