Skip to content

Commit 99b25a4

Browse files
cherylEnkiducherylEnkidu
committed
Improve readability
1 parent 824195e commit 99b25a4

File tree

5 files changed

+74
-34
lines changed

5 files changed

+74
-34
lines changed

‎firebase-firestore/src/androidTest/java/com/google/firebase/firestore/IndexingTest.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ public void testBadIndexDoesNotCrashClient() {
108108
+ "}"));
109109
}
110110

111+
/**
112+
* 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.
113+
*/
111114
@Test
112115
public void testAutoIndexCreationSetSuccessfully() {
113116
// Use persistent disk cache (default)
@@ -128,19 +131,21 @@ public void testAutoIndexCreationSetSuccessfully() {
128131
assertEquals(1, results.size());
129132

130133
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());
131-
132-
results = waitFor(collection.whereEqualTo("match", true).get());
134+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
133135
assertEquals(1, results.size());
134136

135137
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());
136-
137-
results = waitFor(collection.whereEqualTo("match", true).get());
138+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
138139
assertEquals(1, results.size());
139140

140141
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().deleteAllIndexes());
142+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
141143
assertEquals(1, results.size());
142144
}
143145

146+
/**
147+
* 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.
148+
*/
144149
@Test
145150
public void testAutoIndexCreationSetSuccessfullyUsingDefault() {
146151
// Use persistent disk cache (default)
@@ -156,16 +161,15 @@ public void testAutoIndexCreationSetSuccessfullyUsingDefault() {
156161
assertEquals(1, results.size());
157162

158163
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().enableIndexAutoCreation());
159-
160-
results = waitFor(collection.whereEqualTo("match", true).get());
164+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
161165
assertEquals(1, results.size());
162166

163167
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().disableIndexAutoCreation());
164-
165-
results = waitFor(collection.whereEqualTo("match", true).get());
168+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
166169
assertEquals(1, results.size());
167170

168171
assertDoesNotThrow(() -> db.getPersistentCacheIndexManager().deleteAllIndexes());
172+
results = waitFor(collection.whereEqualTo("match", true).get(Source.CACHE));
169173
assertEquals(1, results.size());
170174
}
171175

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
120120
* Decides whether SDK should create a full matched field index for this query based on query
121121
* context and query result size.
122122
*/
123-
// TODO(csi): Auto experiment data.
124123
private void createCacheIndexes(Query query, QueryContext context, int resultSize) {
125124
if (context.getDocumentReadCount() < indexAutoCreationMinCollectionSize) {
126125
Logger.debug(

‎firebase-firestore/src/main/java/com/google/firebase/firestore/model/TargetIndexMatcher.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,9 @@ public FieldIndex buildTargetIndex() {
219219
}
220220
}
221221

222+
// Note: We do not explicitly check `inequalityFilter` but rather rely on the target defining an
223+
// appropriate `orderBys` to ensure that the required index segment is added. The query engine
224+
// would reject a query with an inequality filter that lacks the required order-by clause.
222225
for (OrderBy orderBy : orderBys) {
223226
// Stop adding more segments if we see a order-by on key. Typically this is the default
224227
// implicit order-by which is covered in the index_entry table as a separate column.

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

Lines changed: 58 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import static com.google.firebase.firestore.testutil.TestUtil.key;
2525
import static com.google.firebase.firestore.testutil.TestUtil.keyMap;
2626
import static com.google.firebase.firestore.testutil.TestUtil.map;
27+
import static com.google.firebase.firestore.testutil.TestUtil.orFilters;
2728
import static com.google.firebase.firestore.testutil.TestUtil.orderBy;
2829
import static com.google.firebase.firestore.testutil.TestUtil.query;
2930
import static com.google.firebase.firestore.testutil.TestUtil.setMutation;
@@ -398,19 +399,50 @@ public void testCanAutoCreateIndexes() {
398399
assertQueryReturned("coll/a", "coll/e", "coll/f");
399400
}
400401

402+
@Test
403+
public void testCanAutoCreateIndexesWorksWithOrQuery() {
404+
Query query = query("coll").filter(orFilters(filter("a", "==", 3), filter("b", "==", true))).orderBy(orderBy("time", "desc"));
405+
int targetId = allocateQuery(query);
406+
407+
setIndexAutoCreationEnabled(true);
408+
setMinCollectionSizeToAutoCreateIndex(0);
409+
setRelativeIndexReadCostPerDocument(2);
410+
411+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("time", 0, "b", true)), targetId));
412+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("time", 2, "b", false)), targetId));
413+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("time", 7, "a", 5, "b", false)), targetId));
414+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("time", 3, "a", true)), targetId));
415+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("time", 1, "a", 3, "b", true)), targetId));
416+
417+
// First time query runs without indexes.
418+
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
419+
// Full matched index should be created.
420+
executeQuery(query);
421+
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
422+
assertQueryReturned("coll/e", "coll/a");
423+
424+
backfillIndexes();
425+
426+
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("time", 7, "a", 3, "b", false)), targetId));
427+
428+
executeQuery(query);
429+
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 1);
430+
assertQueryReturned("coll/f", "coll/e", "coll/a");
431+
}
432+
401433
@Test
402434
public void testDoesNotAutoCreateIndexesForSmallCollections() {
403-
Query query = query("coll").filter(filter("count", ">=", 3));
435+
Query query = query("coll").filter(filter("foo", "==", 9)).filter(filter("count", ">=", 3));
404436
int targetId = allocateQuery(query);
405437

406438
setIndexAutoCreationEnabled(true);
407439
setRelativeIndexReadCostPerDocument(2);
408440

409-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("count", 5)), targetId));
410-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("count", 1)), targetId));
411-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("count", 0)), targetId));
412-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 1)), targetId));
413-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("count", 3)), targetId));
441+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("foo", 9, "count", 5)), targetId));
442+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("foo", 8, "count", 6)), targetId));
443+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("foo", 9, "count", 0)), targetId));
444+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("count", 4)), targetId));
445+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("foo", 9, "count", 3)), targetId));
414446

