Bug 1588329 - Introduce `mozIStorageAsyncConnection::variableLimit`. r=mak

This is a wrapper around the `sqlite3_limit` interface that returns the
binding parameter limit. Adding this getter lets us clean up the inline
`SQLITE_MAX_VARIABLE_NUMBER` constants scattered around Places.

Differential Revision: https://phabricator.services.mozilla.com/D49071

--HG--
extra : moz-landing-system : lando

dom/cache/Connection.cpp

@@ -225,6 +225,11 @@ Connection::SetDefaultTransactionType(int32_t aType) {
   return mBase->SetDefaultTransactionType(aType);
 }
 
+NS_IMETHODIMP
+Connection::GetVariableLimit(int32_t* aResultOut) {
+  return mBase->GetVariableLimit(aResultOut);
+}
+
 NS_IMETHODIMP
 Connection::BeginTransaction() { return mBase->BeginTransaction(); }
 

storage/mozIStorageAsyncConnection.idl

@@ -40,6 +40,40 @@ interface mozIStorageAsyncConnection : nsISupports {
    */
   attribute int32_t defaultTransactionType;
 
+  /**
+   * The maximum number of bound parameters for statements executed on this
+   * connection. If your statement has more params than this limit, you'll
+   * need to chunk them into multiple statements. See `PlacesUtils.chunkArray`
+   * and its callers in Places for examples of how to do this, or read on for
+   * an overview.
+   *
+   * Keep in mind that the variable limit is for the _total_ number of
+   * parameters, including ones bound by name (using the `:VVV`, `@VVV`, or
+   * `?VVV` syntax) and index (`?` and `?NNN`).
+   *
+   * This means, when chunking:
+   *
+   * - If you're binding 1 param per 1 value per chunk (for example, if you
+   *   have a list of GUIDs and a clause like `WHERE guid IN (?, ?, ?, ...)`,
+   *   your chunk length is just `variableLimit`.
+   * - If you're binding 1 param per 1 value per chunk, but using that
+   *   param in multiple positions in the query (for example, `WHERE url_hash
+   *   IN (hash(?1), hash(?2), ...) AND url IN (?1, ?2, ...)`), you can use the
+   *   `?NNN` syntax with a chunk length of `variableLimit`.
+   * - If you're binding N params per 1 value per chunk (for example, if you
+   *   have a list of items with GUIDs and parent GUIDs, and you want to bind
+   *   both), your chunk length is `variableLimit / N`, since you're binding
+   *   two params for each element.
+   * - If you're binding K params per L values per chunk, plus M fixed ones
+   *   (for example, `WHERE parentGuid = :parentGuid AND guid IN (?, ?, ...)`),
+   *   your chunk length is `variableLimit - M`, to ensure there's space for the
+   *   fixed variables.
+   *
+   * If you bind more params than this limit, `create{Async}Statement` will
+   * fail with a "too many SQL variables" error.
+   */
+  readonly attribute int32_t variableLimit;
+
   /**
    * Close this database connection, allowing all pending statements
    * to complete first.

storage/mozStorageConnection.cpp

@@ -1942,6 +1942,19 @@ Connection::SetDefaultTransactionType(int32_t aType) {
   return NS_OK;
 }
 
+NS_IMETHODIMP
+Connection::GetVariableLimit(int32_t* _limit) {
+  if (!connectionReady()) {
+    return NS_ERROR_NOT_INITIALIZED;
+  }
+  int limit = ::sqlite3_limit(mDBConn, SQLITE_LIMIT_VARIABLE_NUMBER, -1);
+  if (limit < 0) {
+    return NS_ERROR_UNEXPECTED;
+  }
+  *_limit = limit;
+  return NS_OK;
+}
+
 NS_IMETHODIMP
 Connection::BeginTransaction() {
   if (!connectionReady()) {

storage/rust/src/lib.rs

@@ -26,9 +26,9 @@
 
 #![allow(non_snake_case)]
 
-use std::{borrow::Cow, error, fmt, ops::Deref, result};
+use std::{borrow::Cow, convert::TryFrom, error, fmt, ops::Deref, result};
 
-use nserror::{nsresult, NS_ERROR_NO_INTERFACE};
+use nserror::{nsresult, NS_ERROR_NO_INTERFACE, NS_ERROR_UNEXPECTED};
 use nsstring::nsCString;
 use storage_variant::VariantType;
 use xpcom::{
@@ -68,6 +68,17 @@ impl Conn {
         &self.handle
     }
 
+    /// Returns the maximum number of bound parameters for statements executed
+    /// on this connection.
+    pub fn variable_limit(&self) -> Result<usize> {
+        let mut limit = 0i32;
+        let rv = unsafe { self.handle.GetVariableLimit(&mut limit) };
+        if rv.failed() {
+            return Err(Error::Limit);
+        }
+        usize::try_from(limit).map_err(|_| Error::Limit)
+    }
+
     /// Returns the async thread for this connection. This can be used
     /// with `moz_task` to run synchronous statements on the storage
     /// thread, without blocking the main thread.
@@ -387,6 +398,9 @@ pub enum Error {
     /// closed, or the thread manager may have shut down.
     NoThread,
 
+    /// Failed to get a limit for a database connection.
+    Limit,
+
     /// A database operation failed. The error includes a SQLite result code,
     /// and an explanation string.
     Database {
@@ -453,6 +467,7 @@ impl From<Error> for nsresult {
     fn from(err: Error) -> nsresult {
         match err {
             Error::NoThread => NS_ERROR_NO_INTERFACE,
+            Error::Limit => NS_ERROR_UNEXPECTED,
             Error::Database { rv, .. }
             | Error::BindByIndex { rv, .. }
             | Error::BindByName { rv, .. }
@@ -468,6 +483,7 @@ impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             Error::NoThread => f.write_str("Async thread unavailable for storage connection"),
+            Error::Limit => f.write_str("Failed to get limit for storage connection"),
             Error::Database {
                 op, code, message, ..
             } => {

storage/test/unit/test_storage_connection.js

@@ -1009,6 +1009,13 @@ add_task(async function test_defaultTransactionType() {
   await asyncClose(db);
 });
 
+add_task(async function test_variableLimit() {
+  info("Open connection");
+  let db = Services.storage.openDatabase(getTestDB());
+  Assert.equal(db.variableLimit, 999, "Should return default limit");
+  await asyncClose(db);
+});
+
 add_task(async function test_getInterface() {
   let db = getOpenedDatabase();
   let target = db

third_party/sqlite3/src/sqlite.symbols

@@ -72,6 +72,7 @@ sqlite3_interrupt
 sqlite3_last_insert_rowid
 sqlite3_libversion
 sqlite3_libversion_number
+sqlite3_limit
 sqlite3_load_extension
 sqlite3_malloc
 sqlite3_memory_highwater

toolkit/modules/Sqlite.jsm

@@ -1349,6 +1349,16 @@ OpenedConnection.prototype = Object.freeze({
     return this._connectionData._dbConn;
   },
 
+  /**
+   * Returns the maximum number of bound parameters for statements executed
+   * on this connection.
+   *
+   * @type {number}
+   */
+  get variableLimit() {
+    return this.unsafeRawConnection.variableLimit;
+  },
+
   /**
    * The integer schema version of the database.
    *