Merge branch 'master' into davidmotson.m132_mergeback

Author: davidmotson

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

@@ -22,8 +22,10 @@
 import com.google.firebase.firestore.core.OrderBy;
 import com.google.firebase.firestore.core.Target;
 import java.util.ArrayList;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Set;
 
 /**
  * A light query planner for Firestore.
@@ -144,14 +146,14 @@ public boolean servedByIndex(FieldIndex index) {
     List<FieldIndex.Segment> segments = index.getDirectionalSegments();
     int segmentIndex = 0;
 
+    Set<String> equalitySegments = new HashSet<>();
     // Process all equalities first. Equalities can appear out of order.
     for (; segmentIndex < segments.size(); ++segmentIndex) {
       // We attempt to greedily match all segments to equality filters. If a filter matches an
-      // index segment, we can mark the segment as used. Since it is not possible to use the same
-      // field path in both an equality and inequality/oderBy clause, we do not have to consider the
-      // possibility that a matching equality segment should instead be used to map to an inequality
-      // filter or orderBy clause.
-      if (!hasMatchingEqualityFilter(segments.get(segmentIndex))) {
+      // index segment, we can mark the segment as used.
+      if (hasMatchingEqualityFilter(segments.get(segmentIndex))) {
+        equalitySegments.add(segments.get(segmentIndex).getFieldPath().canonicalString());
+      } else {
         // If we cannot find a matching filter, we need to verify whether the remaining segments map
         // to the target's inequality and its orderBy clauses.
         break;
@@ -165,13 +167,17 @@ public boolean servedByIndex(FieldIndex index) {
       return true;
     }
 
-    // If there is an inequality filter, the next segment must match both the filter and the first
-    // orderBy clause.
     if (inequalityFilter != null) {
-      FieldIndex.Segment segment = segments.get(segmentIndex);
-      if (!matchesFilter(inequalityFilter, segment) || !matchesOrderBy(orderBys.next(), segment)) {
-        return false;
+      // If there is an inequality filter and the field was not in one of the equality filters
+      // above, the next segment must match both the filter and the first orderBy clause.
+      if (!equalitySegments.contains(inequalityFilter.getField().canonicalString())) {
+        FieldIndex.Segment segment = segments.get(segmentIndex);
+        if (!matchesFilter(inequalityFilter, segment)
+            || !matchesOrderBy(orderBys.next(), segment)) {
+          return false;
+        }
       }
+
       ++segmentIndex;
     }
 

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -654,14 +654,11 @@ StructuredAggregationQuery encodeStructuredAggregationQuery(
     int aliasID = 1;
     for (AggregateField aggregateField : aggregateFields) {
       // The code block below is used to deduplicate the same aggregate fields.
-      // If two aggregateFields are identical, their aliases would be the same.
-      // Therefore, when adding duplicated alias into uniqueFields, the size of uniqueFields
-      // won't increase, and we can skip this aggregateField processing.
-      final int count = uniqueFields.size();
-      uniqueFields.add(aggregateField.getAlias());
-      if (count == uniqueFields.size()) {
+      if (uniqueFields.contains(aggregateField.getAlias())) {
         continue;
       }
+      uniqueFields.add(aggregateField.getAlias());
+
       String serverAlias = "aggregate_" + aliasID++;
       aliasMap.put(serverAlias, aggregateField.getAlias());
 

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

@@ -624,6 +624,47 @@ public void testCursorsDoNoExpandResultSet() {
     verifyResults(query, "coll/val2");
   }
 
+  @Test
+  public void testFiltersOnTheSameField() {
+    indexManager.addFieldIndex(fieldIndex("coll", "a", Kind.ASCENDING));
+    indexManager.addFieldIndex(fieldIndex("coll", "a", Kind.ASCENDING, "b", Kind.ASCENDING));
+
+    addDoc("coll/val1", map("a", 1, "b", 1));
+    addDoc("coll/val2", map("a", 2, "b", 2));
+    addDoc("coll/val3", map("a", 3, "b", 3));
+    addDoc("coll/val4", map("a", 4, "b", 4));
+
+    Query query = query("coll").filter(filter("a", ">", 1)).filter(filter("a", "==", 2));
+    verifyResults(query, "coll/val2");
+
+    query = query("coll").filter(filter("a", "<=", 1)).filter(filter("a", "==", 2));
+    verifyResults(query);
+
+    query =
+        query("coll")
+            .filter(filter("a", ">", 1))
+            .filter(filter("a", "==", 2))
+            .orderBy(orderBy("a"))
+            .orderBy(orderBy(DocumentKey.KEY_FIELD_NAME));
+    verifyResults(query, "coll/val2");
+
+    query =
+        query("coll")
+            .filter(filter("a", ">", 1))
+            .filter(filter("a", "==", 2))
+            .orderBy(orderBy("a"))
+            .orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc"));
+    verifyResults(query, "coll/val2");
+
+    query =
+        query("coll")
+            .filter(filter("a", ">", 1))
+            .filter(filter("a", "==", 3))
+            .orderBy(orderBy("a"))
+            .orderBy(orderBy("b"));
+    verifyResults(query, "coll/val3");
+  }
+
   @Test
   public void testAdvancedQueries() {
     // This test compares local query results with those received from the Java Server SDK.

firebase-firestore/src/test/java/com/google/firebase/firestore/model/TargetIndexMatcherTest.java

@@ -565,6 +565,60 @@ public void withInAndOrderBySameField() {
     validateServesTarget(q, "a", FieldIndex.Segment.Kind.ASCENDING);
   }
 
+  @Test
+  public void withEqualityAndInequalityOnTheSameField() {
+    validateServesTarget(
+        query("collId").filter(filter("a", ">=", 5)).filter(filter("a", "==", 0)),
+        "a",
+        FieldIndex.Segment.Kind.ASCENDING);
+
+    validateServesTarget(
+        query("collId")
+            .filter(filter("a", ">=", 5))
+            .filter(filter("a", "==", 0))
+            .orderBy(orderBy("a")),
+        "a",
+        FieldIndex.Segment.Kind.ASCENDING);
+
+    validateServesTarget(
+        query("collId")
+            .filter(filter("a", ">=", 5))
+            .filter(filter("a", "==", 0))
+            .orderBy(orderBy("a"))
+            .orderBy(orderBy(DocumentKey.KEY_FIELD_NAME)),
+        "a",
+        FieldIndex.Segment.Kind.ASCENDING);
+
+    validateServesTarget(
+        query("collId")
+            .filter(filter("a", ">=", 5))
+            .filter(filter("a", "==", 0))
+            .orderBy(orderBy("a"))
+            .orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc")),
+        "a",
+        FieldIndex.Segment.Kind.ASCENDING);
+
+    validateServesTarget(
+        query("collId")
+            .filter(filter("a", ">=", 5))
+            .filter(filter("a", "==", 0))
+            .orderBy(orderBy("a", "asc"))
+            .orderBy(orderBy("b", "asc")),
+        "a",
+        FieldIndex.Segment.Kind.ASCENDING,
+        "b",
+        FieldIndex.Segment.Kind.ASCENDING);
+
+    validateServesTarget(
+        query("collId")
+            .filter(filter("a", ">=", 5))
+            .filter(filter("a", "==", 0))
+            .orderBy(orderBy("a", "desc"))
+            .orderBy(orderBy(DocumentKey.KEY_FIELD_NAME, "desc")),
+        "a",
+        FieldIndex.Segment.Kind.DESCENDING);
+  }
+
   private void validateServesTarget(
       Query query, String field, FieldIndex.Segment.Kind kind, Object... fieldsAndKind) {
     FieldIndex expectedIndex = fieldIndex("collId", field, kind, fieldsAndKind);