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

fix: EXPOSED-405 SQLite bugs: Table with custom ID behaves weirdly in DAO and batchInsert #2119

Merged
merged 3 commits into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions exposed-core/api/exposed-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -2960,6 +2960,7 @@ public class org/jetbrains/exposed/sql/statements/BatchUpsertStatement : org/jet
public final fun getWhere ()Lorg/jetbrains/exposed/sql/Op;
protected fun isColumnValuePreferredFromResultSet (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)Z
public fun prepareSQL (Lorg/jetbrains/exposed/sql/Transaction;Z)Ljava/lang/String;
public fun prepared (Lorg/jetbrains/exposed/sql/Transaction;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;
}

public class org/jetbrains/exposed/sql/statements/DeleteStatement : org/jetbrains/exposed/sql/statements/Statement {
Expand Down Expand Up @@ -3259,6 +3260,7 @@ public class org/jetbrains/exposed/sql/statements/UpsertStatement : org/jetbrain
public final fun getWhere ()Lorg/jetbrains/exposed/sql/Op;
protected fun isColumnValuePreferredFromResultSet (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)Z
public fun prepareSQL (Lorg/jetbrains/exposed/sql/Transaction;Z)Ljava/lang/String;
public fun prepared (Lorg/jetbrains/exposed/sql/Transaction;Ljava/lang/String;)Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;
}

