Skip to content

Commit 17880f9

Browse files
Remove canServeFromIndex
1 parent dde5de1 commit 17880f9

File tree

6 files changed

+30
-36
lines changed

6 files changed

+30
-36
lines changed

firebase-firestore/src/main/java/com/google/firebase/firestore/local/IndexManager.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,6 @@ public interface IndexManager {
7575
/** Returns all configured field indexes. */
7676
Collection<FieldIndex> getFieldIndexes();
7777

78-
/** Returns whether we can serve the given target from an index. */
79-
boolean canServeFromIndex(Target target);
80-
8178
/**
8279
* Iterates over all field indexes that are used to serve the given target, and returns the
8380
* minimum offset of them all. Asserts that the target can be served from index.
@@ -94,7 +91,10 @@ public interface IndexManager {
9491
@Nullable
9592
FieldIndex getFieldIndex(Target target);
9693

97-
/** Returns the documents that match the given target based on the provided index. */
94+
/**
95+
* Returns the documents that match the given target based on the provided index. Returns {@code
96+
* null} if the query cannot be served from an index.
97+
*/
9898
Set<DocumentKey> getDocumentsMatchingTarget(Target target);
9999

100100
/** Returns the next collection group to update. Returns {@code null} if no group exists. */

firebase-firestore/src/main/java/com/google/firebase/firestore/local/MemoryIndexManager.java

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,6 @@ public Collection<FieldIndex> getFieldIndexes() {
9999
return Collections.emptyList();
100100
}
101101

102-
@Override
103-
public boolean canServeFromIndex(Target target) {
104-
return false;
105-
}
106-
107102
@Override
108103
public IndexOffset getMinOffset(Target target) {
109104
return IndexOffset.NONE;

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,13 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
110110
return null;
111111
}
112112

113-
if (!indexManager.canServeFromIndex(target)) {
113+
Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
114+
if (keys == null) {
114115
return null;
115116
}
116117

117-
Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
118118
ImmutableSortedMap<DocumentKey, Document> indexedDocuments =
119119
localDocumentsView.getDocuments(keys);
120-
121120
return appendRemainingResults(
122121
values(indexedDocuments), query, indexManager.getMinOffset(target));
123122
}

firebase-firestore/src/main/java/com/google/firebase/firestore/local/SQLiteIndexManager.java

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -279,18 +279,6 @@ public Collection<FieldIndex> getFieldIndexes() {
279279
return allIndices;
280280
}
281281

282-
@Override
283-
public boolean canServeFromIndex(Target target) {
284-
for (Target subTarget : getSubTargets(target)) {
285-
// If any of the sub-queries cannot be served from the index, the target as a whole cannot be
286-
// served from the index.
287-
if (getFieldIndex(subTarget) == null) {
288-
return false;
289-
}
290-
}
291-
return true;
292-
}
293-
294282
private IndexOffset getMinOffset(Collection<FieldIndex> fieldIndexes) {
295283
hardAssert(
296284
!fieldIndexes.isEmpty(),
@@ -319,9 +307,6 @@ public IndexOffset getMinOffset(String collectionGroup) {
319307

320308
@Override
321309
public IndexOffset getMinOffset(Target target) {
322-
hardAssert(
323-
canServeFromIndex(target),
324-
"Cannot find least recent index offset if target cannot be served from index.");
325310
List<FieldIndex> fieldIndexes = new ArrayList<>();
326311
for (Target subTarget : getSubTargets(target)) {
327312
fieldIndexes.add(getFieldIndex(subTarget));
@@ -458,6 +443,10 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
458443

459444
for (Target subTarget : getSubTargets(target)) {
460445
FieldIndex fieldIndex = getFieldIndex(subTarget);
446+
if (fieldIndex == null) {
447+
return null;
448+
}
449+
461450
@Nullable List<Value> arrayValues = subTarget.getArrayValues(fieldIndex);
462451
@Nullable List<Value> notInValues = subTarget.getNotInValues(fieldIndex);
463452
@Nullable Bound lowerBound = subTarget.getLowerBound(fieldIndex);
@@ -684,13 +673,13 @@ private byte[] encodeSingleElement(Value value) {
684673
* queries, a list of possible values is returned.
685674
*/
686675
private @Nullable Object[] encodeValues(
687-
FieldIndex fieldIndex, Target target, @Nullable List<Value> bound) {
688-
if (bound == null) return null;
676+
FieldIndex fieldIndex, Target target, @Nullable List<Value> values) {
677+
if (values == null) return null;
689678

690679
List<IndexByteEncoder> encoders = new ArrayList<>();
691680
encoders.add(new IndexByteEncoder());
692681

693-
Iterator<Value> position = bound.iterator();
682+
Iterator<Value> position = values.iterator();
694683
for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
695684
Value value = position.next();
696685
for (IndexByteEncoder encoder : encoders) {

firebase-firestore/src/test/java/com/google/firebase/firestore/local/IndexBackfillerTest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -510,12 +510,12 @@ private void addFieldIndex(String collectionGroup, String fieldName, long sequen
510510

511511
private void verifyQueryResults(Query query, String... expectedKeys) {
512512
Target target = query.toTarget();
513-
if (indexManager.canServeFromIndex(target)) {
514-
Set<DocumentKey> actualKeys = indexManager.getDocumentsMatchingTarget(target);
513+
Set<DocumentKey> actualKeys = indexManager.getDocumentsMatchingTarget(target);
514+
if (actualKeys == null) {
515+
assertEquals(0, expectedKeys.length);
516+
} else {
515517
assertThat(actualKeys)
516518
.containsExactlyElementsIn(Arrays.stream(expectedKeys).map(TestUtil::key).toArray());
517-
} else {
518-
assertEquals(0, expectedKeys.length);
519519
}
520520
}
521521

firebase-firestore/src/test/java/com/google/firebase/firestore/local/SQLiteIndexManagerTest.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@
3131
import static com.google.firebase.firestore.testutil.TestUtil.version;
3232
import static com.google.firebase.firestore.testutil.TestUtil.wrap;
3333
import static org.junit.Assert.assertEquals;
34+
import static org.junit.Assert.assertNotNull;
3435
import static org.junit.Assert.assertNull;
35-
import static org.junit.Assert.assertTrue;
3636

3737
import com.google.firebase.firestore.auth.User;
3838
import com.google.firebase.firestore.core.Query;
@@ -88,6 +88,16 @@ public void addsDocuments() {
8888
addDoc("coll/doc2", map());
8989
}
9090

91+
@Test
92+
public void testOrderByFilter() {
93+
indexManager.addFieldIndex(fieldIndex("coll", "count", Kind.ASCENDING));
94+
addDoc("coll/val1", map("count", 1));
95+
addDoc("coll/val2", map("not-count", 2));
96+
addDoc("coll/val3", map("count", 3));
97+
Query query = query("coll").orderBy(orderBy("count"));
98+
verifyResults(query, "coll/val1", "coll/val3");
99+
}
100+
91101
@Test
92102
public void testEqualityFilter() {
93103
setUpSingleValueFilter();
@@ -248,6 +258,7 @@ public void testNoMatchingFilter() {
248258
setUpSingleValueFilter();
249259
Query query = query("coll").filter(filter("unknown", "==", true));
250260
assertNull(indexManager.getFieldIndex(query.toTarget()));
261+
assertNull(indexManager.getDocumentsMatchingTarget(query.toTarget()));
251262
}
252263

253264
@Test
@@ -709,8 +720,8 @@ private void addDoc(String key, Map<String, Object> data) {
709720

710721
private void verifyResults(Query query, String... documents) {
711722
Target target = query.toTarget();
712-
assertTrue("Target cannot be served from index.", indexManager.canServeFromIndex(target));
713723
Iterable<DocumentKey> results = indexManager.getDocumentsMatchingTarget(target);
724+
assertNotNull("Target cannot be served from index.", results);
714725
List<DocumentKey> keys = Arrays.stream(documents).map(s -> key(s)).collect(Collectors.toList());
715726
assertWithMessage("Result for %s", query).that(results).containsExactlyElementsIn(keys);
716727
}

0 commit comments

Comments
 (0)