cashapp / sqldelight

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

R2dbcPreparedStatement parameters are incorrectly generated when tuple object substitution is used #5313

Closed mike-miroliubov closed 3 days ago

mike-miroliubov commented 5 days ago

SQLDelight Version

2.0.2

Operating System

MacOS

Gradle Version

8.5

Kotlin Version

2.0.0

Dialect

postgresql-dialect

AGP Version

No response

Describe the Bug

Thanks for a great library! I've noticed a bug in the generated queries code when using R2DBC: incorrect parameters syntax is used when a query has tuple object substitution.

Example:

insertUser:
INSERT INTO system_user VALUES ?;

This generates code:

public suspend fun insertUser(system_user: System_user) {
    driver.execute(-75_574_914,
        """INSERT INTO system_user (id, user_name, type, created_at) VALUES (?, ?, ?, ?)""", 4) {
          check(this is R2dbcPreparedStatement)
          bindObject(0, system_user.id)
          bindString(1, system_user.user_name)
          bindString(2, system_userAdapter.typeAdapter.encode(system_user.type))
          bindObject(3, system_user.created_at)
        }.await()
    notifyQueries(-75_574_914) { emit ->
      emit("system_user")
    }
  }

But R2DBC uses $1...1N parameter syntax and this code throws an exception:

java.lang.IndexOutOfBoundsException: Binding index 0 when only 0 parameters are expected
    at io.r2dbc.postgresql.client.Binding.add(Binding.java:75)
    at io.r2dbc.postgresql.PostgresqlStatement.bind(PostgresqlStatement.java:108)
    at io.r2dbc.postgresql.PostgresqlStatement.bind(PostgresqlStatement.java:59)
    at org.mike.miroliubov.kotlin.UsersQueries.insertUser$lambda$6(UsersQueries.kt:163)

However, when a query has individual field parameters:

insertUser:
INSERT INTO system_user (id, user_name, type, created_at) VALUES (?, ?, ?, ?);

The generated code is correct:

public suspend fun insertUser(
    id: UUID,
    user_name: String,
    type: UserType,
    created_at: OffsetDateTime,
  ) {
    driver.execute(-75_574_914,
        """INSERT INTO system_user (id, user_name, type, created_at) VALUES ($1, $2, $3, $4)""", 4) {
          check(this is R2dbcPreparedStatement)
          bindObject(0, id)
          bindString(1, user_name)
          bindString(2, system_userAdapter.typeAdapter.encode(type))
          bindObject(3, created_at)
        }.await()
    notifyQueries(-75_574_914) { emit ->
      emit("system_user")
    }
  }

Stacktrace

java.lang.IndexOutOfBoundsException: Binding index 0 when only 0 parameters are expected
    at io.r2dbc.postgresql.client.Binding.add(Binding.java:75)
    at io.r2dbc.postgresql.PostgresqlStatement.bind(PostgresqlStatement.java:108)
    at io.r2dbc.postgresql.PostgresqlStatement.bind(PostgresqlStatement.java:59)
    at org.mike.miroliubov.kotlin.UsersQueries.insertUser$lambda$6(UsersQueries.kt:163)

Gradle Build Script

plugins {
    kotlin("jvm") version "2.0.0"
    id("io.ktor.plugin") version "2.3.11"
    id("org.jetbrains.kotlin.plugin.serialization") version "2.0.0"
    id("com.google.devtools.ksp") version "2.0.0-1.0.21"
    id("app.cash.sqldelight") version "2.0.2"
}

dependencies {
    implementation("app.cash.sqldelight:r2dbc-driver:2.0.2")
    implementation("app.cash.sqldelight:async-extensions:2.0.2")
    implementation("io.r2dbc:r2dbc-pool:1.0.1.RELEASE")
    implementation("org.postgresql:r2dbc-postgresql:1.0.5.RELEASE")

    implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactive:1.8.1")
}

sqldelight {
    databases {
        create("Database") {
            packageName.set("...")
            dialect("app.cash.sqldelight:postgresql-dialect:2.0.2")
            generateAsync = true
            deriveSchemaFromMigrations.set(true)
            migrationOutputDirectory = file("${buildDir}/resources/main/migrations")
            migrationOutputFileFormat = ".sql" // Defaults to .sql
        }
    }
}
griffio commented 5 days ago

🤔 Problem is here (data class tuple insert) where the ? is hardcoded https://github.com/cashapp/sqldelight/blob/6aa2924c49dff814e1d4f757e15d723848969724/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt#L195 🪄 The code from here (single parameter insert) is needed to determine if the async bind param is generated https://github.com/cashapp/sqldelight/blob/6aa2924c49dff814e1d4f757e15d723848969724/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/compiler/QueryGenerator.kt#L222

🌳 Somehow TreeUtil will need to know that async generation is being used and the bind parameters are indexed e.g in PsiElement.rangesToReplace() do something like ...

    val generateAsync = this.sqFile().generateAsync
    val bindExpr = childOfType(SqlTypes.BIND_EXPR) as SqlBindExpr
    val bindParameterMixin = bindExpr.bindParameter as BindParameterMixin
    buildList {
      if (parent!!.columnNameList.isEmpty()) {
        add(
          Pair(
            first = parent!!.tableName.range,
            second = parent!!.columns.joinToString(
              separator = ", ",
              prefix = "${parent!!.tableName.text} (",
              postfix = ")",
            ) { it.name },
          ),
        )
      }
      add(
        Pair(
          first = childOfType(SqlTypes.BIND_EXPR)!!.range,
          second = (1..parent!!.columns.size).joinToString(separator = ", ", prefix = "(", postfix = ")") { bindParameterMixin.replaceWith(generateAsync, it) },
        ),
      )