Skip to content

Commit

Permalink
Merge b6c3465 into f2a0fa3
Browse files Browse the repository at this point in the history
  • Loading branch information
@cherylEnkidu
cherylEnkidu authored Aug 17, 2023
2 parents f2a0fa3 + b6c3465 commit 5647871
Show file tree
Hide file tree
Showing 7 changed files with 198 additions and 27 deletions.
73 changes: 73 additions & 0 deletions firebase-firestore/src/androidTest/java/com/google/firebase/firestore/FirestoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1419,4 +1419,77 @@ public void testCannotGetDocumentWithMemoryEagerGcEnabled() {
assertTrue(e instanceof FirebaseFirestoreException);
assertEquals(((FirebaseFirestoreException) e).getCode(), Code.UNAVAILABLE);
}

@Test
public void testGetPersistentCacheIndexManager() {
// Use persistent disk cache (explicit)
FirebaseFirestore db1 = testFirestore();
FirebaseFirestoreSettings settings1 =
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
.build();
db1.setFirestoreSettings(settings1);
assertNotNull(db1.getPersistentCacheIndexManager());

// Use persistent disk cache (default)
FirebaseFirestore db2 = testFirestore();
assertNotNull(db2.getPersistentCacheIndexManager());

// Disable persistent disk cache
FirebaseFirestore db3 = testFirestore();
FirebaseFirestoreSettings settings3 =
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
.setLocalCacheSettings(MemoryCacheSettings.newBuilder().build())
.build();
db3.setFirestoreSettings(settings3);
assertNull(db3.getPersistentCacheIndexManager());

// Disable persistent disk cache (deprecated)
FirebaseFirestore db4 = testFirestore();
FirebaseFirestoreSettings settings4 =
new FirebaseFirestoreSettings.Builder(db4.getFirestoreSettings())
.setPersistenceEnabled(false)
.build();
db4.setFirestoreSettings(settings4);
assertNull(db4.getPersistentCacheIndexManager());
}

@Test
public void testCanGetSameOrDifferentPersistentCacheIndexManager() {
// Use persistent disk cache (explicit)
FirebaseFirestore db1 = testFirestore();
FirebaseFirestoreSettings settings1 =
new FirebaseFirestoreSettings.Builder(db1.getFirestoreSettings())
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
.build();
db1.setFirestoreSettings(settings1);
PersistentCacheIndexManager indexManager1 = db1.getPersistentCacheIndexManager();
PersistentCacheIndexManager indexManager2 = db1.getPersistentCacheIndexManager();
assertSame(indexManager1, indexManager2);

// Use persistent disk cache (default)
FirebaseFirestore db2 = testFirestore();
PersistentCacheIndexManager indexManager3 = db2.getPersistentCacheIndexManager();
PersistentCacheIndexManager indexManager4 = db2.getPersistentCacheIndexManager();
assertSame(indexManager3, indexManager4);

assertNotSame(indexManager1, indexManager3);

FirebaseFirestore db3 = testFirestore();
FirebaseFirestoreSettings settings3 =
new FirebaseFirestoreSettings.Builder(db3.getFirestoreSettings())
.setLocalCacheSettings(PersistentCacheSettings.newBuilder().build())
.build();
db3.setFirestoreSettings(settings3);
PersistentCacheIndexManager indexManager5 = db3.getPersistentCacheIndexManager();
assertNotSame(indexManager1, indexManager5);
assertNotSame(indexManager3, indexManager5);

// Use persistent disk cache (default)
FirebaseFirestore db4 = testFirestore();
PersistentCacheIndexManager indexManager6 = db4.getPersistentCacheIndexManager();
assertNotSame(indexManager1, indexManager6);
assertNotSame(indexManager3, indexManager6);
assertNotSame(indexManager5, indexManager6);
}
}
26 changes: 17 additions & 9 deletions firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,9 +108,13 @@ 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)
// Use persistent disk cache (explicit)
FirebaseFirestore db = testFirestore();
FirebaseFirestoreSettings settings =
new FirebaseFirestoreSettings.Builder(db.getFirestoreSettings())
Expand All @@ -128,19 +132,22 @@ 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)
Expand All @@ -156,16 +163,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());
}

Expand All @@ -186,4 +192,6 @@ public void testAutoIndexCreationAfterFailsTermination() {
() -> db.getPersistentCacheIndexManager().deleteAllIndexes(),
"The client has already been terminated");
}

