Backport indexing changes (#3444)

Author: schmidt-sebastian

firebase-firestore/src/main/java/com/google/firebase/firestore/core/Target.java

@@ -14,6 +14,8 @@
 
 package com.google.firebase.firestore.core;
 
+import static com.google.firebase.firestore.model.Values.MAX_VALUE;
+import static com.google.firebase.firestore.model.Values.MIN_VALUE;
 import static com.google.firebase.firestore.model.Values.max;
 import static com.google.firebase.firestore.model.Values.min;
 
@@ -212,14 +214,11 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
             filterValue = Values.MIN_VALUE;
             break;
           case NOT_IN:
-            {
-              ArrayValue.Builder arrayValue = ArrayValue.newBuilder();
-              for (int i = 0; i < fieldFilter.getValue().getArrayValue().getValuesCount(); ++i) {
-                arrayValue.addValues(Values.MIN_VALUE);
-              }
-              filterValue = Value.newBuilder().setArrayValue(arrayValue).build();
-              break;
-            }
+            filterValue =
+                Value.newBuilder()
+                    .setArrayValue(ArrayValue.newBuilder().addValues(MIN_VALUE))
+                    .build();
+            break;
           default:
             // Remaining filters cannot be used as lower bounds.
         }
@@ -295,14 +294,11 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
             filterValue = Values.MAX_VALUE;
             break;
           case NOT_IN:
-            {
-              ArrayValue.Builder arrayValue = ArrayValue.newBuilder();
-              for (int i = 0; i < fieldFilter.getValue().getArrayValue().getValuesCount(); ++i) {
-                arrayValue.addValues(Values.MAX_VALUE);
-              }
-              filterValue = Value.newBuilder().setArrayValue(arrayValue).build();
-              break;
-            }
+            filterValue =
+                Value.newBuilder()
+                    .setArrayValue(ArrayValue.newBuilder().addValues(MAX_VALUE))
+                    .build();
+            break;
           default:
             // Remaining filters cannot be used as upper bounds.
         }

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

