Skip to content

Commit

Permalink
Improve readability
Browse files Browse the repository at this point in the history
  • Loading branch information
@cherylEnkidu
cherylEnkidu committed Aug 10, 2023
1 parent 177c51f commit 09498d1
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 34 deletions.
20 changes: 12 additions & 8 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,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)
Expand All @@ -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)
Expand All @@ -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());
}

Expand Down
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
82 changes: 58 additions & 24 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))).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);
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,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).
Expand All @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -564,49 +596,51 @@ 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);

backfillIndexes();

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
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 09498d1

Please sign in to comment.