WIP

Author: schmidt-sebastian

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

@@ -24,18 +24,25 @@
 public class OrderBy {
   /** The direction of the ordering */
   public enum Direction {
-    ASCENDING(1),
-    DESCENDING(-1);
+    ASCENDING(1, "asc"),
+    DESCENDING(-1, "desc");
 
     private final int comparisonModifier;
+    private final String shorthand;
 
-    Direction(int comparisonModifier) {
+    Direction(int comparisonModifier, String canonicalString) {
       this.comparisonModifier = comparisonModifier;
+      this.shorthand = canonicalString;
     }
 
     int getComparisonModifier() {
       return comparisonModifier;
     }
+
+    /** Returns "asc" for ascending or "desc" for descending. */
+    public String canonicalString() {
+      return shorthand;
+    }
   }
 
   public static OrderBy getInstance(Direction direction, FieldPath path) {

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

@@ -18,7 +18,6 @@
 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;
 import com.google.firebase.firestore.model.FieldIndex;
 import com.google.firebase.firestore.model.ResourcePath;
@@ -312,7 +311,7 @@ public String getCanonicalId() {
     builder.append("|ob:");
     for (OrderBy orderBy : getOrderBy()) {
       builder.append(orderBy.getField().canonicalString());
-      builder.append(orderBy.getDirection().equals(Direction.ASCENDING) ? "asc" : "desc");
+      builder.append(orderBy.getDirection().canonicalString());
     }
 
     // Add limit.

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

@@ -22,7 +22,6 @@
 import com.google.firebase.firestore.core.Bound;
 import com.google.firebase.firestore.core.FieldFilter;
 import com.google.firebase.firestore.core.Filter;
-import com.google.firebase.firestore.core.OrderBy;
 import com.google.firebase.firestore.core.Target;
 import com.google.firebase.firestore.index.FirestoreIndexValueWriter;
 import com.google.firebase.firestore.index.IndexByteEncoder;
@@ -241,28 +240,21 @@ private SQLitePersistence.Query generateQuery(
       String lowerBoundOp,
       @Nullable Object[] upperBounds,
       @Nullable String upperBoundOp) {
-    String fields;
     String orderBy;
     String limit;
 
     if (target.getLimit() != -1) {
-      fields = "document_name, index_value";
-      orderBy =
-              "ORDER BY index_value "
-                      + (target.getFirstOrderBy().getDirection().equals(OrderBy.Direction.ASCENDING)
-                      ? "ASC"
-                      : "DESC");
+      String direction = target.getFirstOrderBy().getDirection().canonicalString();
+      orderBy = "ORDER BY index_value " + direction + ", document_name " + direction;
       limit = "LIMIT " + target.getLimit();
     } else {
-      fields = "document_name";
       orderBy = "";
       limit = "";
     }
 
     StringBuilder statement = new StringBuilder();
-    statement.append("SELECT ");
-    statement.append(fields);
-    statement.append(" FROM index_entries WHERE index_id = ? AND index_value ");
+    statement.append(
+        "SELECT document_name, index_value FROM index_entries WHERE index_id = ? AND index_value ");
     statement.append(lowerBoundOp);
     statement.append(" ? ");
     if (upperBounds != null) {
@@ -272,11 +264,9 @@ private SQLitePersistence.Query generateQuery(
     }
 
     int statementCount = lowerBounds.length * (upperBounds == null ? 1 : upperBounds.length);
-    String sql = repeatSequence(statement, statementCount, " UNION ");
-
-    if (!limit.isEmpty()) {
-      sql = String.format("SELECT document_name FROM (%s) %s %s", sql, orderBy, limit);
-    }
+    String sql =
+        String.format(
+            "%s %s %s", repeatSequence(statement, statementCount, " UNION "), orderBy, limit);
 
     Object[] bingArgs = new Object[statementCount * (upperBounds == null ? 2 : 3)];
     int i = 0;

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

@@ -268,6 +268,34 @@ public void testCollectionGroup() {
     verifyResults(query, "coll1/doc1", "coll2/doc2/coll1/doc1");
   }
 
+  @Test
+  public void testLimitFilter() {
+    indexManager.addFieldIndex(
+        new FieldIndex("coll").withAddedField(field("value"), FieldIndex.Segment.Kind.ORDERED));
+    addDoc("coll/doc1", map("value", 1));
+    addDoc("coll/doc2", map("value", 1));
+    addDoc("coll/doc3", map("value", 1));
+    Query query = query("coll").filter(filter("value", "==", 1)).limitToFirst(2);
+    verifyResults(query, "coll/doc1", "coll/doc2");
+  }
+
+  @Test
+  public void testLimitAppliesOrdering() {
+    indexManager.addFieldIndex(
+        new FieldIndex("coll")
+            .withAddedField(field("value"), FieldIndex.Segment.Kind.ORDERED)
+            .withAddedField(field("value"), FieldIndex.Segment.Kind.CONTAINS));
+    addDoc("coll/doc1", map("value", Arrays.asList(1, "foo")));
+    addDoc("coll/doc2", map("value", Arrays.asList(3, "foo")));
+    addDoc("coll/doc3", map("value", Arrays.asList(2, "foo")));
+    Query query =
+        query("coll")
+            .filter(filter("value", "array-contains", "foo"))
+            .orderBy(orderBy("value"))
+            .limitToFirst(2);
+    verifyResults(query, "coll/doc1", "coll/doc3");
+  }
+
   @Test
   public void testUpdateTime() {
     indexManager.addFieldIndex(

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

@@ -637,7 +637,7 @@ public void createsIndexingTables() {
 
   private SQLiteRemoteDocumentCache createRemoteDocumentCache() {
     SQLitePersistence persistence =
-        new SQLitePersistence(serializer,  databaseId, LruGarbageCollector.Params.Default(), opener);
+        new SQLitePersistence(serializer, databaseId, LruGarbageCollector.Params.Default(), opener);
     persistence.start();
     return new SQLiteRemoteDocumentCache(persistence, serializer);
   }