@@ -75,9 +75,6 @@ public interface IndexManager {
   /** Returns all configured field indexes. */
   Collection<FieldIndex> getFieldIndexes();
 
-  /** Returns whether we can serve the given target from an index. */
-  boolean canServeFromIndex(Target target);
-
   /**
    * Iterates over all field indexes that are used to serve the given target, and returns the
    * minimum offset of them all. Asserts that the target can be served from index.
@@ -94,7 +91,10 @@ public interface IndexManager {
   @Nullable
   FieldIndex getFieldIndex(Target target);
 
-  /** Returns the documents that match the given target based on the provided index. */
+  /**
+   * Returns the documents that match the given target based on the provided index or {@code null}
+   * if the query cannot be served from an index.
+   */
   Set<DocumentKey> getDocumentsMatchingTarget(Target target);
 
   /** 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

@@ -72,7 +72,7 @@ public FieldIndex getFieldIndex(Target target) {
   @Nullable
   public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
     // Field indices are not supported with memory persistence.
-    return Collections.emptySet();
+    return null;
   }
 
   @Nullable
@@ -99,11 +99,6 @@ public Collection<FieldIndex> getFieldIndexes() {
     return Collections.emptyList();
   }
 
-  @Override
-  public boolean canServeFromIndex(Target target) {
-    return false;
-  }
-
   @Override
   public IndexOffset getMinOffset(Target target) {
     return IndexOffset.NONE;

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

@@ -110,14 +110,13 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
       return null;
     }
 
-    if (!indexManager.canServeFromIndex(target)) {
+    Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
+    if (keys == null) {
       return null;
     }
 
-    Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
     ImmutableSortedMap<DocumentKey, Document> indexedDocuments =
         localDocumentsView.getDocuments(keys);
-
     return appendRemainingResults(
         values(indexedDocuments), query, indexManager.getMinOffset(target));
   }

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

@@ -279,18 +279,6 @@ public Collection<FieldIndex> getFieldIndexes() {
     return allIndices;
   }
 
-  @Override
-  public boolean canServeFromIndex(Target target) {
-    for (Target subTarget : getSubTargets(target)) {
-      // If any of the sub-queries cannot be served from the index, the target as a whole cannot be
-      // served from the index.
-      if (getFieldIndex(subTarget) == null) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   private IndexOffset getMinOffset(Collection<FieldIndex> fieldIndexes) {
     hardAssert(
         !fieldIndexes.isEmpty(),
@@ -319,9 +307,6 @@ public IndexOffset getMinOffset(String collectionGroup) {
 
   @Override
   public IndexOffset getMinOffset(Target target) {
-    hardAssert(
-        canServeFromIndex(target),
-        "Cannot find least recent index offset if target cannot be served from index.");
     List<FieldIndex> fieldIndexes = new ArrayList<>();
     for (Target subTarget : getSubTargets(target)) {
       fieldIndexes.add(getFieldIndex(subTarget));
@@ -458,6 +443,10 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
 
     for (Target subTarget : getSubTargets(target)) {
       FieldIndex fieldIndex = getFieldIndex(subTarget);
+      if (fieldIndex == null) {
+        return null;
+      }
+
       @Nullable List<Value> arrayValues = subTarget.getArrayValues(fieldIndex);
       @Nullable List<Value> notInValues = subTarget.getNotInValues(fieldIndex);
       @Nullable Bound lowerBound = subTarget.getLowerBound(fieldIndex);
@@ -684,13 +673,13 @@ private byte[] encodeSingleElement(Value value) {
    * queries, a list of possible values is returned.
    */
   private @Nullable Object[] encodeValues(
-      FieldIndex fieldIndex, Target target, @Nullable List<Value> bound) {
-    if (bound == null) return null;
+      FieldIndex fieldIndex, Target target, @Nullable List<Value> values) {
+    if (values == null) return null;
 
     List<IndexByteEncoder> encoders = new ArrayList<>();
     encoders.add(new IndexByteEncoder());
 
-    Iterator<Value> position = bound.iterator();
+    Iterator<Value> position = values.iterator();
     for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
       Value value = position.next();
       for (IndexByteEncoder encoder : encoders) {

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

@@ -510,12 +510,12 @@ private void addFieldIndex(String collectionGroup, String fieldName, long sequen
 
   private void verifyQueryResults(Query query, String... expectedKeys) {
     Target target = query.toTarget();
-    if (indexManager.canServeFromIndex(target)) {
-      Set<DocumentKey> actualKeys = indexManager.getDocumentsMatchingTarget(target);
+    Set<DocumentKey> actualKeys = indexManager.getDocumentsMatchingTarget(target);
+    if (actualKeys == null) {
+      assertEquals(0, expectedKeys.length);
+    } else {
       assertThat(actualKeys)
           .containsExactlyElementsIn(Arrays.stream(expectedKeys).map(TestUtil::key).toArray());
-    } else {
-      assertEquals(0, expectedKeys.length);
     }
   }
 

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

@@ -31,8 +31,8 @@
 import static com.google.firebase.firestore.testutil.TestUtil.version;
 import static com.google.firebase.firestore.testutil.TestUtil.wrap;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
 
 import com.google.firebase.firestore.auth.User;
 import com.google.firebase.firestore.core.Query;
@@ -88,6 +88,16 @@ public void addsDocuments() {
     addDoc("coll/doc2", map());
   }
 
+  @Test
+  public void testOrderByFilter() {
+    indexManager.addFieldIndex(fieldIndex("coll", "count", Kind.ASCENDING));
+    addDoc("coll/val1", map("count", 1));
+    addDoc("coll/val2", map("not-count", 2));
+    addDoc("coll/val3", map("count", 3));
+    Query query = query("coll").orderBy(orderBy("count"));
+    verifyResults(query, "coll/val1", "coll/val3");
+  }
+
   @Test
   public void testEqualityFilter() {
     setUpSingleValueFilter();
@@ -248,6 +258,7 @@ public void testNoMatchingFilter() {
     setUpSingleValueFilter();
     Query query = query("coll").filter(filter("unknown", "==", true));
     assertNull(indexManager.getFieldIndex(query.toTarget()));
+    assertNull(indexManager.getDocumentsMatchingTarget(query.toTarget()));
   }
 
   @Test
@@ -724,8 +735,8 @@ private void addDoc(String key, Map<String, Object> data) {
 
   private void verifyResults(Query query, String... documents) {
     Target target = query.toTarget();
-    assertTrue("Target cannot be served from index.", indexManager.canServeFromIndex(target));
     Iterable<DocumentKey> results = indexManager.getDocumentsMatchingTarget(target);
+    assertNotNull("Target cannot be served from index.", results);
     List<DocumentKey> keys = Arrays.stream(documents).map(s -> key(s)).collect(Collectors.toList());
     assertWithMessage("Result for %s", query).that(results).containsExactlyElementsIn(keys);
   }