Only run certain PRAGMAs on writer connections (#127)

- foreign_key and synchronous only affect writes, so they aren't
    needed on reader connections
  - Also prevent use of connections before schema creation/migration

Author: eygraber

library/src/commonMain/kotlin/com/eygraber/sqldelight/androidx/driver/AndroidxSqliteDriver.kt

@@ -37,7 +37,7 @@ public class AndroidxSqliteDriver(
   connectionFactory: AndroidxSqliteConnectionFactory,
   databaseType: AndroidxSqliteDatabaseType,
   private val schema: SqlSchema<QueryResult.Value<Unit>>,
-  private val configuration: AndroidxSqliteConfiguration = AndroidxSqliteConfiguration(),
+  configuration: AndroidxSqliteConfiguration = AndroidxSqliteConfiguration(),
   private val migrateEmptySchema: Boolean = false,
   /**
    * A callback to configure the database connection when it's first opened.
@@ -124,6 +124,8 @@ public class AndroidxSqliteDriver(
   @Suppress("NonBooleanPropertyPrefixedWithIs")
   private val isFirstInteraction = atomic(true)
 
+  private val configuration get() = connectionPool.configuration
+
   private val connectionPool by lazy {
     val nameProvider = when(databaseType) {
       is AndroidxSqliteDatabaseType.File -> databaseType::databaseFilePath
@@ -196,39 +198,52 @@ public class AndroidxSqliteDriver(
   private val migrationCallbacks = migrationCallbacks
 
   /**
-   * True if foreign key constraints are enabled.
+   * Journal mode to use.
+   *
+   * This function will block until pending schema creation/migration is completed,
+   * and all created connections have been updated.
+   *
+   * Note that this means that this [SqliteJournalMode] **will not** be used for schema creation/migration.
    *
-   * This function will block until all created connections have been updated.
+   * Please use [AndroidxSqliteConfiguration] or [onConfigure] if a specific [SqliteJournalMode] is needed
+   * during schema creation/migration.
    *
    * An exception will be thrown if this is called from within a transaction.
    */
-  public fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {
+  public fun setJournalMode(journalMode: SqliteJournalMode) {
     check(currentTransaction() == null) {
-      "setForeignKeyConstraintsEnabled cannot be called from within a transaction"
+      "setJournalMode cannot be called from within a transaction"
     }
 
-    connectionPool.setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled)
+    // run creation or migration if needed before setting the journal mode
+    createOrMigrateIfNeeded()
+
+    connectionPool.setJournalMode(journalMode)
   }
 
   /**
-   * Journal mode to use.
-   *
-   * This function will block until all created connections have been updated.
+   * This function will block until executed on the writer connection.
    *
    * An exception will be thrown if this is called from within a transaction.
    */
-  public fun setJournalMode(journalMode: SqliteJournalMode) {
+  public fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {
     check(currentTransaction() == null) {
-      "setJournalMode cannot be called from within a transaction"
+      "setForeignKeyConstraintsEnabled cannot be called from within a transaction"
     }
 
-    connectionPool.setJournalMode(journalMode)
+    connectionPool.updateForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled)
+
+    val foreignKeys = if(isForeignKeyConstraintsEnabled) "ON" else "OFF"
+    execute(
+      identifier = null,
+      sql = "PRAGMA foreign_keys = $foreignKeys;",
+      parameters = 0,
+      binders = null,
+    )
   }
 
   /**
-   * Synchronous mode to use.
-   *
-   * This function will block until all created connections have been updated.
+   * This function will block until executed on the writer connection.
    *
    * An exception will be thrown if this is called from within a transaction.
    */
@@ -237,7 +252,14 @@ public class AndroidxSqliteDriver(
       "setSync cannot be called from within a transaction"
     }
 
-    connectionPool.setSync(sync)
+    connectionPool.updateSync(sync)
+
+    execute(
+      identifier = null,
+      sql = "PRAGMA synchronous = ${sync.value};",
+      parameters = 0,
+      binders = null,
+    )
   }
 
   override fun addListener(vararg queryKeys: String, listener: Query.Listener) {
@@ -407,8 +429,15 @@ public class AndroidxSqliteDriver(
   ): QueryResult.Value<R> {
     createOrMigrateIfNeeded()
 
+    // PRAGMA foreign_keys and synchronous should always be queried from the writer connection
+    // since these are per-connection settings and only the writer connection has them set
+    val shouldUseWriterConnection = sql.trim().run {
+      startsWith("PRAGMA foreign_keys", ignoreCase = true) ||
+        startsWith("PRAGMA synchronous", ignoreCase = true)
+    }
+
     val transaction = currentTransaction()
-    if(transaction == null) {
+    if(transaction == null && !shouldUseWriterConnection) {
       val connection = connectionPool.acquireReaderConnection()
       try {
         return execute(
@@ -428,20 +457,30 @@ public class AndroidxSqliteDriver(
         connectionPool.releaseReaderConnection(connection)
       }
     } else {
-      val connection = (transaction as Transaction).connection
-      return execute(
-        identifier = identifier,
-        connection = connection,
-        createStatement = { c ->
-          AndroidxQuery(
-            sql = sql,
-            statement = c.prepare(sql),
-            argCount = parameters,
-          )
-        },
-        binders = binders,
-        result = { executeQuery(mapper) },
-      )
+      val connection = when(transaction) {
+        null -> connectionPool.acquireWriterConnection()
+        else -> (transaction as Transaction).connection
+      }
+
+      try {
+        return execute(
+          identifier = identifier,
+          connection = connection,
+          createStatement = { c ->
+            AndroidxQuery(
+              sql = sql,
+              statement = c.prepare(sql),
+              argCount = parameters,
+            )
+          },
+          binders = binders,
+          result = { executeQuery(mapper) },
+        )
+      } finally {
+        if(transaction == null) {
+          connectionPool.releaseWriterConnection()
+        }
+      }
     }
   }
 

library/src/commonMain/kotlin/com/eygraber/sqldelight/androidx/driver/ConnectionPool.kt

@@ -9,13 +9,15 @@ import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 
 public interface ConnectionPool : AutoCloseable {
+  public val configuration: AndroidxSqliteConfiguration
+
   public fun acquireWriterConnection(): SQLiteConnection
   public fun releaseWriterConnection()
   public fun acquireReaderConnection(): SQLiteConnection
   public fun releaseReaderConnection(connection: SQLiteConnection)
-  public fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean)
   public fun setJournalMode(journalMode: SqliteJournalMode)
-  public fun setSync(sync: SqliteSync)
+  public fun updateForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean)
+  public fun updateSync(sync: SqliteSync)
 }
 
 internal class AndroidxDriverConnectionPool(
@@ -29,14 +31,14 @@ internal class AndroidxDriverConnectionPool(
     val connection: Lazy<SQLiteConnection>,
   )
 
-  private var configuration by atomic(configuration)
+  override var configuration by atomic(configuration)
 
   private val name by lazy { nameProvider() }
 
   private val writerConnection: SQLiteConnection by lazy {
     connectionFactory
       .createConnection(name)
-      .withConfiguration(configuration)
+      .withWriterConfiguration(configuration)
   }
 
   private val writerMutex = Mutex()
@@ -56,7 +58,7 @@ internal class AndroidxDriverConnectionPool(
           lazy {
             connectionFactory
               .createConnection(name)
-              .withConfiguration(configuration)
+              .withReaderConfiguration(configuration)
           },
         ),
       )
@@ -108,32 +110,27 @@ internal class AndroidxDriverConnectionPool(
     }
   }
 
-  override fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {
+  override fun setJournalMode(journalMode: SqliteJournalMode) {
     configuration = configuration.copy(
-      isForeignKeyConstraintsEnabled = isForeignKeyConstraintsEnabled,
+      journalMode = journalMode,
     )
 
-    val foreignKeys = if(isForeignKeyConstraintsEnabled) "ON" else "OFF"
-    runPragmaOnAllConnections("PRAGMA foreign_keys = $foreignKeys;")
+    runPragmaOnAllCreatedConnections("PRAGMA journal_mode = ${configuration.journalMode.value};")
   }
 
-  override fun setJournalMode(journalMode: SqliteJournalMode) {
+  override fun updateForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {
     configuration = configuration.copy(
-      journalMode = journalMode,
+      isForeignKeyConstraintsEnabled = isForeignKeyConstraintsEnabled,
     )
-
-    runPragmaOnAllConnections("PRAGMA journal_mode = ${configuration.journalMode.value};")
   }
 
-  override fun setSync(sync: SqliteSync) {
+  override fun updateSync(sync: SqliteSync) {
     configuration = configuration.copy(
       sync = sync,
     )
-
-    runPragmaOnAllConnections("PRAGMA synchronous = ${configuration.sync.value};")
   }
 
-  private fun runPragmaOnAllConnections(sql: String) {
+  private fun runPragmaOnAllCreatedConnections(sql: String) {
     val writer = acquireWriterConnection()
     try {
       writer.execSQL(sql)
@@ -180,12 +177,12 @@ internal class PassthroughConnectionPool(
   nameProvider: () -> String,
   configuration: AndroidxSqliteConfiguration,
 ) : ConnectionPool {
-  private var configuration by atomic(configuration)
+  override var configuration by atomic(configuration)
 
   private val name by lazy { nameProvider() }
 
   private val delegatedConnection: SQLiteConnection by lazy {
-    connectionFactory.createConnection(name).withConfiguration(configuration)
+    connectionFactory.createConnection(name).withWriterConfiguration(configuration)
   }
 
   override fun acquireWriterConnection() = delegatedConnection
@@ -196,15 +193,6 @@ internal class PassthroughConnectionPool(
 
   override fun releaseReaderConnection(connection: SQLiteConnection) {}
 
-  override fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {
-    configuration = configuration.copy(
-      isForeignKeyConstraintsEnabled = isForeignKeyConstraintsEnabled,
-    )
-
-    val foreignKeys = if(isForeignKeyConstraintsEnabled) "ON" else "OFF"
-    delegatedConnection.execSQL("PRAGMA foreign_keys = $foreignKeys;")
-  }
-
   override fun setJournalMode(journalMode: SqliteJournalMode) {
     configuration = configuration.copy(
       journalMode = journalMode,
@@ -217,20 +205,24 @@ internal class PassthroughConnectionPool(
     delegatedConnection.execSQL("PRAGMA foreign_keys = $foreignKeys;")
   }
 
-  override fun setSync(sync: SqliteSync) {
+  override fun updateForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {
     configuration = configuration.copy(
-      sync = sync,
+      isForeignKeyConstraintsEnabled = isForeignKeyConstraintsEnabled,
     )
+  }
 
-    delegatedConnection.execSQL("PRAGMA synchronous = ${configuration.sync.value};")
+  override fun updateSync(sync: SqliteSync) {
+    configuration = configuration.copy(
+      sync = sync,
+    )
   }
 
   override fun close() {
     delegatedConnection.close()
   }
 }
 
-private fun SQLiteConnection.withConfiguration(
+private fun SQLiteConnection.withWriterConfiguration(
   configuration: AndroidxSqliteConfiguration,
 ): SQLiteConnection = this.apply {
   // copy the configuration for thread safety
@@ -243,3 +235,10 @@ private fun SQLiteConnection.withConfiguration(
     execSQL("PRAGMA foreign_keys = $foreignKeys;")
   }
 }
+
+private fun SQLiteConnection.withReaderConfiguration(
+  configuration: AndroidxSqliteConfiguration,
+): SQLiteConnection = this.apply {
+  // copy the configuration for thread safety
+  execSQL("PRAGMA journal_mode = ${configuration.copy().journalMode.value};")
+}

library/src/commonTest/kotlin/com/eygraber/sqldelight/androidx/driver/AndroidxSqliteTransacterTest.kt

@@ -341,11 +341,14 @@ private class FirstTransactionsFailConnectionPool : ConnectionPool {
   override fun close() {
     firstTransactionFailConnection.close()
   }
+
+  override val configuration = AndroidxSqliteConfiguration()
+
   override fun acquireWriterConnection() = firstTransactionFailConnection
   override fun releaseWriterConnection() {}
   override fun acquireReaderConnection() = firstTransactionFailConnection
   override fun releaseReaderConnection(connection: SQLiteConnection) {}
-  override fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {}
   override fun setJournalMode(journalMode: SqliteJournalMode) {}
-  override fun setSync(sync: SqliteSync) {}
+  override fun updateForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {}
+  override fun updateSync(sync: SqliteSync) {}
 }