Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Expose more JDBC/R2DBC statement methods for dialect authors #5098

Merged
merged 9 commits into from
Apr 4, 2024
Merged

Expose more JDBC/R2DBC statement methods for dialect authors #5098

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
508e287
Expose more JDBC/R2DBC statement methods for dialect authors
hfhbd Mar 22, 2024
9fd033c
Use new typed api
hfhbd Mar 27, 2024
52111b7
Test old behavior
hfhbd Mar 27, 2024
ce9fa31
Adopt drivers
hfhbd Mar 27, 2024
da8a257
Hack: add ignored parameter
hfhbd Mar 27, 2024
aed0efe
Hack: add ignored parameter
hfhbd Mar 27, 2024
be9203c
Revert
hfhbd Mar 27, 2024
e0c54a2
Merge branch 'master' into hfhbd/exposeJDBC
hfhbd Apr 2, 2024
e775f83
Bump api
hfhbd Apr 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions dialects/mysql/src/main/kotlin/app/cash/sqldelight/dialects/mysql/MySqlType.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.INT
import com.squareup.kotlinpoet.LONG
import com.squareup.kotlinpoet.MemberName
import com.squareup.kotlinpoet.SHORT
import com.squareup.kotlinpoet.TypeName

Expand Down Expand Up @@ -45,16 +46,14 @@ internal enum class MySqlType(override val javaType: TypeName) : DialectType {
;

override fun prepareStatementBinder(columnIndex: CodeBlock, value: CodeBlock): CodeBlock {
return CodeBlock.builder()
.add(
when (this) {
TINY_INT, TINY_INT_BOOL, SMALL_INT, INTEGER, BIG_INT, BIT -> "bindLong"
DATE, TIME, DATETIME, TIMESTAMP -> "bindObject"
NUMERIC -> "bindBigDecimal"
},
)
.add("(%L, %L)\n", columnIndex, value)
.build()
return when (this) {
TINY_INT, TINY_INT_BOOL, SMALL_INT, INTEGER, BIG_INT, BIT -> CodeBlock.of("bindLong(%L, %L)\n", columnIndex, value)
DATE -> CodeBlock.of("bindObject(%L, %L, %M)\n", columnIndex, value, MemberName(ClassName("java.sql", "Types"), "DATE"))
TIME -> CodeBlock.of("bindObject(%L, %L, %M)\n", columnIndex, value, MemberName(ClassName("java.sql", "Types"), "TIME"))
DATETIME -> CodeBlock.of("bindObject(%L, %L, %M)\n", columnIndex, value, MemberName(ClassName("java.sql", "Types"), "OTHER"))
TIMESTAMP -> CodeBlock.of("bindObject(%L, %L, %M)\n", columnIndex, value, MemberName(ClassName("java.sql", "Types"), "TIMESTAMP_WITH_TIMEZONE"))
NUMERIC -> CodeBlock.of("bindBigDecimal(%L, %L)\n", columnIndex, value)
}
}

override fun cursorGetter(columnIndex: Int, cursorName: String): CodeBlock {
Expand Down
30 changes: 18 additions & 12 deletions ...ects/postgresql/src/main/kotlin/app/cash/sqldelight/dialects/postgresql/PostgreSqlType.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.INT
import com.squareup.kotlinpoet.LONG
import com.squareup.kotlinpoet.MemberName
import com.squareup.kotlinpoet.SHORT
import com.squareup.kotlinpoet.STRING
import com.squareup.kotlinpoet.TypeName
Expand All @@ -24,19 +25,24 @@ internal enum class PostgreSqlType(override val javaType: TypeName) : DialectTyp
;

override fun prepareStatementBinder(columnIndex: CodeBlock, value: CodeBlock): CodeBlock {
return CodeBlock.builder()
.add(
when (this) {
SMALL_INT -> "bindShort"
INTEGER -> "bindInt"
BIG_INT -> "bindLong"
DATE, TIME, TIMESTAMP, TIMESTAMP_TIMEZONE, INTERVAL, UUID -> "bindObject"
NUMERIC -> "bindBigDecimal"
JSON -> "bindObjectOther"
},
return when (this) {
SMALL_INT -> CodeBlock.of("bindShort(%L, %L)\n", columnIndex, value)
INTEGER -> CodeBlock.of("bindInt(%L, %L)\n", columnIndex, value)
BIG_INT -> CodeBlock.of("bindLong(%L, %L)\n", columnIndex, value)
DATE, TIME, TIMESTAMP, TIMESTAMP_TIMEZONE, INTERVAL, UUID -> CodeBlock.of(
"bindObject(%L, %L)\n",
columnIndex,
value,
)
.add("(%L, %L)\n", columnIndex, value)
.build()

NUMERIC -> CodeBlock.of("bindBigDecimal(%L, %L)\n", columnIndex, value)
JSON -> CodeBlock.of(
"bindObject(%L, %L, %M)\n",
columnIndex,
value,
MemberName(ClassName("java.sql", "Types"), "OTHER"),
)
}
}

override fun cursorGetter(columnIndex: Int, cursorName: String): CodeBlock {
Expand Down
4 changes: 3 additions & 1 deletion ...tgresql/src/main/kotlin/app/cash/sqldelight/dialects/postgresql/PostgreSqlTypeResolver.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ import com.alecstrong.sql.psi.core.psi.SqlStmt
import com.alecstrong.sql.psi.core.psi.SqlTypeName
import com.alecstrong.sql.psi.core.psi.SqlTypes
import com.intellij.psi.tree.TokenSet
import com.squareup.kotlinpoet.ClassName
import com.squareup.kotlinpoet.CodeBlock
import com.squareup.kotlinpoet.MemberName
import com.squareup.kotlinpoet.ParameterizedTypeName.Companion.parameterizedBy
import com.squareup.kotlinpoet.asTypeName

Expand Down Expand Up @@ -285,7 +287,7 @@ class PostgreSqlTypeResolver(private val parentResolver: TypeResolver) : TypeRes
object : DialectType {
override val javaType = Array::class.asTypeName().parameterizedBy(type.javaType)
override fun prepareStatementBinder(columnIndex: CodeBlock, value: CodeBlock) =
CodeBlock.of("bindObject(%L, %L)\n", columnIndex, value)
CodeBlock.of("bindObject(%L, %L, %M)\n", columnIndex, value, MemberName(ClassName("java.sql", "Types"), "ARRAY"))
override fun cursorGetter(columnIndex: Int, cursorName: String) =
CodeBlock.of("$cursorName.getArray<%T>($columnIndex)", type.javaType)
},
Expand Down
49 changes: 26 additions & 23 deletions drivers/jdbc-driver/src/main/kotlin/app/cash/sqldelight/driver/jdbc/JdbcDriver.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,11 +182,7 @@ class JdbcPreparedStatement(
private val preparedStatement: PreparedStatement,
) : SqlPreparedStatement {
override fun bindBytes(index: Int, bytes: ByteArray?) {
if (bytes == null) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not needed, the JDBC driver calls setNull for non primitive types itself.

preparedStatement.setNull(index + 1, Types.BLOB)
} else {
preparedStatement.setBytes(index + 1, bytes)
}
preparedStatement.setBytes(index + 1, bytes)
}

override fun bindBoolean(index: Int, boolean: Boolean?) {
Expand Down Expand Up @@ -246,35 +242,39 @@ class JdbcPreparedStatement(
}

fun bindBigDecimal(index: Int, decimal: BigDecimal?) {
if (decimal == null) {
preparedStatement.setNull(index + 1, Types.NUMERIC)
} else {
preparedStatement.setBigDecimal(index + 1, decimal)
}
preparedStatement.setBigDecimal(index + 1, decimal)
}

@Deprecated(
"For maximum compatibility with all JDBC drivers, setObject requires the sql type.",
replaceWith = ReplaceWith("bindObject(index, obj, Types.OTHER", "java.sql.Types"),
)
fun bindObject(index: Int, obj: Any?) {
if (obj == null) {
preparedStatement.setNull(index + 1, Types.OTHER)
} else {
preparedStatement.setObject(index + 1, obj)
}
bindObject(index, obj, Types.OTHER)
}

fun bindObjectOther(index: Int, obj: Any?) {
fun bindObject(index: Int, obj: Any?, type: Int) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes this a binary-incompatible change, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we can also keep it but it's only used by the Postgres driver.
And according to Alec only the runtime needs to be api compatible.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that was pre-2.0.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Binary compatibility matters less for a driver like R2DBC which is still more "experimental", but the JDBC driver is widely used (and the SQLite one is an implementation of it) so we should definitely maintain compatibility for it.

We should enable the API dumper plugin!

Copy link
Contributor

@griffio griffio Apr 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💁 I can refactor uses of bindObjectOther in the Postgres dialect to use this new api once it has been merged, then remove bindObjectOther before any new release.
bindObjectOther has only recently been added in the snapshot builds - so can be changed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can refactor uses of bindObjectOther

This is already part of this PR, but thanks.

It's only recently been added in the snapshot builds - so can be changed.

In this case I agree, we can change it.

We should enable the API dumper plugin!

Yes, will do it in another PR.

if (obj == null) {
preparedStatement.setNull(index + 1, Types.OTHER)
preparedStatement.setNull(index + 1, type)
} else {
preparedStatement.setObject(index + 1, obj, Types.OTHER)
preparedStatement.setObject(index + 1, obj, type)
}
}

override fun bindString(index: Int, string: String?) {
if (string == null) {
preparedStatement.setNull(index + 1, Types.VARCHAR)
} else {
preparedStatement.setString(index + 1, string)
}
preparedStatement.setString(index + 1, string)
}

fun bindDate(index: Int, date: java.sql.Date?) {
preparedStatement.setDate(index, date)
}

fun bindTime(index: Int, date: java.sql.Time?) {
preparedStatement.setTime(index, date)
}

fun bindTimestamp(index: Int, timestamp: java.sql.Timestamp?) {
preparedStatement.setTimestamp(index, timestamp)
}

fun <R> executeQuery(mapper: (SqlCursor) -> R): R {
Expand Down Expand Up @@ -312,6 +312,9 @@ class JdbcCursor(val resultSet: ResultSet) : SqlCursor {
override fun getDouble(index: Int): Double? = getAtIndex(index, resultSet::getDouble)
fun getBigDecimal(index: Int): BigDecimal? = resultSet.getBigDecimal(index + 1)
inline fun <reified T : Any> getObject(index: Int): T? = resultSet.getObject(index + 1, T::class.java)
fun getDate(index: Int): java.sql.Date? = resultSet.getDate(index)
fun getTime(index: Int): java.sql.Time? = resultSet.getTime(index)
fun getTimestamp(index: Int): java.sql.Timestamp? = resultSet.getTimestamp(index)

@Suppress("UNCHECKED_CAST")
fun <T> getArray(index: Int) = getAtIndex(index, resultSet::getArray)?.array as Array<T>?
Expand Down
60 changes: 49 additions & 11 deletions drivers/r2dbc-driver/src/main/kotlin/app/cash/sqldelight/driver/r2dbc/R2dbcDriver.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import app.cash.sqldelight.db.SqlDriver
import app.cash.sqldelight.db.SqlPreparedStatement
import io.r2dbc.spi.Connection
import io.r2dbc.spi.Statement
import java.math.BigDecimal
import kotlinx.coroutines.CancellationException
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -155,7 +156,7 @@ fun CoroutineScope.R2dbcDriver(
}

// R2DBC uses boxed Java classes instead primitives: https://r2dbc.io/spec/1.0.0.RELEASE/spec/html/#datatypes
class R2dbcPreparedStatement(private val statement: Statement) : SqlPreparedStatement {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposed to allow casting to a driver specific Statement to access driver methods.

class R2dbcPreparedStatement(val statement: Statement) : SqlPreparedStatement {
override fun bindBytes(index: Int, bytes: ByteArray?) {
if (bytes == null) {
statement.bindNull(index, ByteArray::class.java)
Expand All @@ -164,6 +165,22 @@ class R2dbcPreparedStatement(private val statement: Statement) : SqlPreparedStat
}
}

override fun bindBoolean(index: Int, boolean: Boolean?) {
if (boolean == null) {
statement.bindNull(index, Boolean::class.javaObjectType)
} else {
statement.bind(index, boolean)
}
}

fun bindByte(index: Int, byte: Byte?) {
if (byte == null) {
statement.bindNull(index, Byte::class.javaObjectType)
} else {
statement.bind(index, byte)
}
}

fun bindShort(index: Int, short: Short?) {
if (short == null) {
statement.bindNull(index, Short::class.javaObjectType)
Expand All @@ -188,6 +205,14 @@ class R2dbcPreparedStatement(private val statement: Statement) : SqlPreparedStat
}
}

fun bindFloat(index: Int, float: Float?) {
if (float == null) {
statement.bindNull(index, Float::class.javaObjectType)
} else {
statement.bind(index, float)
}
}

override fun bindDouble(index: Int, double: Double?) {
if (double == null) {
statement.bindNull(index, Double::class.javaObjectType)
Expand All @@ -196,29 +221,42 @@ class R2dbcPreparedStatement(private val statement: Statement) : SqlPreparedStat
}
}

override fun bindString(index: Int, string: String?) {
if (string == null) {
statement.bindNull(index, String::class.java)
fun bindBigDecimal(index: Int, decimal: BigDecimal?) {
if (decimal == null) {
statement.bindNull(index, BigDecimal::class.java)
} else {
statement.bind(index, string)
statement.bind(index, decimal)
}
}

override fun bindBoolean(index: Int, boolean: Boolean?) {
if (boolean == null) {
statement.bindNull(index, Boolean::class.javaObjectType)
@Deprecated(
"use bindObject with generics instead",
ReplaceWith("bindObject<T>", ""),
)
fun bindObject(index: Int, any: Any?, ignoredSqlType: Int) {
Copy link
Collaborator Author

@hfhbd hfhbd Mar 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ignoredSqlType parameter is needed because the dialect does not know if it is generating sync or async code. Ideally, we would have two dialects focussing on sync/async or we push the Gradle flag down to the dialect. I am okay with both.

if (any == null) {
statement.bindNull(index, Any::class.java)
} else {
statement.bind(index, boolean)
statement.bind(index, any)
}
}

fun bindObject(index: Int, any: Any?) {
@JvmName("bindTypedObject")
inline fun <reified T : Any> bindObject(index: Int, any: T?) {
if (any == null) {
statement.bindNull(index, Any::class.java)
statement.bindNull(index, T::class.java)
} else {
statement.bind(index, any)
}
}

override fun bindString(index: Int, string: String?) {
if (string == null) {
statement.bindNull(index, String::class.java)
} else {
statement.bind(index, string)
}
}
}

internal fun <T : Any> Publisher<T>.asIterator(): AsyncPublisherIterator<T> =
Expand Down
Loading