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.

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

Behavior change in 2.x when coercing Boolean column types #5247

Open
ashughes opened this issue May 10, 2024 · 0 comments
Open

Behavior change in 2.x when coercing Boolean column types #5247

ashughes opened this issue May 10, 2024 · 0 comments

Comments

@ashughes
Copy link

SQLDelight Version

2.0.2

SQLDelight Dialect

sqlite-3-25-dialect

Describe the Bug

The behavior of parsing a Boolean column changed between 1.x and 2.x from having a single implementation to now being dependent on the driver implementation.

In the generated *Queries class, a query containing a Boolean column changed from the following in 1.x:

cursor.getLong(4)!! == 1L,

to the following in 2.x:

cursor.getBoolean(4)!!,

In our project we use 2 drivers: Android and JDBC (for tests).

AndroidCursor in sqldelight/drivers/android-driver/src/main/java/app/cash/sqldelight/driver/android/AndroidSqliteDriver.kt has the following implementation:

override fun getBoolean(index: Int) = if (cursor.isNull(index)) null else cursor.getLong(index) == 1L

JdbcCursor in drivers/jdbc-driver/src/main/kotlin/app/cash/sqldelight/driver/jdbc/JdbcDriver.kt has the following implementation:

override fun getBoolean(index: Int): Boolean? = getAtIndex(index, resultSet::getBoolean)

and JDBC3ResultSet in src/main/java/org/sqlite/jdbc3/JDBC3ResultSet.java has the following implementation:

public boolean getBoolean(int col) throws SQLException {
    return getInt(col) != 0;
}

We have some tests that validate coercing erroneous data (which includes values like -1 and 2 for the Boolean column). In 1.x these values were always coerced to false since the implementation was cursor.getLong(col)!! == 1L. Now in 2.x these evaluate to true in our tests since they use the JDBC implementation of getInt(col) != 0, while on Android they would still evaluate to false.

I'm not necessarily sure this is a bug in SQLDelight, but at the very least it's an unexpected behavior change in 2.x.

In order to ensure consistent behavior across drivers, is there a way to define a ColumnAdapter<Boolean, Long> or would we have to specify our own domain type in order to use a ColumnAdapter? We already do this for our domain types, but I'm not seeing how we could do this for a Boolean.

Stacktrace

No response

@ashughes ashughes added the bug label May 10, 2024
@JakeWharton JakeWharton removed the bug label Oct 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants