Run one instead of n queries

Author: schmidt-sebastian

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

@@ -17,6 +17,7 @@
 import static com.google.firebase.firestore.util.Assert.fail;
 import static com.google.firebase.firestore.util.Assert.hardAssert;
 
+import android.text.TextUtils;
 import androidx.annotation.Nullable;
 import com.google.firebase.firestore.core.Bound;
 import com.google.firebase.firestore.core.FieldFilter;
@@ -134,8 +135,8 @@ public void addIndexEntries(Document document) {
                       fieldIndex);
                 }
 
-                List<byte[]> encodeValues = encodeDocumentValues(fieldIndex, values);
-                for (byte[] encoded : encodeValues) {
+                Object[] encodeValues = encodeDocumentValues(fieldIndex, values);
+                for (Object encoded : encodeValues) {
                   // TODO(indexing): Handle different values for different users
                   db.execute(
                       "INSERT OR IGNORE INTO index_entries ("
@@ -176,8 +177,6 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
     if (fieldIndex == null) return null;
 
     Bound lowerBound = target.getLowerBound(fieldIndex);
-    String lowerBoundOp = lowerBound.isBefore() ? ">=" : ">"; // `startAt()` versus `startAfter()`
-
     @Nullable Bound upperBound = target.getUpperBound(fieldIndex);
 
     if (Logger.isDebugEnabled()) {
@@ -191,49 +190,59 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
     }
 
     Set<DocumentKey> result = new HashSet<>();
+    BindArgs bindArgs;
 
+    Object[] lowerBoundValues = encodeTargetValues(fieldIndex, target, lowerBound.getPosition());
+    String lowerBoundOp = lowerBound.isBefore() ? ">=" : ">"; // `startAt()` versus `startAfter()`
     if (upperBound != null) {
-      List<byte[]> lowerBoundValues =
-          encodeTargetValues(fieldIndex, target, lowerBound.getPosition());
-      List<byte[]> upperBoundValues =
-          encodeTargetValues(fieldIndex, target, upperBound.getPosition());
-
-      hardAssert(
-          lowerBoundValues.size() == upperBoundValues.size(),
-          "Expected upper and lower bound size to match");
-
+      Object[] upperBoundValues = encodeTargetValues(fieldIndex, target, upperBound.getPosition());
       String upperBoundOp = upperBound.isBefore() ? "<" : "<="; // `endBefore()` versus `endAt()`
-
-      // 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(
-                    "SELECT document_name from index_entries WHERE index_id = ? AND index_value %s ? AND index_value %s ?",
-                    lowerBoundOp, upperBoundOp))
-            .binding(fieldIndex.getIndexId(), lowerBoundValues.get(i), upperBoundValues.get(i))
-            .forEach(
-                row -> result.add(DocumentKey.fromPath(ResourcePath.fromString(row.getString(0)))));
-      }
+      bindArgs = generateBindArgs(lowerBoundValues, lowerBoundOp, upperBoundValues, upperBoundOp);
     } else {
-      List<byte[]> lowerBoundValues =
-          encodeTargetValues(fieldIndex, target, lowerBound.getPosition());
-      for (byte[] lowerBoundValue : lowerBoundValues) {
-        db.query(
-                String.format(
-                    "SELECT document_name from index_entries WHERE index_id = ? AND index_value %s  ?",
-                    lowerBoundOp))
-            .binding(fieldIndex.getIndexId(), lowerBoundValue)
-            .forEach(
-                row -> result.add(DocumentKey.fromPath(ResourcePath.fromString(row.getString(0)))));
-      }
+      bindArgs = generateBindArgs(lowerBoundValues, lowerBoundOp);
     }
 
