Tighten the query bounds

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

@@ -14,6 +14,9 @@
 
 package com.google.firebase.firestore.core;
 
+import static com.google.firebase.firestore.model.Values.max;
+import static com.google.firebase.firestore.model.Values.min;
+
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.core.OrderBy.Direction;
 import com.google.firebase.firestore.model.DocumentKey;
@@ -121,25 +124,29 @@ public boolean hasLimit() {
    */
   public Bound getLowerBound(FieldIndex fieldIndex) {
     List<Value> values = new ArrayList<>();
-    boolean before = true;
+    boolean inclusive = true;
 
     // Go through all filters to find a value for the current field segment
     for (FieldIndex.Segment segment : fieldIndex) {
-      Value lowestValue = Values.NULL_VALUE;
+      Value segmentValue = Values.NULL_VALUE;
+      boolean segmentInclusive = true;
+
       for (Filter filter : filters) {
         if (filter.getField().equals(segment.getFieldPath())) {
           FieldFilter fieldFilter = (FieldFilter) filter;
+          Value filterValue = null;
+          boolean filterInclusive = true;
+
           switch (fieldFilter.getOperator()) {
             case LESS_THAN:
             case LESS_THAN_OR_EQUAL:
-              // TODO(indexing): Implement type clamping. Only field values with the same type
-              // should match the query.
+              filterValue = Values.getLowerBound(fieldFilter.getValue().getValueTypeCase());
               break;
             case NOT_EQUAL:
-              // These filters cannot be used as a lower bound. Skip.
+              filterValue = Values.NULL_VALUE;
               break;
             case NOT_IN:
-              lowestValue =
+              filterValue =
                   Value.newBuilder()
                       .setArrayValue(ArrayValue.newBuilder().addValues(Values.NULL_VALUE))
                       .build();
@@ -149,13 +156,18 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
             case ARRAY_CONTAINS_ANY:
             case ARRAY_CONTAINS:
             case GREATER_THAN_OR_EQUAL:
-              lowestValue = fieldFilter.getValue();
+              filterValue = fieldFilter.getValue();
               break;
             case GREATER_THAN:
-              lowestValue = fieldFilter.getValue();
-              before = false;
+              filterValue = fieldFilter.getValue();
+              filterInclusive = false;
               break;
           }
+
+          if (max(segmentValue, filterValue) == filterValue) {
+            segmentValue = filterValue;
+            segmentInclusive = filterInclusive;
+          }
         }
       }
 
@@ -166,20 +178,20 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
           OrderBy orderBy = this.orderBys.get(i);
           if (orderBy.getField().equals(segment.getFieldPath())) {
             Value cursorValue = startAt.getPosition().get(i);
-            if (Values.compare(lowestValue, cursorValue) <= 0) {
-              lowestValue = cursorValue;
-              // `before` is shared by all cursor values. If any cursor value is used, we set before
-              // to the cursor's value.
-              before = startAt.isBefore();
+            if (max(segmentValue, cursorValue) == cursorValue) {
+              segmentValue = cursorValue;
+              segmentInclusive = startAt.isBefore();
             }
             break;
           }
         }
       }
-      values.add(lowestValue);
+
+      values.add(segmentValue);
+      inclusive &= segmentInclusive;
     }
 
-    return new Bound(values, before);
+    return new Bound(values, inclusive);
   }
 
   /**
@@ -192,38 +204,46 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
    */
   public @Nullable Bound getUpperBound(FieldIndex fieldIndex) {
     List<Value> values = new ArrayList<>();
-    boolean before = false;
+    boolean inclusive = true;
 
     for (FieldIndex.Segment segment : fieldIndex) {
-      @Nullable Value largestValue = null;
+      @Nullable Value segmentValue = null;
+      boolean segmentInclusive = true;
 
       // Go through all filters to find a value for the current field segment
       for (Filter filter : filters) {
         if (filter.getField().equals(segment.getFieldPath())) {
           FieldFilter fieldFilter = (FieldFilter) filter;
+          Value filterValue = null;
+          boolean filterInclusive = true;
+
           switch (fieldFilter.getOperator()) {
             case NOT_IN:
             case NOT_EQUAL:
               // These filters cannot be used as an upper bound. Skip.
               break;
             case GREATER_THAN_OR_EQUAL:
             case GREATER_THAN:
-              // TODO(indexing): Implement type clamping. Only field values with the same type
-              // should match the query.
+              filterValue = Values.getUpperBound(fieldFilter.getValue().getValueTypeCase());
+              filterInclusive = false;
               break;
             case EQUAL:
             case IN:
             case ARRAY_CONTAINS_ANY:
             case ARRAY_CONTAINS:
             case LESS_THAN_OR_EQUAL:
-              largestValue = fieldFilter.getValue();
-              before = false;
+              filterValue = fieldFilter.getValue();
               break;
             case LESS_THAN:
-              largestValue = fieldFilter.getValue();
-              before = true;
+              filterValue = fieldFilter.getValue();
+              filterInclusive = false;
               break;
           }
+
+          if (min(segmentValue, filterValue) == filterValue) {
+            segmentValue = filterValue;
+            segmentInclusive = filterInclusive;
+          }
         }
       }
 
@@ -234,28 +254,29 @@ public Bound getLowerBound(FieldIndex fieldIndex) {
           OrderBy orderBy = this.orderBys.get(i);
           if (orderBy.getField().equals(segment.getFieldPath())) {
             Value cursorValue = endAt.getPosition().get(i);
-            if (largestValue == null || Values.compare(largestValue, cursorValue) > 0) {
-              largestValue = cursorValue;
-              before = endAt.isBefore();
+            if (min(segmentValue, cursorValue) == cursorValue) {
+              segmentValue = cursorValue;
+              segmentInclusive = !endAt.isBefore();
             }
             break;
           }
         }
       }
 
-      if (largestValue == null) {
+      if (segmentValue == null) {
         // No upper bound exists
         return null;
       }
 
-      values.add(largestValue);
+      values.add(segmentValue);
+      inclusive &= segmentInclusive;
     }
 
     if (values.isEmpty()) {
       return null;
     }
 
-    return new Bound(values, before);
+    return new Bound(values, !inclusive);
   }
 
   public List<OrderBy> getOrderBy() {

firebase-firestore/src/main/java/com/google/firebase/firestore/index/FirestoreIndexValueWriter.java

@@ -34,7 +34,7 @@ public class FirestoreIndexValueWriter {
   public static final int INDEX_TYPE_BOOLEAN = 10;
   public static final int INDEX_TYPE_NAN = 13;
   public static final int INDEX_TYPE_NUMBER = 15;
-  public static final int INDEX_TYPE_TIMESTAMP = 10;
+  public static final int INDEX_TYPE_TIMESTAMP = 20;
   public static final int INDEX_TYPE_STRING = 25;
   public static final int INDEX_TYPE_BLOB = 30;
   public static final int INDEX_TYPE_REFERENCE = 37;

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

@@ -206,6 +206,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
 
       // TODO(indexing): To avoid reading the same documents multiple times, we should ideally only
       // send one query that combines all clauses.
+      // TODO(indexing): Add limit handling
       for (int i = 0; i < lowerBoundValues.size(); ++i) {
         db.query(
                 String.format(

firebase-firestore/src/main/java/com/google/firebase/firestore/model/DatabaseId.java

@@ -21,6 +21,7 @@
 /** Represents a particular database in Firestore */
 public final class DatabaseId implements Comparable<DatabaseId> {
   public static final String DEFAULT_DATABASE_ID = "(default)";
+  public static final DatabaseId EMPTY = DatabaseId.forDatabase("", "");
 
   public static DatabaseId forProject(String projectId) {
     return forDatabase(projectId, DEFAULT_DATABASE_ID);

firebase-firestore/src/main/java/com/google/firebase/firestore/model/Values.java

@@ -25,6 +25,7 @@
 import com.google.firestore.v1.ArrayValueOrBuilder;
 import com.google.firestore.v1.MapValue;
 import com.google.firestore.v1.Value;
+import com.google.protobuf.ByteString;
 import com.google.protobuf.NullValue;
 import com.google.protobuf.Timestamp;
 import com.google.type.LatLng;
@@ -211,6 +212,30 @@ public static int compare(Value left, Value right) {
     }
   }
 
+  public static @Nullable Value max(@Nullable Value left, @Nullable Value right) {
+    if (left == null && right == null) {
+      return null;
+    } else if (left == null) {
+      return right;
+    } else if (right == null) {
+      return left;
+    } else {
+      return compare(left, right) > 0 ? left : right;
+    }
+  }
+
+  public static @Nullable Value min(@Nullable Value left, @Nullable Value right) {
+    if (left == null && right == null) {
+      return null;
+    } else if (left == null) {
+      return right;
+    } else if (right == null) {
+      return left;
+    } else {
+      return compare(left, right) < 0 ? left : right;
+    }
+  }
+
   private static int compareNumbers(Value left, Value right) {
     if (left.getValueTypeCase() == Value.ValueTypeCase.DOUBLE_VALUE) {
       double leftDouble = left.getDoubleValue();
@@ -432,4 +457,69 @@ public static Value refValue(DatabaseId databaseId, DocumentKey key) {
             .build();
     return value;
   }
+
+  /** Returns the lowest value for the given value type (inclusive). */
+  public static Value getLowerBound(Value.ValueTypeCase valueTypeCase) {
+    switch (valueTypeCase) {
+      case NULL_VALUE:
+        return Values.NULL_VALUE;
+      case BOOLEAN_VALUE:
+        return Value.newBuilder().setBooleanValue(false).build();
+      case INTEGER_VALUE:
+      case DOUBLE_VALUE:
+        return Value.newBuilder().setDoubleValue(Double.NaN).build();
+      case TIMESTAMP_VALUE:
+        return Value.newBuilder()
+            .setTimestampValue(Timestamp.newBuilder().setSeconds(Long.MIN_VALUE))
+            .build();
+      case STRING_VALUE:
+        return Value.newBuilder().setStringValue("").build();
+      case BYTES_VALUE:
+        return Value.newBuilder().setBytesValue(ByteString.EMPTY).build();
+      case REFERENCE_VALUE:
+        return refValue(DatabaseId.EMPTY, DocumentKey.empty());
+      case GEO_POINT_VALUE:
+        return Value.newBuilder()
+            .setGeoPointValue(LatLng.newBuilder().setLatitude(-90.0).setLongitude(-180.0))
+            .build();
+      case ARRAY_VALUE:
+        return Value.newBuilder().setArrayValue(ArrayValue.getDefaultInstance()).build();
+      case MAP_VALUE:
+        return Value.newBuilder().setMapValue(MapValue.getDefaultInstance()).build();
+      default:
+        throw new IllegalArgumentException("Unknown value type: " + valueTypeCase);
+    }
+  }
+
+  /**
+   * Returns the largest value for the given value type (exclusive). Returns {@code null} for maps.
+   */
+  public static @Nullable Value getUpperBound(Value.ValueTypeCase valueTypeCase) {
+    switch (valueTypeCase) {
+      case NULL_VALUE:
+        return getLowerBound(Value.ValueTypeCase.BOOLEAN_VALUE);
+      case BOOLEAN_VALUE:
+        return getLowerBound(Value.ValueTypeCase.INTEGER_VALUE);
+      case INTEGER_VALUE:
+      case DOUBLE_VALUE:
+        return getLowerBound(Value.ValueTypeCase.TIMESTAMP_VALUE);
+      case TIMESTAMP_VALUE:
+        return getLowerBound(Value.ValueTypeCase.STRING_VALUE);
+      case STRING_VALUE:
+        return getLowerBound(Value.ValueTypeCase.BYTES_VALUE);
+      case BYTES_VALUE:
+        return getLowerBound(Value.ValueTypeCase.REFERENCE_VALUE);
+      case REFERENCE_VALUE:
+        return getLowerBound(Value.ValueTypeCase.GEO_POINT_VALUE);
+      case GEO_POINT_VALUE:
+        return getLowerBound(Value.ValueTypeCase.ARRAY_VALUE);
+      case ARRAY_VALUE:
+        return getLowerBound(Value.ValueTypeCase.MAP_VALUE);
+      case MAP_VALUE:
+        // There is no type that sorts higher than a map.
+        return null;
+      default:
+        throw new IllegalArgumentException("Unknown value type: " + valueTypeCase);
+    }
+  }
 }