// TODO(b/296100693) Add testing hooks to verify indexes are created as expected.
}
1 change: 0 additions & 1 deletion firebase-firestore/src/main/java/com/google/firebase/firestore/local/QueryEngine.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ public ImmutableSortedMap<DocumentKey, Document> 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(
Expand Down
3 changes: 3 additions & 0 deletions firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
44 changes: 44 additions & 0 deletions ...e-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static com.google.firebase.firestore.model.FieldIndex.IndexState;
import static com.google.firebase.firestore.model.FieldIndex.Segment.Kind;
import static com.google.firebase.firestore.testutil.TestUtil.andFilters;
import static com.google.firebase.firestore.testutil.TestUtil.bound;
import static com.google.firebase.firestore.testutil.TestUtil.deletedDoc;
import static com.google.firebase.firestore.testutil.TestUtil.doc;
Expand Down Expand Up @@ -1189,6 +1190,49 @@ public void testIndexTypeForOrQueries() throws Exception {
validateIndexType(query11, IndexManager.IndexType.PARTIAL);
}

@Test
public void TestCreateTargetIndexesCreatesFullIndexesForEachSubTarget() {
Query query =
query("coll")
.filter(orFilters(filter("a", "==", 1), filter("b", "==", 2), filter("c", "==", 3)));

Query subQuery1 = query("coll").filter(filter("a", "==", 1));
Query subQuery2 = query("coll").filter(filter("b", "==", 2));
Query subQuery3 = query("coll").filter(filter("c", "==", 3));

validateIndexType(query, IndexManager.IndexType.NONE);
validateIndexType(subQuery1, IndexManager.IndexType.NONE);
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
validateIndexType(subQuery3, IndexManager.IndexType.NONE);

indexManager.createTargetIndexes(query.toTarget());

validateIndexType(query, IndexManager.IndexType.FULL);
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
validateIndexType(subQuery2, IndexManager.IndexType.FULL);
validateIndexType(subQuery3, IndexManager.IndexType.FULL);
}

@Test
public void TestCreateTargetIndexesUpgradesPartialIndexToFullIndex() {
Query query = query("coll").filter(andFilters(filter("a", "==", 1), filter("b", "==", 2)));

Query subQuery1 = query("coll").filter(filter("a", "==", 1));
Query subQuery2 = query("coll").filter(filter("b", "==", 2));

indexManager.createTargetIndexes(subQuery1.toTarget());

validateIndexType(query, IndexManager.IndexType.PARTIAL);
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
validateIndexType(subQuery2, IndexManager.IndexType.NONE);

indexManager.createTargetIndexes(query.toTarget());

validateIndexType(query, IndexManager.IndexType.FULL);
validateIndexType(subQuery1, IndexManager.IndexType.FULL);
validateIndexType(subQuery2, IndexManager.IndexType.NONE);
}

private void validateIndexType(Query query, IndexManager.IndexType expected) {
IndexManager.IndexType indexType = indexManager.getIndexType(query.toTarget());
assertEquals(indexType, expected);
Expand Down
76 changes: 60 additions & 16 deletions ...ase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteLocalStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)));
int targetId = allocateQuery(query);

setIndexAutoCreationEnabled(true);
setMinCollectionSizeToAutoCreateIndex(0);
setRelativeIndexReadCostPerDocument(2);

applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("b", true)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("b", false)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("a", 5, "b", false)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("a", true)), targetId));
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("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("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);
Expand All @@ -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);
Expand Down Expand Up @@ -460,18 +492,22 @@ 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).
Expand All @@ -483,7 +519,8 @@ 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);
Expand Down Expand Up @@ -564,12 +601,14 @@ 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
Expand All @@ -594,8 +633,6 @@ public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
assertQueryReturned("coll/a", "coll/e");

setIndexAutoCreationEnabled(false);

backfillIndexes();

executeQuery(query);
Expand All @@ -607,6 +644,13 @@ public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
executeQuery(query);
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
assertQueryReturned("coll/a", "coll/e");

// Field index is created again.
backfillIndexes();

executeQuery(query);
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 0);
assertQueryReturned("coll/a", "coll/e");
}

@Test
Expand Down
2 changes: 1 addition & 1 deletion ...e-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ public void testBuildTargetIndexWithQueriesWithArrayContains() {
}

@Test
public void testBuildTargetIndexWithQueriesWithOrderBy() {
public void testBuildTargetIndexWithQueriesWithOrderBys() {
for (Query query : queriesWithOrderBys) {
validateBuildTargetIndexCreateFullMatchIndex(query);
}
Expand Down

0 comments on commit 5647871

Please sign in to comment.