cashapp / sqldelight

SQLDelight - Generates typesafe Kotlin APIs from SQL
https://cashapp.github.io/sqldelight/
Apache License 2.0
6.14k stars 514 forks source link

Expose Transacter.Transaction components to be used from a SqlDriver #1856

Open rharter opened 4 years ago

rharter commented 4 years ago

I've been working on a fairly generalized migration mechanism, which has highlighted that a SqlDriver's transactions aren't usable externally, even though they're documented as such.

The underlying problem is that, while I can create a transaction from the driver using SqlDriver.newTransaction(), there is no way to set the transaction successful, or call endTransaction(), since those functions and properties have internal scoping.

In the migration example that I alluded to earlier, the functionality needs to be executed at the driver level (since it's bootstrapping the generated Database), and ultimately should happen in a transaction. As I mentioned above, I'm not able to get a usable transaction from the driver, so I'm stuck in this weird limbo.

Here is my code, for reference.

SqlDriver.migrate ```kotlin fun SqlDriver.migrate(schema: SqlDriver.Schema, logger: Logger? = null) { var needsMetaTable = false val version = try { executeQuery(null, "SELECT value FROM __sqldelight__ WHERE name = 'schema_version'", 0).use { (if (it.next()) it.getLong(0)?.toInt() else 0) ?: 0 } } catch (e: Exception) { needsMetaTable = true 0 } if (version < schema.version) { logger?.debug("Migrating database from schema version $version to version ${schema.version}") execute(null, "SET REFERENTIAL_INTEGRITY FALSE", 0) if (version == 0) schema.create(this) else schema.migrate(this, version, schema.version) execute(null, "SET REFERENTIAL_INTEGRITY TRUE", 0) if (needsMetaTable) { execute(null, "CREATE TABLE __sqldelight__(name VARCHAR NOT NULL PRIMARY KEY, value VARCHAR)", 0) } if (version == 0) { execute(null, "INSERT INTO __sqldelight__(name, value) VALUES('schema_version', ${Schema.version})", 0) } else { execute(null, "UPDATE __sqldelight__ SET value='${Schema.version}' WHERE name='schema_version'", 0) } } } ```

This type of bootstrapping migration needs to happen at the driver layer, since the generated Database doesn't support executing arbitrary queries. Using a .sq file with proper tables and queries also doesn't work since that runs into the bootstrap problem.

Ideally this would execute within a transaction, but I can't currently do that from the driver. Here's an example of what would make this possible:

val transaction = driver.newTransaction()
try {
  // ... All of that migration code
  transaction.successful = true //<- successful is internal, so can't be set.
} finally {
  transaction.endTransaction() //<- endTransaction() is also internal
}

Another cleaner option would be to make SqlDriver implement the Transacter interface, allowing the standard transaction mechanisms that are used from a Database.

driver.transaction {
  // ... All of that migration code
}

I'm happy to contribute this change, but wanted to make sure I'm not missing something, or using the wrong approach.

AlecKazakova commented 4 years ago

i'd be in favor of the driver implementing transacter if possible. The thing i want to avoid is having extra APIs exposed in consumer code:

database.transaction {
  successful = true
  endTransaction()
}

so if implementing transacter isn't possible, my next thought would be having driver.newTransaction() expose a type different from the receiver on transaction {} so they can have separate APIs. I guess the type exposed by driver.newTransaction() could be a subtype of Transacter.Transaction?

also that <details><summary> trick is dope

Virelion commented 3 years ago

Is there any sane way of handling transaction outside transaction {} function. Lack of ability to use endTransaction() is making use SQLDelight with coroutines impossible.

marcardar commented 3 years ago

Not having an inline transaction extension function (an equivalent to the one in androidx) is making adoption of SQLDelight very difficult for me, because of the inability to call suspending functions. Have there been any recent thoughts/progress regarding this?

JakeWharton commented 3 years ago

There are no plans to allow suspending functions in a transaction since they are intrinsically tied to a single thread and take write locks on the database. Room was forced to throw heaps of code at this problem because without it you would have no way of calling your suspending DAO functions from inside a transaction. SQL Delight does not have this problem since we expose everything as a Query that can be run synchronously.