Skip to content

Commit

Permalink
EXPOSED-372 UpsertStatement.resultedValues contains incorrect value (#…
Browse files Browse the repository at this point in the history
…2075)

* EXPOSED-372 UpsertStatement.resultedValues contains incorrect value
  • Loading branch information
obabichevjb committed May 13, 2024
1 parent db4c708 commit fd519d3
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 5 deletions.
2 changes: 2 additions & 0 deletions exposed-core/api/exposed-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,10 @@ fun <T : Table> 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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,22 +100,22 @@ open class InsertStatement<Key : Any>(
}
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<Expression<*>, 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<Column<*>, Any?> = this.values): Map<Column<*>, Any?> {
val result = values.toMutableMap()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,15 @@ open class UpsertStatement<Key : Any>(
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
}
Original file line number Diff line number Diff line change
@@ -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.*
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit fd519d3

Please sign in to comment.