Skip to content

Commit 09498d1

Browse files
cherylEnkiducherylEnkidu
committed
Improve readability
1 parent 177c51f commit 09498d1

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);
@@ -460,18 +492,18 @@ public void testDoesNotAutoCreateIndexesWhenIndexLookUpIsExpensive() {
460492

461493
@Test
462494
public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
463-
Query query = query("coll").filter(filter("matches", "==", "foo"));
495+
Query query = query("coll").filter(filter("matches", "==", "foo")).filter(filter("count", ">", 10));
464496
int targetId = allocateQuery(query);
465497

466498
setIndexAutoCreationEnabled(true);
467499
setMinCollectionSizeToAutoCreateIndex(0);
468500
setRelativeIndexReadCostPerDocument(2);
469501

470-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo")), targetId));
471-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "")), targetId));
472-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "bar")), targetId));
473-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7)), targetId));
474-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo")), targetId));
502+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("matches", "foo", "count", 11)), targetId));
503+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("matches", "foo", "count", 9)), targetId));
504+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("matches", "foo")), targetId));
505+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("matches", 7, "count", 11)), targetId));
506+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("matches", "foo", "count", 21)), targetId));
475507

476508
// First time query is running without indexes.
477509
// Based on current heuristic, collection document counts (5) > 2 * resultSize (2).
@@ -483,7 +515,7 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
483515
setBackfillerMaxDocumentsToProcess(2);
484516
backfillIndexes();
485517

486-
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo")), targetId));
518+
applyRemoteEvent(addedRemoteEvent(doc("coll/f", 20, map("matches", "foo" , "count", 15)), targetId));
487519

488520
executeQuery(query);
489521
assertRemoteDocumentsRead(/* byKey= */ 1, /* byCollection= */ 2);
@@ -492,7 +524,7 @@ public void testIndexAutoCreationWorksWhenBackfillerRunsHalfway() {
492524

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

498530
setIndexAutoCreationEnabled(true);
@@ -564,49 +596,51 @@ public void testDisableIndexAutoCreationWorks() {
564596

565597
executeQuery(query2);
566598
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
599+
assertQueryReturned("foo/a", "foo/e");
567600

568601
backfillIndexes();
569602

570603
// Run the query in second time, test index won't be created
571604
executeQuery(query2);
572605
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
606+
assertQueryReturned("foo/a", "foo/e");
573607
}
574608

575609
@Test
576610
public void testDeleteAllIndexesWorksWithIndexAutoCreation() {
577-
Query query = query("coll").filter(filter("value", "==", "match"));
611+
Query query = query("coll").filter(filter("value", "==", "match")).orderBy(orderBy("time", "asc"));
578612
int targetId = allocateQuery(query);
579613

580614
setIndexAutoCreationEnabled(true);
581615
setMinCollectionSizeToAutoCreateIndex(0);
582616
setRelativeIndexReadCostPerDocument(2);
583617

584-
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("value", "match")), targetId));
585-
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("value", Double.NaN)), targetId));
586-
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("value", null)), targetId));
587-
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("value", "mismatch")), targetId));
588-
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("value", "match")), targetId));
618+
applyRemoteEvent(addedRemoteEvent(doc("coll/a", 10, map("time", 1, "value", "match")), targetId));
619+
applyRemoteEvent(addedRemoteEvent(doc("coll/b", 10, map("time", 2, "value", Double.NaN)), targetId));
620+
applyRemoteEvent(addedRemoteEvent(doc("coll/c", 10, map("time", 7, "value", null)), targetId));
621+
applyRemoteEvent(addedRemoteEvent(doc("coll/d", 10, map("time", 3, "value", "mismatch")), targetId));
622+
applyRemoteEvent(addedRemoteEvent(doc("coll/e", 10, map("time", 0, "value", "match")), targetId));
589623

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

597631
setIndexAutoCreationEnabled(false);
598632

599633
backfillIndexes();
600634

601635
executeQuery(query);
602636
assertRemoteDocumentsRead(/* byKey= */ 2, /* byCollection= */ 0);
603-
assertQueryReturned("coll/a", "coll/e");
637+
assertQueryReturned("coll/e", "coll/a");
604638

605639
deleteAllIndexes();
606640

607641
executeQuery(query);
608642
assertRemoteDocumentsRead(/* byKey= */ 0, /* byCollection= */ 2);
609-
assertQueryReturned("coll/a", "coll/e");
643+
assertQueryReturned("coll/e", "coll/a");
610644
}
611645

612646
@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)