Bug 1422383 - Clone temporary tables, views, and triggers when cloning a storage connection. r=mak
authorKit Cambridge <kit@yakshaving.ninja>
Thu, 30 Nov 2017 19:21:10 -0800
changeset 394902 cc8eaa26f2ed97c7a481f5a8a7e325f5d7794fe7
parent 394901 dd84a4ad910edc4f587dc08c722bb54c4a3d1a3c
child 394903 48e74a9e3496f65dcfc142533fe59f5253e7cc25
push id33025
push usershindli@mozilla.com
push dateTue, 05 Dec 2017 09:57:50 +0000
treeherdermozilla-central@390c1aad9d4d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1422383
milestone59.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1422383 - Clone temporary tables, views, and triggers when cloning a storage connection. r=mak MozReview-Commit-ID: HwVMvvn7cui
storage/mozIStorageAsyncConnection.idl
storage/mozIStorageConnection.idl
storage/mozStorageConnection.cpp
storage/test/unit/test_storage_connection.js
--- a/storage/mozIStorageAsyncConnection.idl
+++ b/storage/mozIStorageAsyncConnection.idl
@@ -90,16 +90,21 @@ interface mozIStorageAsyncConnection : n
    *        - temp_store
    *       The following pragmas are copied over to a writeable clone:
    *        - cache_size
    *        - temp_store
    *        - foreign_keys
    *        - journal_size_limit
    *        - synchronous
    *        - wal_autocheckpoint
+   *       All SQL functions are copied over to read-only and writeable clones.
+   *       Additionally, all temporary tables, triggers, and views, as well as
+   *       any indexes on temporary tables, are copied over to writeable clones.
+   *       For temporary tables, only the schemas are copied, not their
+   *       contents.
    */
   void asyncClone(in boolean aReadOnly,
                   in mozIStorageCompletionCallback aCallback);
 
   /**
    * The current database nsIFile.  Null if the database
    * connection refers to an in-memory database.
    */
--- a/storage/mozIStorageConnection.idl
+++ b/storage/mozIStorageConnection.idl
@@ -77,16 +77,21 @@ interface mozIStorageConnection : mozISt
    *        - temp_store
    *       The following pragmas are copied over to a writeable clone:
    *        - cache_size
    *        - temp_store
    *        - foreign_keys
    *        - journal_size_limit
    *        - synchronous
    *        - wal_autocheckpoint
+   *       All SQL functions are copied over to read-only and writeable clones.
+   *       Additionally, all temporary tables, triggers, and views, as well as
+   *       any indexes on temporary tables, are copied over to writeable clones.
+   *       For temporary tables, only the schemas are copied, not their
+   *       contents.
    *
    */
   mozIStorageConnection clone([optional] in boolean aReadOnly);
 
   /**
    * The default size for SQLite database pages used by mozStorage for new
    * databases.
    */