+    db.query(
+            String.format(
+                "SELECT document_name from index_entries WHERE index_id = ? AND (%s)",
+                bindArgs.sql))
+        .binding(fieldIndex.getIndexId(), bindArgs.bindArgs)
+        .forEach(
+            row -> result.add(DocumentKey.fromPath(ResourcePath.fromString(row.getString(0)))));
+    // TODO(indexing): Add limit handling
+
     Logger.debug(TAG, "Index scan returned %s documents", result.size());
     return result;
   }
 
+  /** Returns a SQL filter that concatenates all {@code value} into a disjunction. */
+  private BindArgs generateBindArgs(Object[] values, String op) {
+    String[] filters = new String[values.length];
+    for (int i = 0; i < values.length; ++i) {
+      filters[i] = String.format("index_value %s ?", op);
+    }
+    return new BindArgs(TextUtils.join(" OR ", filters), values);
+  }
+
+  /**
+   * Returns a SQL filter that combines each element from the left list with each element from the
+   * right list and returns all combinations (e.g. `(left1 AND right1) OR (left1 AND right2) ...`).
+   */
+  private BindArgs generateBindArgs(Object[] left, String leftOp, Object[] right, String rightOp) {
+    String[] filters = new String[left.length * right.length];
+    Object[] bingArgs = new Object[left.length * right.length * 2];
+    int i = 0;
+    for (Object value1 : left) {
+      for (Object value2 : right) {
+        filters[i] = String.format("index_value %s ? AND index_value %s ?", leftOp, rightOp);
+        bingArgs[i * 2] = value1;
+        bingArgs[i * 2 + 1] = value2;
+        ++i;
+      }
+    }
+    return new BindArgs(TextUtils.join(" OR ", filters), bingArgs);
+  }
+
   /**
    * Returns an index that can be used to serve the provided target. Returns {@code null} if no
    * index is configured.
@@ -278,7 +287,7 @@ public Set<DocumentKey> getDocumentsMatchingTarget(Target target) {
    * Encodes the given field values according to the specification in {@code fieldIndex}. For
    * CONTAINS indices, a list of possible values is returned.
    */