public final class org/jetbrains/exposed/sql/statements/api/ExposedBlob {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package org.jetbrains.exposed.sql.statements

import org.jetbrains.exposed.sql.*
import org.jetbrains.exposed.sql.statements.api.PreparedStatementApi
import org.jetbrains.exposed.sql.vendors.H2Dialect
import org.jetbrains.exposed.sql.vendors.H2FunctionProvider
import org.jetbrains.exposed.sql.vendors.MysqlFunctionProvider
import org.jetbrains.exposed.sql.vendors.currentDialect

/**
* Represents the SQL statement that either batch inserts new rows into a table, or updates the existing rows if insertions violate unique constraints.
Expand Down Expand Up @@ -51,6 +53,15 @@ open class BatchUpsertStatement(
}
}

override fun prepared(transaction: Transaction, sql: String): PreparedStatementApi {
// We must return values from upsert because returned id could be different depending on insert or upsert happened
if (!currentDialect.supportsOnlyIdentifiersInGeneratedKeys) {
return transaction.connection.prepareStatement(sql, shouldReturnGeneratedValues)
}

return super.prepared(transaction, sql)
}

override fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean {
return isEntityIdClientSideGeneratedUUID(column) ||
super.isColumnValuePreferredFromResultSet(column, value)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ open class InsertStatement<Key : Any>(

protected open fun PreparedStatementApi.execInsertFunction(): Pair<Int, ResultSet?> {
val inserted = if (arguments().count() > 1 || isAlwaysBatch) executeBatch().sum() else executeUpdate()
val rs = if (autoIncColumns.isNotEmpty()) {
// According to the `processResults()` method when supportsOnlyIdentifiersInGeneratedKeys is false
// all the columns could be taken from result set
val rs = if (autoIncColumns.isNotEmpty() || !currentDialect.supportsOnlyIdentifiersInGeneratedKeys) {
resultSet
} else {
null
Expand All @@ -205,7 +207,6 @@ open class InsertStatement<Key : Any>(
column.autoIncColumnType?.nextValExpression != null -> currentDialect.supportsSequenceAsGeneratedKeys
column.columnType.isAutoInc -> true
column in nextValExpressionColumns -> currentDialect.supportsSequenceAsGeneratedKeys
column.columnType is EntityIDColumnType<*> -> !currentDialect.supportsOnlyIdentifiersInGeneratedKeys
else -> false
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package org.jetbrains.exposed.sql.statements

import org.jetbrains.exposed.sql.*
import org.jetbrains.exposed.sql.statements.api.PreparedStatementApi
import org.jetbrains.exposed.sql.vendors.H2Dialect
import org.jetbrains.exposed.sql.vendors.H2FunctionProvider
import org.jetbrains.exposed.sql.vendors.MysqlFunctionProvider
import org.jetbrains.exposed.sql.vendors.currentDialect

/**
* Represents the SQL statement that either inserts a new row into a table, or updates the existing row if insertion would violate a unique constraint.
Expand Down Expand Up @@ -49,4 +51,13 @@ open class UpsertStatement<Key : Any>(
return isEntityIdClientSideGeneratedUUID(column) ||
super.isColumnValuePreferredFromResultSet(column, value)
}

override fun prepared(transaction: Transaction, sql: String): PreparedStatementApi {
// We must return values from upsert because returned id could be different depending on insert or upsert happened
if (!currentDialect.supportsOnlyIdentifiersInGeneratedKeys) {
return transaction.connection.prepareStatement(sql, true)
}

return super.prepared(transaction, sql)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -703,4 +703,21 @@ class InsertTests : DatabaseTestsBase() {
}
}
}

@Test
fun testNoAutoIncrementAppliedToCustomStringPrimaryKey() {
val tester = object : IdTable<String>("test_no_auto_increment_table") {
val customId = varchar("custom_id", 128)
override val primaryKey: PrimaryKey = PrimaryKey(customId)
override val id: Column<EntityID<String>> = customId.entityId()
}

withTables(tester) {
val result1 = tester.batchInsert(listOf("custom-id-value")) { username ->
this[tester.customId] = username
}.single()
assertEquals("custom-id-value", result1[tester.id].value)
assertEquals("custom-id-value", result1[tester.customId])
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,9 @@ class UpsertTests : DatabaseTestsBase() {
Words.batchUpsert(
lettersWithDuplicates,
onUpdate = incrementCount,
// PostgresNG throws IndexOutOfBound if shouldReturnGeneratedValues == true
// Related issue in pgjdbc-ng repository: https://github.com/impossibl/pgjdbc-ng/issues/545
shouldReturnGeneratedValues = false,
where = { Words.word inList firstThreeVowels }
) { letter ->
this[Words.word] = letter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ package org.jetbrains.exposed.sql.tests.shared.entities
import org.jetbrains.exposed.dao.*
import org.jetbrains.exposed.dao.id.EntityID
import org.jetbrains.exposed.dao.id.IdTable
import org.jetbrains.exposed.dao.id.IntIdTable
import org.jetbrains.exposed.sql.Column
import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
import org.jetbrains.exposed.sql.tests.currentDialectTest
import org.jetbrains.exposed.sql.tests.shared.assertEquals
import org.jetbrains.exposed.sql.transactions.TransactionManager
import org.jetbrains.exposed.sql.update
import org.junit.Test
import java.util.concurrent.atomic.AtomicInteger

Expand Down Expand Up @@ -69,35 +69,61 @@ class NonAutoIncEntities : DatabaseTestsBase() {
}
}

object NonAutoIncSharedTable : BaseNonAutoIncTable("SharedTable")
object CustomPrimaryKeyColumnTable : IdTable<String>() {
val customId: Column<String> = varchar("customId", 256)
override val primaryKey = PrimaryKey(customId)
override val id: Column<EntityID<String>> = customId.entityId()
}

object AutoIncSharedTable : IntIdTable("SharedTable") {
val b1 = bool("b1")
class CustomPrimaryKeyColumnEntity(id: EntityID<String>) : Entity<String>(id) {
companion object : EntityClass<String, CustomPrimaryKeyColumnEntity>(CustomPrimaryKeyColumnTable)

var customId by CustomPrimaryKeyColumnTable.customId
}

class SharedNonAutoIncEntity(id: EntityID<Int>) : IntEntity(id) {
var bool by NonAutoIncSharedTable.b1
@Test
fun testIdValueIsTheSameAsCustomPrimaryKeyColumn() {
withTables(CustomPrimaryKeyColumnTable) {
val request = CustomPrimaryKeyColumnEntity.new {
customId = "customIdValue"
}

companion object : IntEntityClass<SharedNonAutoIncEntity>(NonAutoIncSharedTable)
assertEquals("customIdValue", request.id.value)
}
}

@Test fun testFlushNonAutoincEntityWithoutDefaultValue() {
withTables(AutoIncSharedTable) {
if (!currentDialectTest.supportsOnlyIdentifiersInGeneratedKeys) {
SharedNonAutoIncEntity.new {
bool = true
}
object RequestsTable : IdTable<String> () {
val requestId = varchar("request_id", 256)
val deleted = bool("deleted")
override val primaryKey: PrimaryKey = PrimaryKey(requestId)
override val id: Column<EntityID<String>> = requestId.entityId()
}

SharedNonAutoIncEntity.new {
bool = false
}
class Request(id: EntityID<String>) : Entity<String>(id) {
companion object : EntityClass<String, Request>(RequestsTable)

var requestId by RequestsTable.requestId
var deleted by RequestsTable.deleted

val entities = flushCache()
override fun delete() {
RequestsTable.update({ RequestsTable.id eq id }) {
it[deleted] = true
}
}
}

assertEquals(2, entities.size)
assertEquals(1, entities[0].id._value)
assertEquals(2, entities[1].id._value)
@Test
fun testAccessEntityIdFromOverrideEntityMethod() {
withTables(RequestsTable) {
val request = Request.new {
requestId = "test1"
deleted = false
}

request.delete()

val updated = Request["test1"]
assertEquals(true, updated.deleted)
}
}
}
Loading