--- a/storage/mozStorageConnection.cpp
+++ b/storage/mozStorageConnection.cpp
@@ -1521,16 +1521,49 @@ nsresult
 Connection::initializeClone(Connection* aClone, bool aReadOnly)
 {
   nsresult rv = mFileURL ? aClone->initialize(mFileURL)
                          : aClone->initialize(mDatabaseFile);
   if (NS_FAILED(rv)) {
     return rv;
   }
 
+  // Copy over temporary tables, triggers, and views from the original
+  // connections. Entities in `sqlite_temp_master` are only visible to the
+  // connection that created them.
+  if (!aReadOnly) {
+    nsCOMPtr<mozIStorageStatement> stmt;
+    rv = CreateStatement(NS_LITERAL_CSTRING("SELECT sql FROM sqlite_temp_master "
+                                            "WHERE type IN ('table', 'view', "
+                                                           "'index', 'trigger')"),
+                         getter_AddRefs(stmt));
+    // Propagate errors, because failing to copy triggers might cause schema
+    // coherency issues when writing to the database from the cloned connection.
+    NS_ENSURE_SUCCESS(rv, rv);
+    bool hasResult = false;
+    while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
+      nsAutoCString query;
+      rv = stmt->GetUTF8String(0, query);
+      NS_ENSURE_SUCCESS(rv, rv);
+
+      // The `CREATE` SQL statements in `sqlite_temp_master` omit the `TEMP`
+      // keyword. We need to add it back, or we'll recreate temporary entities
+      // as persistent ones. `sqlite_temp_master` also holds `CREATE INDEX`
+      // statements, but those don't need `TEMP` keywords.
+      if (StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE TABLE ")) ||
+          StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE TRIGGER ")) ||
+          StringBeginsWith(query, NS_LITERAL_CSTRING("CREATE VIEW "))) {
+        query.Replace(0, 6, "CREATE TEMP");
+      }
+
+      rv = aClone->ExecuteSimpleSQL(query);
+      NS_ENSURE_SUCCESS(rv, rv);
+    }
+  }
+
   // Re-attach on-disk databases that were attached to the original connection.
   {
     nsCOMPtr<mozIStorageStatement> stmt;
     rv = CreateStatement(NS_LITERAL_CSTRING("PRAGMA database_list"),
                          getter_AddRefs(stmt));
     MOZ_ASSERT(NS_SUCCEEDED(rv));
     bool hasResult = false;
     while (stmt && NS_SUCCEEDED(stmt->ExecuteStep(&hasResult)) && hasResult) {
--- a/storage/test/unit/test_storage_connection.js
+++ b/storage/test/unit/test_storage_connection.js
@@ -748,16 +748,158 @@ add_task(async function test_clone_attac
   stmt = db3.createStatement("SELECT * FROM attached_2.sqlite_master");
   stmt.finalize();
 
   db1.close();
   db2.close();
   db3.close();
 });
 
+add_task(async function test_async_clone_with_temp_trigger_and_table() {
+  do_print("Open connection");
+  let db = getService().openDatabase(getTestDB());
+  do_check_true(db instanceof Ci.mozIStorageAsyncConnection);
+
+  do_print("Set up tables on original connection");
+  let createQueries = [
+    `CREATE TEMP TABLE test_temp(name TEXT)`,
+    `CREATE INDEX test_temp_idx ON test_temp(name)`,
+    `CREATE TEMP TRIGGER test_temp_afterdelete_trigger
+     AFTER DELETE ON test_temp FOR EACH ROW
+     BEGIN
+       INSERT INTO test(name) VALUES(OLD.name);
+     END`];
+  for (let query of createQueries) {
+    let stmt = db.createAsyncStatement(query);
+    await executeAsync(stmt);
+    stmt.finalize();
+  }
+
+  do_print("Create read-write clone with temp tables");
+  let readWriteClone = await asyncClone(db, false);
+  do_check_true(readWriteClone instanceof Ci.mozIStorageAsyncConnection);
+
+  do_print("Insert into temp table on read-write clone");
+  let insertStmt = readWriteClone.createAsyncStatement(`
+    INSERT INTO test_temp(name) VALUES('mak'), ('standard8'), ('markh')`);
+  await executeAsync(insertStmt);
+  insertStmt.finalize();
+
+  do_print("Fire temp trigger on read-write clone");
+  let deleteStmt = readWriteClone.createAsyncStatement(`
+    DELETE FROM test_temp`);
+  await executeAsync(deleteStmt);
+  deleteStmt.finalize();
+
+  do_print("Read from original connection");
+  let names = [];
+  let readStmt = db.createStatement(`SELECT name FROM test`);
+  while (readStmt.executeStep()) {
+    names.push(readStmt.getUTF8String(0));
+  }
+  readStmt.finalize();
+  do_check_true(names.includes("mak"));
+  do_check_true(names.includes("standard8"));
+  do_check_true(names.includes("markh"));
+
+  do_print("Create read-only clone");
+  let readOnlyClone = await asyncClone(db, true);
+  do_check_true(readOnlyClone instanceof Ci.mozIStorageAsyncConnection);
+
+  do_print("Read-only clone shouldn't have temp entities");
+  let badStmt = readOnlyClone.createAsyncStatement(`SELECT 1 FROM test_temp`);
+  await Assert.rejects(executeAsync(badStmt));
+  badStmt.finalize();
+
+  do_print("Clean up");
+  for (let conn of [db, readWriteClone, readOnlyClone]) {
+    await asyncClose(conn);
+  }
+});
+
+add_task(async function test_sync_clone_in_transaction() {
+  do_print("Open connection");
+  let db = getService().openDatabase(getTestDB());
+  do_check_true(db instanceof Ci.mozIStorageAsyncConnection);
+
+  do_print("Begin transaction on main connection");
+  db.beginTransaction();
+
+  do_print("Create temp table and trigger in transaction");
+  let createQueries = [
+    `CREATE TEMP TABLE test_temp(name TEXT)`,
+    `CREATE TEMP TRIGGER test_temp_afterdelete_trigger
+     AFTER DELETE ON test_temp FOR EACH ROW
+     BEGIN
+       INSERT INTO test(name) VALUES(OLD.name);
+     END`,
+  ];
+  for (let query of createQueries) {
+    db.executeSimpleSQL(query);
+  }
+
+  do_print("Clone main connection while transaction is in progress");
+  let clone = db.clone(/* aReadOnly */ false);
+
+  // Dropping the table also drops `test_temp_afterdelete_trigger`.
+  do_print("Drop temp table on main connection");
+  db.executeSimpleSQL(`DROP TABLE test_temp`);
+
+  do_print("Commit transaction");
+  db.commitTransaction();
+
+  do_print("Clone connection should still have temp entities");
+  let readTempStmt = clone.createStatement(`SELECT 1 FROM test_temp`);
+  readTempStmt.execute();
+  readTempStmt.finalize();
+
+  do_print("Clean up");
+
+  db.close();
+  clone.close();
+});
+
+add_task(async function test_sync_clone_with_function() {
+  do_print("Open connection");
+  let db = getService().openDatabase(getTestDB());
+  do_check_true(db instanceof Ci.mozIStorageAsyncConnection);
+
+  do_print("Create SQL function");
+  function storeLastInsertedNameFunc() {
+    this.name = null;
+  }
+  storeLastInsertedNameFunc.prototype = {
+    onFunctionCall(args) {
+      this.name = args.getUTF8String(0);
+    },
+  };
+  let func = new storeLastInsertedNameFunc();
+  db.createFunction("store_last_inserted_name", 1, func);
+
+  do_print("Create temp trigger on main connection");
+  db.executeSimpleSQL(`
+    CREATE TEMP TRIGGER test_afterinsert_trigger
+    AFTER INSERT ON test FOR EACH ROW
+    BEGIN
+      SELECT store_last_inserted_name(NEW.name);
+    END`);
+
+  do_print("Clone main connection");
+  let clone = db.clone(/* aReadOnly */ false);
+
+  do_print("Write to clone");
+  clone.executeSimpleSQL(`INSERT INTO test(name) VALUES('kit')`);
+
+  do_check_eq(func.name, "kit");
+
+  do_print("Clean up");
+  db.close();
+  clone.close();
+});
+
 add_task(async function test_getInterface() {
   let db = getOpenedDatabase();
   let target = db.QueryInterface(Ci.nsIInterfaceRequestor)
                  .getInterface(Ci.nsIEventTarget);
   // Just check that target is non-null.  Other tests will ensure that it has
   // the correct value.
   do_check_true(target != null);