From fd519d37e3213e574d6fc920a115843a55f48937 Mon Sep 17 00:00:00 2001 From: obabichevjb <166523824+obabichevjb@users.noreply.github.com> Date: Mon, 13 May 2024 12:22:12 +0200 Subject: [PATCH] EXPOSED-372 UpsertStatement.resultedValues contains incorrect value (#2075) * EXPOSED-372 UpsertStatement.resultedValues contains incorrect value --- exposed-core/api/exposed-core.api | 2 ++ .../org/jetbrains/exposed/sql/Queries.kt | 4 +++ .../exposed/sql/statements/InsertStatement.kt | 10 +++---- .../exposed/sql/statements/UpsertStatement.kt | 11 +++++++ .../sql/tests/shared/dml/UpsertTests.kt | 29 +++++++++++++++++++ 5 files changed, 51 insertions(+), 5 deletions(-) diff --git a/exposed-core/api/exposed-core.api b/exposed-core/api/exposed-core.api index d16b31f4f5..2f2fbb7594 100644 --- a/exposed-core/api/exposed-core.api +++ b/exposed-core/api/exposed-core.api @@ -2993,6 +2993,7 @@ public class org/jetbrains/exposed/sql/statements/InsertStatement : org/jetbrain public final fun getOrNull (Lorg/jetbrains/exposed/sql/Column;)Ljava/lang/Object; public final fun getResultedValues ()Ljava/util/List; public final fun getTable ()Lorg/jetbrains/exposed/sql/Table; + protected fun isColumnValuePreferredFromResultSet (Lorg/jetbrains/exposed/sql/Column;Ljava/lang/Object;)Z public final fun isIgnore ()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; @@ -3169,6 +3170,7 @@ public class org/jetbrains/exposed/sql/statements/UpsertStatement : org/jetbrain public final fun getOnUpdate ()Ljava/util/List; public final fun getOnUpdateExclude ()Ljava/util/List; 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; } diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Queries.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Queries.kt index 8792bab70a..e58734ff41 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Queries.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Queries.kt @@ -466,6 +466,10 @@ fun T.updateReturning( * * **Note:** Vendors that do not support this operation directly implement the standard MERGE USING command. * + * **Note:** Currently, the `upsert()` function might return an incorrect auto-generated ID (such as a UUID) if it performs an update. + * In this case, it returns a new auto-generated ID instead of the ID of the updated row. + * Postgres should not be affected by this issue as it implicitly returns the IDs of updated rows. + * * @param keys (optional) Columns to include in the condition that determines a unique constraint match. * If no columns are provided, primary keys will be used. If the table does not have any primary keys, the first unique index will be attempted. * @param onUpdate List of pairs of specific columns to update and the expressions to update them with. 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 index aeb5a05081..4977859f37 100644 --- 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 @@ -100,22 +100,22 @@ open class InsertStatement( } pairs.forEach { (col, value) -> if (value != DefaultValueMarker) { - if (col.columnType.isAutoInc || value is NextVal<*>) { + if (isColumnValuePreferredFromResultSet(col, value)) { map.getOrPut(col) { value } } else { map[col] = value } } } - -// pairs.filter{ it.second != DefaultValueMarker }.forEach { (col, value) -> -// map.getOrPut(col){ value } -// } } @Suppress("UNCHECKED_CAST") return autoGeneratedKeys.map { ResultRow.createAndFillValues(it as Map, Any?>) } } + protected open fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean { + return column.columnType.isAutoInc || value is NextVal<*> + } + @Suppress("NestedBlockDepth") protected open fun valuesAndDefaults(values: Map, Any?> = this.values): Map, Any?> { val result = values.toMutableMap() 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 index b8133d5c4b..83a6e445dd 100644 --- 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 @@ -48,4 +48,15 @@ open class UpsertStatement( builder.args } ?: emptyList() } + + override fun isColumnValuePreferredFromResultSet(column: Column<*>, value: Any?): Boolean { + return isEntityIdClientSideGeneratedUUID(column) || + super.isColumnValuePreferredFromResultSet(column, value) + } + + private fun isEntityIdClientSideGeneratedUUID(column: Column<*>) = + (column.columnType as? EntityIDColumnType<*>) + ?.idColumn + ?.takeIf { it.columnType is UUIDColumnType } + ?.defaultValueFun != null } 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 index abd30c14c5..28f9b484ac 100644 --- 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 @@ -1,6 +1,7 @@ package org.jetbrains.exposed.sql.tests.shared.dml import org.jetbrains.exposed.dao.id.IntIdTable +import org.jetbrains.exposed.dao.id.UUIDTable import org.jetbrains.exposed.exceptions.ExposedSQLException import org.jetbrains.exposed.exceptions.UnsupportedByDialectException import org.jetbrains.exposed.sql.* @@ -626,6 +627,34 @@ class UpsertTests : DatabaseTestsBase() { } } + @Test + fun testUpsertWithUUIDPrimaryKey() { + val tester = object : UUIDTable("upsert_test", "id") { + val key = integer("test_key").uniqueIndex() + val value = text("test_value") + } + + // At present, only Postgres returns the correct UUID directly from the result set. + // For other databases incorrect ID is returned from the 'upsert' command. + withTables(excludeSettings = TestDB.entries - listOf(TestDB.POSTGRESQL, TestDB.POSTGRESQLNG), tester) { testDb -> + val insertId = tester.insertAndGetId { + it[key] = 1 + it[value] = "one" + } + + val upsertId = tester.upsert( + keys = arrayOf(tester.key), + onUpdateExclude = listOf(tester.id), + ) { + it[key] = 1 + it[value] = "two" + }.resultedValues!!.first()[tester.id] + + assertEquals(insertId, upsertId) + assertEquals("two", tester.selectAll().where { tester.id eq insertId }.first()[tester.value]) + } + } + private object AutoIncTable : Table("auto_inc_table") { val id = integer("id").autoIncrement() val name = varchar("name", 64)