Skip to content

Fix 5519 var args with r2dbc driver #5523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

griffio
Copy link
Contributor

@griffio griffio commented Oct 22, 2024

fix #5519

🦥 In Progress

select:
SELECT *
FROM X WHERE col1 IN ?;

I think the only place in SqlDelight that generates Sql at runtime is to expand var args to IN (?, ?,...)

🛞 The Driver and Dialect are responsible for any specific implementation of runtime argument binding
⚡ Transacter feels like the wrong place for generating runtime sql. Keep existing createArguments as there is a lot of test code generated and instead forward to the Driver implementation
🚨 MySql/MariaDb R2dbc uses ? and PostgreSql R2dbc uses $1, $2 ... makes it difficult to determine the argument binding needed as we need to know the dialect and driver

  • Adds createArguments is part of Driver/Dialect behaviour - PostgreSql native R2dbc has specific bind argument and reference by position and index. If you were manually using the driver to execute sql then createArguments should be part of that
  • Add default implementation for ? binding
  • Update Transacter.createArguments forward to Driver.createArguments so there is no change to compiled code
  • Update SqlDriver for runtime.api and android.api and jdbc.api
  • Add class PostgresqlR2dbcDriver to override createArguments for special case
    • Currently uses delegation as a SqlDriver unless we really need it then could open and inherit from R2dbcDriver
  • Add async integration test for var arg binding using IN ?

Example client use

suspend fun getAsyncSqlDriver(): SqlDriver {
    val connectionFactory = getR2dbcConnectionFactory()
    return PostgreSqlR2dbcDriver(connectionFactory.create().awaitFirst())
}

Note: This is different than the fix for #5319 where the expansion of INSERT VALUES var args can be known at compile time.

The Driver should be responsible for specific implementation of runtime argument binding - add default implementation

Transacter feels like the wrong place for that
Keep existing createArguments as there is alot of test code generated and forward to Driver implementation
createArguments is part of Driver behaviour - R2dbc has specific bind argument and reference by position and index
Add PostgresqlR2dbcDriver class to allow override of createArguments and keep default inherited from SqlDriver

Use delegation to avoid opening R2dbcDriver
Make compatible with R2dbcDriver
For integration tests
var args driver tests
@jtlapp
Copy link

jtlapp commented Feb 28, 2025

Gosh, if you merge this, I could use be using SQLDelight. The tool does look like such a delight.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

(r2dbc) PostgreSQL uses index parameters that are prefixed with $
2 participants