From 3fd9a90b4cf9f21bba7522aecad5db8b3c0140f0 Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Sat, 25 Nov 2023 22:29:25 -0500 Subject: [PATCH 1/2] fix: EXPOSED-226 Upsert fails with all key columns in update If all columns in an upsert block are conflict/key columns and onUpdate is not set, an empty update clause is generated, which throws an SQLException in all databases except MySQL and MariaDB. Including these key columns in the update clause is valid SQL in all databases except Oracle, so filtering the update columns now only happens if the result will not be an empty list. --- .../exposed/sql/vendors/FunctionProvider.kt | 2 +- .../exposed/sql/vendors/PostgreSQL.kt | 3 +- .../exposed/sql/vendors/SQLiteDialect.kt | 3 +- .../sql/tests/shared/dml/UpsertTests.kt | 36 +++++++++++++++++++ 4 files changed, 41 insertions(+), 3 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt index a29b267723..2c0e59c1f8 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/FunctionProvider.kt @@ -539,7 +539,7 @@ abstract class FunctionProvider { val autoIncColumn = table.autoIncColumn val nextValExpression = autoIncColumn?.autoIncColumnType?.nextValExpression val dataColumnsWithoutAutoInc = autoIncColumn?.let { dataColumns - autoIncColumn } ?: dataColumns - val updateColumns = dataColumns.filter { it !in keyColumns } + val updateColumns = dataColumns.filter { it !in keyColumns }.ifEmpty { dataColumns } return with(QueryBuilder(true)) { +"MERGE INTO " diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/PostgreSQL.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/PostgreSQL.kt index 8946649cb6..63fefcb0fa 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/PostgreSQL.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/PostgreSQL.kt @@ -249,7 +249,8 @@ internal object PostgreSQLFunctionProvider : FunctionProvider() { transaction.throwUnsupportedException("UPSERT requires a unique key or constraint as a conflict target") } - val updateColumns = data.unzip().first.filter { it !in keyColumns } + val dataColumns = data.unzip().first + val updateColumns = dataColumns.filter { it !in keyColumns }.ifEmpty { dataColumns } return with(QueryBuilder(true)) { appendInsertToUpsertClause(table, data, transaction) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLiteDialect.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLiteDialect.kt index 31410675b2..90d16627cb 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLiteDialect.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLiteDialect.kt @@ -216,7 +216,8 @@ internal object SQLiteFunctionProvider : FunctionProvider() { } +" DO" - val updateColumns = data.unzip().first.filter { it !in keyColumns } + val dataColumns = data.unzip().first + val updateColumns = dataColumns.filter { it !in keyColumns }.ifEmpty { dataColumns } appendUpdateToUpsertClause(table, updateColumns, onUpdate, transaction, isAliasNeeded = false) where?.let { 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 554f95a621..56d33e7969 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 @@ -14,6 +14,7 @@ import org.jetbrains.exposed.sql.tests.shared.expectException import org.junit.Test import java.util.* import kotlin.properties.Delegates +import kotlin.test.assertNotNull // Upsert implementation does not support H2 version 1 // https://youtrack.jetbrains.com/issue/EXPOSED-30/Phase-Out-Support-for-H2-Version-1.x @@ -86,6 +87,41 @@ class UpsertTests : DatabaseTestsBase() { } } + @Test + fun testUpsertWithAllColumnsInPK() { + val tester = object : Table("tester") { + val userId = varchar("user_id", 32) + val keyId = varchar("key_id", 32) + val keyValue = varchar("key_value", 32) + override val primaryKey = PrimaryKey(userId, keyId, keyValue) + } + + // Oracle explicitly prohibits and throws 'ORA-38104: Columns referenced in the ON Clause cannot be updated' + // All other databases allow all key columns in update clause as valid SQL + withTables(excludeSettings = listOf(TestDB.ORACLE), tester) { testDb -> + excludingH2Version1(testDb) { + val primaryKeyValues = Triple("User A", "Key A", "A") + tester.upsert { + it[userId] = primaryKeyValues.first + it[keyId] = primaryKeyValues.second + it[keyValue] = primaryKeyValues.third + } + // existing row 'updated' to have identical values + tester.upsert { + it[userId] = primaryKeyValues.first + it[keyId] = primaryKeyValues.second + it[keyValue] = primaryKeyValues.third + } + + val result = tester.selectAll().singleOrNull() + assertNotNull(result) + + val resultValues = Triple(result[tester.userId], result[tester.keyId], result[tester.keyValue]) + assertEquals(primaryKeyValues, resultValues) + } + } + } + @Test fun testUpsertWithUniqueIndexConflict() { withTables(Words) { testDb -> From 8665c78b0336238e92639b9d40fce5d5a98a3efe Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Sun, 26 Nov 2023 20:01:21 -0500 Subject: [PATCH 2/2] fix: EXPOSED-226 Upsert fails with only key columns in update Edit test to include Oracle. --- .../sql/tests/shared/dml/UpsertTests.kt | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) 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 56d33e7969..82d97c8841 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.exceptions.ExposedSQLException import org.jetbrains.exposed.exceptions.UnsupportedByDialectException import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.SqlExpressionBuilder.concat @@ -92,32 +93,37 @@ class UpsertTests : DatabaseTestsBase() { val tester = object : Table("tester") { val userId = varchar("user_id", 32) val keyId = varchar("key_id", 32) - val keyValue = varchar("key_value", 32) - override val primaryKey = PrimaryKey(userId, keyId, keyValue) + override val primaryKey = PrimaryKey(userId, keyId) } - // Oracle explicitly prohibits and throws 'ORA-38104: Columns referenced in the ON Clause cannot be updated' - // All other databases allow all key columns in update clause as valid SQL - withTables(excludeSettings = listOf(TestDB.ORACLE), tester) { testDb -> + fun upsertOnlyKeyColumns(values: Pair) { + tester.upsert { + it[userId] = values.first + it[keyId] = values.second + } + } + + withTables(tester) { testDb -> excludingH2Version1(testDb) { - val primaryKeyValues = Triple("User A", "Key A", "A") - tester.upsert { - it[userId] = primaryKeyValues.first - it[keyId] = primaryKeyValues.second - it[keyValue] = primaryKeyValues.third - } - // existing row 'updated' to have identical values - tester.upsert { - it[userId] = primaryKeyValues.first - it[keyId] = primaryKeyValues.second - it[keyValue] = primaryKeyValues.third - } + val primaryKeyValues = Pair("User A", "Key A") + if (testDb == TestDB.ORACLE) { + // Oracle explicitly prohibits using key columns in update clause + // throws 'ORA-38104: Columns referenced in the ON Clause cannot be updated' + expectException { + upsertOnlyKeyColumns(primaryKeyValues) + } + } else { + // insert new row + upsertOnlyKeyColumns(primaryKeyValues) + // 'update' existing row to have identical values + upsertOnlyKeyColumns(primaryKeyValues) - val result = tester.selectAll().singleOrNull() - assertNotNull(result) + val result = tester.selectAll().singleOrNull() + assertNotNull(result) - val resultValues = Triple(result[tester.userId], result[tester.keyId], result[tester.keyValue]) - assertEquals(primaryKeyValues, resultValues) + val resultValues = Pair(result[tester.userId], result[tester.keyId]) + assertEquals(primaryKeyValues, resultValues) + } } } }