415447
// SDK will not create indexes since collection size is too small.
416448
executeQuery(query);
@@ -419,7 +451,7 @@ public void testDoesNotAutoCreateIndexesForSmallCollections() {
419451

420452
backfillIndexes();
421453

422-
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("count", 4)), targetId));
454+
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("foo", 9, "count", 4)), targetId));
423455

424456
executeQuery(query);
425457
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 3);
@@ -462,18 +494,18 @@ public void testDoesNotAutoCreateIndexesWhenIndexLookUpIsExpensive() {
462494

463495
@Test
464496
public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
465-
Query query = query("coll").filter(filter("matches", "==", "foo"));
497+
Query query = query("coll").filter(filter("matches", "==", "foo")).filter(filter("count", ">", 10));
466498
int targetId = allocateQuery(query);
467499

468500
setIndexAutoCreationEnabled(true);
469501
setMinCollectionSizeToAutoCreateIndex(0);
470502
setRelativeIndexReadCostPerDocument(2);
471503

472-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo")), targetId));
473-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "")), targetId));
474-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "bar")), targetId));
475-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7)), targetId));
476-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo")), targetId));
504+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo", "count", 11)), targetId));
505+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "foo", "count", 9)), targetId));
506+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "foo")), targetId));
507+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7, "count", 11)), targetId));
508+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo", "count", 21)), targetId));
477509

478510
// First time query is running without indexes.
479511
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
@@ -486,7 +518,7 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
486518
setBackfillerMaxDocumentsToProcess(2);
487519
backfillIndexes();
488520

489-
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo")), targetId));
521+
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo" , "count", 15)), targetId));
490522

491523
executeQuery(query);
492524
assertRemoteDocumentsRead(/* byKey= */ 1, /* byCollection= */ 2);
@@ -495,7 +527,7 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
495527

496528
@Test
497529
public void testIndexCreatedByIndexAutoCreationExistsAfterTurnOffAutoCreation() {
498-
Query query = query("coll").filter(filter("value", "not-in", Collections.singletonList(3)));
530+
Query query = query("coll").filter(filter("value", "not-in", Collections.singletonList(3))).orderBy(orderBy("value", "asc"));
499531
int targetId = allocateQuery(query);
500532

501533
setIndexAutoCreationEnabled(true);
@@ -567,49 +599,51 @@ public void testDisableIndexAutoCreationWorks() {
567599

568600
executeQuery(query2);
569601
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
602+
assertQueryReturned("foo/a", "foo/e");
570603

571604
backfillIndexes();
572605

573606
// Run the query in second time, test index won't be created
574607
executeQuery(query2);
575608
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
609+
assertQueryReturned("foo/a", "foo/e");
576610
}
577611

578612
@Test
579613
public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
580-
Query query = query("coll").filter(filter("value", "==", "match"));
614+
Query query = query("coll").filter(filter("value", "==", "match")).orderBy(orderBy("time", "asc"));
581615
int targetId = allocateQuery(query);
582616

583617
setIndexAutoCreationEnabled(true);
584618
setMinCollectionSizeToAutoCreateIndex(0);
585619
setRelativeIndexReadCostPerDocument(2);
586620

587-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("value", "match")), targetId));
588-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("value", Double.NaN)), targetId));
589-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("value", null)), targetId));
590-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("value", "mismatch")), targetId));
591-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("value", "match")), targetId));
621+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("time", 1, "value", "match")), targetId));
622+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("time", 2, "value", Double.NaN)), targetId));
623+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("time", 7, "value", null)), targetId));
624+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("time", 3, "value", "mismatch")), targetId));
625+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("time", 0, "value", "match")), targetId));
592626

593627
// First time query is running without indexes.
594628
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
595629
// Full matched index should be created.
596630
executeQuery(query);
597631
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
598-
assertQueryReturned("coll/a", "coll/e");
632+
assertQueryReturned("coll/e", "coll/a");
599633

600634
setIndexAutoCreationEnabled(false);
601635

602636
backfillIndexes();
603637

604638
executeQuery(query);
605639
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 0);
606-
assertQueryReturned("coll/a", "coll/e");
640+
assertQueryReturned("coll/e", "coll/a");
607641

608642
deleteAllIndexes();
609643

610644
executeQuery(query);
611645
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
612-
assertQueryReturned("coll/a", "coll/e");
646+
assertQueryReturned("coll/e", "coll/a");
613647
}
614648

615649
@Test

‎firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -664,7 +664,7 @@ public void testBuildTargetIndexWithQueriesWithArrayContains() {
664664
}
665665

666666
@Test
667-
public void testBuildTargetIndexWithQueriesWithOrderBy() {
667+
public void testBuildTargetIndexWithQueriesWithOrderBys() {
668668
for (Query query : queriesWithOrderBys) {
669669
validateBuildTargetIndexCreateFullMatchIndex(query);
670670
}

0 commit comments

Comments
 (0)