Support descending queries (#3499)

Author: schmidt-sebastian

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

@@ -17,7 +17,9 @@
 import static com.google.firebase.firestore.model.Values.max;
 import static com.google.firebase.firestore.model.Values.min;
 
+import android.util.Pair;
 import androidx.annotation.Nullable;
+import com.google.firebase.firestore.core.OrderBy.Direction;
 import com.google.firebase.firestore.model.DocumentKey;
 import com.google.firebase.firestore.model.FieldIndex;
 import com.google.firebase.firestore.model.FieldPath;
@@ -185,65 +187,18 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
 
     // For each segment, retrieve a lower bound if there is a suitable filter or startAt.
     for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
-      Value segmentValue = null;
-      boolean segmentInclusive = true;
+      Pair<Value, Boolean> segmentBound =
+          segment.getKind().equals(FieldIndex.Segment.Kind.ASCENDING)
+              ? getAscendingBound(segment, startAt)
+              : getDescendingBound(segment, startAt);
 
-      // Process all filters to find a value for the current field segment
-      for (FieldFilter fieldFilter : getFieldFiltersForPath(segment.getFieldPath())) {
-        Value filterValue = null;
-        boolean filterInclusive = true;
-
-        switch (fieldFilter.getOperator()) {
-          case LESS_THAN:
-          case LESS_THAN_OR_EQUAL:
-            filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
-            break;
-          case EQUAL:
-          case IN:
-          case GREATER_THAN_OR_EQUAL:
-            filterValue = fieldFilter.getValue();
-            break;
-          case GREATER_THAN:
-            filterValue = fieldFilter.getValue();
-            filterInclusive = false;
-            break;
-          case NOT_EQUAL:
-          case NOT_IN:
-            filterValue = Values.MIN_VALUE;
-            break;
-          default:
-            // Remaining filters cannot be used as lower bounds.
-        }
-
-        if (max(segmentValue, filterValue) == filterValue) {
-          segmentValue = filterValue;
-          segmentInclusive = filterInclusive;
-        }
-      }
-
-      // If there is a startAt bound, compare the values against the existing boundary to see
-      // if we can narrow the scope.
-      if (startAt != null) {
-        for (int i = 0; i < orderBys.size(); ++i) {
-          OrderBy orderBy = this.orderBys.get(i);
-          if (orderBy.getField().equals(segment.getFieldPath())) {
-            Value cursorValue = startAt.getPosition().get(i);
-            if (max(segmentValue, cursorValue) == cursorValue) {
-              segmentValue = cursorValue;
-              segmentInclusive = startAt.isInclusive();
-            }
-            break;
-          }
-        }
-      }
-
-      if (segmentValue == null) {
+      if (segmentBound.first == null) {
         // No lower bound exists
         return null;
       }
 
-      values.add(segmentValue);
-      inclusive &= segmentInclusive;
+      values.add(segmentBound.first);
+      inclusive &= segmentBound.second;
     }
 
     return new Bound(values, inclusive);
@@ -259,75 +214,159 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
 
     // For each segment, retrieve an upper bound if there is a suitable filter or endAt.
     for (FieldIndex.Segment segment : fieldIndex.getDirectionalSegments()) {
-      @Nullable Value segmentValue = null;
-      boolean segmentInclusive = true;
+      Pair<Value, Boolean> segmentBound =
+          segment.getKind().equals(FieldIndex.Segment.Kind.ASCENDING)
+              ? getDescendingBound(segment, endAt)
+              : getAscendingBound(segment, endAt);
 
-      // Process all filters to find a value for the current field segment
-      for (FieldFilter fieldFilter : getFieldFiltersForPath(segment.getFieldPath())) {
-        Value filterValue = null;
-        boolean filterInclusive = true;
+      if (segmentBound.first == null) {
+        // No upper bound exists
+        return null;
+      }
 
-        switch (fieldFilter.getOperator()) {
-          case GREATER_THAN_OR_EQUAL:
-          case GREATER_THAN:
-            filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
-            filterInclusive = false;
-            break;
-          case EQUAL:
-          case IN:
-          case LESS_THAN_OR_EQUAL:
-            filterValue = fieldFilter.getValue();
-            break;
-          case LESS_THAN:
-            filterValue = fieldFilter.getValue();
-            filterInclusive = false;
-            break;
-          case NOT_EQUAL:
-          case NOT_IN:
-            filterValue = Values.MAX_VALUE;
-            break;
-          default:
-            // Remaining filters cannot be used as upper bounds.
-        }
+      values.add(segmentBound.first);
+      inclusive &= segmentBound.second;
+    }
 
-        if (min(segmentValue, filterValue) == filterValue) {
-          segmentValue = filterValue;
-          segmentInclusive = filterInclusive;
-        }
+    return new Bound(values, inclusive);
+  }
+
+  /**
+   * Returns the value for an ascending bound of `segment`.
+   *
+   * @param segment The segment to get the value for.
+   * @param bound A bound to restrict the index range.
+   * @return a Pair with a nullable Value and a boolean indicating whether the bound is inclusive
+   */
+  private Pair<Value, Boolean> getAscendingBound(
+      FieldIndex.Segment segment, @Nullable Bound bound) {
+    Value segmentValue = null;
+    boolean segmentInclusive = true;
+
+    // Process all filters to find a value for the current field segment
+    for (FieldFilter fieldFilter : getFieldFiltersForPath(segment.getFieldPath())) {
+      Value filterValue = null;
+      boolean filterInclusive = true;
+
+      switch (fieldFilter.getOperator()) {
+        case LESS_THAN:
+        case LESS_THAN_OR_EQUAL:
+          filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
+          break;
+        case EQUAL:
+        case IN:
+        case GREATER_THAN_OR_EQUAL:
+          filterValue = fieldFilter.getValue();
+          break;
+        case GREATER_THAN:
+          filterValue = fieldFilter.getValue();
+          filterInclusive = false;
+          break;
+        case NOT_EQUAL:
+        case NOT_IN:
+          filterValue = Values.MIN_VALUE;
+          break;
+        default:
+          // Remaining filters cannot be used as bound.
       }
 
-      // If there is an endAt bound, compare the values against the existing boundary to see
-      // if we can narrow the scope.
-      if (endAt != null) {
-        for (int i = 0; i < orderBys.size(); ++i) {
-          OrderBy orderBy = this.orderBys.get(i);
-          if (orderBy.getField().equals(segment.getFieldPath())) {
-            Value cursorValue = endAt.getPosition().get(i);
-            if (min(segmentValue, cursorValue) == cursorValue) {
-              segmentValue = cursorValue;
-              segmentInclusive = endAt.isInclusive();
-            }
-            break;
+      if (max(segmentValue, filterValue) == filterValue) {
+        segmentValue = filterValue;
+        segmentInclusive = filterInclusive;
+      }
+    }
+
+    // If there is an additional bound, compare the values against the existing range to see if we
+    // can narrow the scope.
+    if (bound != null) {
+      for (int i = 0; i < orderBys.size(); ++i) {
+        OrderBy orderBy = this.orderBys.get(i);
+        if (orderBy.getField().equals(segment.getFieldPath())) {
+          Value cursorValue = bound.getPosition().get(i);
+          if (max(segmentValue, cursorValue) == cursorValue) {
+            segmentValue = cursorValue;
+            segmentInclusive = bound.isInclusive();
           }
         }
       }
+    }
 
-      if (segmentValue == null) {
-        // No upper bound exists
-        return null;
+    return new Pair<>(segmentValue, segmentInclusive);
+  }
+
+  /**
+   * Returns the value for a descending bound of `segment`.
+   *
+   * @param segment The segment to get the value for.
+   * @param bound A bound to restrict the index range.
+   * @return a Pair with a nullable Value and a boolean indicating whether the bound is inclusive
+   */
+  private Pair<Value, Boolean> getDescendingBound(
+      FieldIndex.Segment segment, @Nullable Bound bound) {
+    Value segmentValue = null;
+    boolean segmentInclusive = true;
+
+    // Process all filters to find a value for the current field segment
+    for (FieldFilter fieldFilter : getFieldFiltersForPath(segment.getFieldPath())) {
+      Value filterValue = null;
+      boolean filterInclusive = true;
+
+      switch (fieldFilter.getOperator()) {
+        case GREATER_THAN_OR_EQUAL:
+        case GREATER_THAN:
+          filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
+          filterInclusive = false;
+          break;
+        case EQUAL:
+        case IN:
+        case LESS_THAN_OR_EQUAL:
+          filterValue = fieldFilter.getValue();
+          break;
+        case LESS_THAN:
+          filterValue = fieldFilter.getValue();
+          filterInclusive = false;
+          break;
+        case NOT_EQUAL:
+        case NOT_IN:
+          filterValue = Values.MAX_VALUE;
+          break;
+        default:
+          // Remaining filters cannot be used as bound.
       }
 
-      values.add(segmentValue);
-      inclusive &= segmentInclusive;
+      if (min(segmentValue, filterValue) == filterValue) {
+        segmentValue = filterValue;
+        segmentInclusive = filterInclusive;
+      }
     }
 
-    return new Bound(values, inclusive);
+    // If there is an additional bound, compare the values against the existing range to see if we
+    // can narrow the scope.
+    if (bound != null) {
+      for (int i = 0; i < orderBys.size(); ++i) {
+        OrderBy orderBy = this.orderBys.get(i);
+        if (orderBy.getField().equals(segment.getFieldPath())) {
+          Value cursorValue = bound.getPosition().get(i);
+          if (min(segmentValue, cursorValue) == cursorValue) {
+            segmentValue = cursorValue;
+            segmentInclusive = bound.isInclusive();
+          }
+        }
+      }
+    }
+
+    return new Pair<>(segmentValue, segmentInclusive);
   }
 
   public List<OrderBy> getOrderBy() {
     return this.orderBys;
   }
 
+  /** Returns the order of the document key component. */
+  public Direction getKeyOrder() {
+    return this.orderBys.get(this.orderBys.size() - 1).getDirection();
+  }
+
   /** Returns a canonical string representing this target. */
   public String getCanonicalId() {
     if (memoizedCanonicalId != null) {

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

@@ -24,7 +24,6 @@
 import com.google.firebase.firestore.model.ResourcePath;
 import java.util.Collection;
 import java.util.List;
-import java.util.Set;
 
 /**
  * Represents a set of indexes that are used to execute queries efficiently.
@@ -95,7 +94,7 @@ public interface IndexManager {
    * 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);
+  List<DocumentKey> getDocumentsMatchingTarget(Target target);
 
   /** Returns the next collection group to update. Returns {@code null} if no group exists. */
   @Nullable

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

@@ -29,7 +29,6 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 /** An in-memory implementation of IndexManager. */
 class MemoryIndexManager implements IndexManager {
@@ -70,7 +69,7 @@ public FieldIndex getFieldIndex(Target target) {
 
   @Override
   @Nullable
-  public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
+  public List<DocumentKey> getDocumentsMatchingTarget(Target target) {
     // Field indices are not supported with memory persistence.
     return null;
   }

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

@@ -28,8 +28,8 @@
 import com.google.firebase.firestore.model.SnapshotVersion;
 import com.google.firebase.firestore.util.Logger;
 import java.util.Collections;
+import java.util.List;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.Nullable;
 
 /**
@@ -110,7 +110,7 @@ public ImmutableSortedMap<DocumentKey, Document> getDocumentsMatchingQuery(
       return null;
     }
 
-    Set<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
+    List<DocumentKey> keys = indexManager.getDocumentsMatchingTarget(target);
     if (keys == null) {
       return null;
     }