fix: EXPOSED-405 SQLite bugs: Table with custom ID behaves weirdly in…

… DAO and batchInsert (#2119) * fix: EXPOSED-405 SQLite bugs: Table with custom ID behaves weirdly in DAO and batchInsert
JetBrains · Jun 24, 2024 · b2e4883 · b2e4883
1 parent d58e0dd
commit b2e4883
Show file tree

Hide file tree

Showing 7 changed files with 94 additions and 23 deletions.
diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api
@@ -2958,6 +2958,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 {
@@ -3257,6 +3258,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 {

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BatchUpsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/BatchUpsertStatement.kt
@@ -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.
@@ -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)

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/InsertStatement.kt
@@ -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
@@ -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
                 }
             }

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpsertStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpsertStatement.kt
@@ -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.
@@ -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)
+    }
 }
diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/InsertTests.kt
@@ -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])
+        }
+    }
 }
diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpsertTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpsertTests.kt
@@ -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

diff --git a/...sts/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/NonAutoIncEntities.kt b/...sts/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/entities/NonAutoIncEntities.kt
@@ -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
 
@@ -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)
         }
     }
 }