-  private List<byte[]> encodeDocumentValues(FieldIndex fieldIndex, List<Value> values) {
+  private Object[] encodeDocumentValues(FieldIndex fieldIndex, List<Value> values) {
     List<IndexByteEncoder> encoders = new ArrayList<>();
     encoders.add(new IndexByteEncoder());
     for (int i = 0; i < fieldIndex.segmentCount(); ++i) {
@@ -302,8 +311,7 @@ private List<byte[]> encodeDocumentValues(FieldIndex fieldIndex, List<Value> val
    * Encodes the given field values according to the specification in {@code target}. For IN and
    * ArrayContainsAny queries, a list of possible values is returned.
    */
-  private List<byte[]> encodeTargetValues(
-      FieldIndex fieldIndex, Target target, List<Value> values) {
+  private Object[] encodeTargetValues(FieldIndex fieldIndex, Target target, List<Value> values) {
     List<IndexByteEncoder> encoders = new ArrayList<>();
     encoders.add(new IndexByteEncoder());
     for (int i = 0; i < fieldIndex.segmentCount(); ++i) {
@@ -321,10 +329,10 @@ private List<byte[]> encodeTargetValues(
   }
 
   /** Returns the byte representation for all encoders. */
-  private List<byte[]> getEncodedBytes(List<IndexByteEncoder> encoders) {
-    List<byte[]> result = new ArrayList<>();
-    for (IndexByteEncoder encoder : encoders) {
-      result.add(encoder.getEncodedBytes());
+  private Object[] getEncodedBytes(List<IndexByteEncoder> encoders) {
+    Object[] result = new Object[encoders.size()];
+    for (int i = 0; i < encoders.size(); ++i) {
+      result[i] = encoders.get(i).getEncodedBytes();
     }
     return result;
   }
@@ -368,4 +376,15 @@ private boolean isMultiValueFilter(Target target, FieldPath fieldPath) {
   private byte[] encodeFieldIndex(FieldIndex fieldIndex) {
     return serializer.encodeFieldIndex(fieldIndex).toByteArray();
   }
+
+  /** Stores a SQL statement of filters and their corresponding bind arguments. */
+  static class BindArgs {
+    final String sql;
+    final Object[] bindArgs;
+
+    BindArgs(String sql, Object[] bindArgs) {
+      this.sql = sql;
+      this.bindArgs = bindArgs;
+    }
+  }
 }

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

@@ -374,7 +374,7 @@ SQLiteStatement prepare(String sql) {
    */
   int execute(SQLiteStatement statement, Object... args) {
     statement.clearBindings();
-    bind(statement, args);
+    bind(statement, 0, args);
     return statement.executeUpdateDelete();
   }
 
@@ -442,7 +442,7 @@ static class Query {
      *
      * @return this Query object, for chaining.
      */
-    Query binding(Object... args) {
+    Query binding(Object arg, Object... args) {
       // This is gross, but the best way to preserve both the readability of the caller (since
       // values don't have be arbitrarily converted to Strings) and allows BLOBs to be used as
       // bind arguments.
@@ -458,7 +458,8 @@ Query binding(Object... args) {
 
       cursorFactory =
           (db1, masterQuery, editTable, query) -> {
-            bind(query, args);
+            bind(query, /* offset= */ 0, arg);
+            bind(query, /* offset= */ 1, args);
             return new SQLiteCursor(masterQuery, editTable, query);
           };
       return this;
@@ -684,8 +685,14 @@ int getSubqueriesPerformed() {
     }
   }
 
+  /** Binds the given arguments to the given SQLite statement or query. */
+  private static void bind(SQLiteProgram program, int offset, Object[] bindArgs) {
+    for (int i = 0; i < bindArgs.length; i++) {
+      bind(program, i + offset, bindArgs[i]);
+    }
+  }
   /**
-   * Binds the given arguments to the given SQLite statement or query.
+   * Binds a single argument to the given SQLite statement or query.
    *
    * <p>This method helps work around the fact that all of the querying methods on SQLiteDatabase
    * take an array of strings for bind arguments. Most values can be straightforwardly converted to
@@ -695,24 +702,22 @@ int getSubqueriesPerformed() {
    * This method bridges the gap by examining the types of the bindArgs and calling to the
    * appropriate bind method on the program.
    */
-  private static void bind(SQLiteProgram program, Object[] bindArgs) {
-    for (int i = 0; i < bindArgs.length; i++) {
-      Object arg = bindArgs[i];
-      if (arg == null) {
-        program.bindNull(i + 1);
-      } else if (arg instanceof String) {
-        program.bindString(i + 1, (String) arg);
-      } else if (arg instanceof Integer) {
-        program.bindLong(i + 1, (Integer) arg);
-      } else if (arg instanceof Long) {
-        program.bindLong(i + 1, (Long) arg);
-      } else if (arg instanceof Double) {
-        program.bindDouble(i + 1, (Double) arg);
-      } else if (arg instanceof byte[]) {
-        program.bindBlob(i + 1, (byte[]) arg);
-      } else {
-        throw fail("Unknown argument %s of type %s", arg, arg.getClass());
-      }
+  private static void bind(SQLiteProgram program, int pos, Object bindArg) {
+    Object arg = bindArg;
+    if (arg == null) {
+      program.bindNull(pos + 1);
+    } else if (arg instanceof String) {
+      program.bindString(pos + 1, (String) arg);
+    } else if (arg instanceof Integer) {
+      program.bindLong(pos + 1, (Integer) arg);
+    } else if (arg instanceof Long) {
+      program.bindLong(pos + 1, (Long) arg);
+    } else if (arg instanceof Double) {
+      program.bindDouble(pos + 1, (Double) arg);
+    } else if (arg instanceof byte[]) {
+      program.bindBlob(pos + 1, (byte[]) arg);
+    } else {
+      throw fail("Unknown argument %s of type %s", arg, arg.getClass());
     }
   }
 }