From b28fe483be384aac4b1c7f3641aa8a62b16b1310 Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Sun, 18 Aug 2024 23:43:27 -0400 Subject: [PATCH 1/2] fix: EXPOSED-493 Update with join query throws if WHERE clause present Previous refactoring of UpdateStatement.arguments() to properly order registered arguments in an update-from-join statement did not factor in the possibility that the join target would be a subquery or QueryAlias. If this query itself holds a WHERE clause with arguments, the latter will not be registered when preparing the statement, leading to the same 'parameter not filled' errors as previously. This PR ensures any query arguments are registered just before the rest of the join part additional arguments, at whatever point that may be for each database. Tests for single and multiple joins, and joins with update-where, have been edited to include joins with a subquery. --- .../exposed/sql/statements/UpdateStatement.kt | 1 + .../sql/tests/shared/dml/UpdateTests.kt | 44 ++++++++++++++++++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpdateStatement.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpdateStatement.kt index 02846d1226..91d7a6fe24 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpdateStatement.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/statements/UpdateStatement.kt @@ -82,6 +82,7 @@ open class UpdateStatement(val targetsSet: ColumnSet, val limit: Int?, val where private fun QueryBuilder.registerAdditionalArgs(join: Join) { join.joinParts.forEach { + (it.joinPart as? QueryAlias)?.query?.prepareSQL(this) it.additionalConstraint?.invoke(SqlExpressionBuilder)?.toQueryBuilder(this) } } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt index e404e3d578..04b01c956b 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt @@ -6,11 +6,11 @@ import org.jetbrains.exposed.exceptions.UnsupportedByDialectException import org.jetbrains.exposed.sql.* import org.jetbrains.exposed.sql.tests.DatabaseTestsBase import org.jetbrains.exposed.sql.tests.TestDB +import org.jetbrains.exposed.sql.tests.currentTestDB import org.jetbrains.exposed.sql.tests.shared.assertEquals import org.jetbrains.exposed.sql.tests.shared.expectException import org.jetbrains.exposed.sql.vendors.SQLiteDialect import org.junit.Test -import java.lang.IllegalArgumentException class UpdateTests : DatabaseTestsBase() { private val notSupportLimit by lazy { @@ -92,6 +92,20 @@ class UpdateTests : DatabaseTestsBase() { assertEquals(it[users.name], it[userData.comment]) assertEquals(0, it[userData.value]) } + + val userAlias = users.selectAll().where { users.cityId neq 1 }.alias("u2") + val joinWithSubQuery = userData.innerJoin(userAlias, { userData.user_id }, { userAlias[users.id] }) + joinWithSubQuery.update { + it[userData.value] = 123 + } + + joinWithSubQuery.selectAll().forEach { + assertEquals(123, it[userData.value]) + } + + if (currentTestDB in TestDB.ALL_H2_V1) { // h2_v1 treats 'U2' query as a table/view dependent on USERS + exec("${users.dropStatement().single()} CASCADE") + } } } @@ -108,6 +122,22 @@ class UpdateTests : DatabaseTestsBase() { assertEquals(it[users.name], it[userData.comment]) assertEquals(123, it[userData.value]) } + + val singleJoinQuery = userData.joinQuery( + on = { userData.user_id eq it[users.id] }, + joinPart = { users.selectAll().where { users.cityId neq 1 } } + ) + val doubleJoinQuery = singleJoinQuery.joinQuery( + on = { userData.user_id eq it[users.id] }, + joinPart = { users.selectAll().where { users.name like "%ey" } } + ) + doubleJoinQuery.update { + it[userData.value] = 0 + } + + doubleJoinQuery.selectAll().forEach { + assertEquals(0, it[userData.value]) + } } } @@ -131,7 +161,6 @@ class UpdateTests : DatabaseTestsBase() { } val join = tableA.innerJoin(tableB) - val joinWithConstraint = tableA.innerJoin(tableB, { tableA.id }, { tableB.tableAId }) { tableB.bar eq "foo" } if (testingDb in supportWhere) { join.update({ tableA.foo eq "foo" }) { @@ -141,10 +170,21 @@ class UpdateTests : DatabaseTestsBase() { assertEquals("baz", it[tableB.bar]) } + val joinWithConstraint = tableA.innerJoin(tableB, { tableA.id }, { tableB.tableAId }) { tableB.bar eq "foo" } joinWithConstraint.update({ tableA.foo eq "foo" }) { it[tableB.bar] = "baz" } assertEquals(0, joinWithConstraint.selectAll().count()) + + val subquery = tableA.selectAll().where { tableA.foo eq "foo" }.alias("sq") + val joinWithSubquery = tableB.innerJoin(subquery, { tableB.tableAId }, { subquery[tableA.id] }) + joinWithSubquery.update({ tableB.bar eq "baz" }) { + it[tableB.bar] = "zip" + } + + joinWithSubquery.selectAll().single().also { + assertEquals("zip", it[tableB.bar]) + } } else { expectException { join.update({ tableA.foo eq "foo" }) { From e17ce9cd3e17a826ef569b99c9aeda7d64471ba8 Mon Sep 17 00:00:00 2001 From: Chantal Loncle <82039410+bog-walk@users.noreply.github.com> Date: Tue, 20 Aug 2024 10:53:24 -0400 Subject: [PATCH 2/2] fix: EXPOSED-493 Update with join query throws if WHERE clause present - Break up new tests into separate unit tests - Fix typos in update(where) unsupported exceptions - Rebase from main --- .../org/jetbrains/exposed/sql/vendors/H2.kt | 8 +- .../sql/tests/shared/dml/UpdateTests.kt | 84 ++++++++++--------- 2 files changed, 50 insertions(+), 42 deletions(-) diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt index 074a5b9152..12f1767da5 100644 --- a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt +++ b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/H2.kt @@ -82,9 +82,13 @@ internal object H2FunctionProvider : FunctionProvider() { transaction.throwUnsupportedException("H2 doesn't support WHERE in UPDATE with join clause.") } val tableToUpdate = columnsAndValues.map { it.first.table }.distinct().singleOrNull() - ?: transaction.throwUnsupportedException("H2 supports a join updates with a single table columns to update.") + ?: transaction.throwUnsupportedException( + "H2 doesn't support UPDATE with join clause that uses columns from multiple tables." + ) val joinPart = targets.joinParts.singleOrNull() - ?: transaction.throwUnsupportedException("H2 supports a join updates with only one table to join.") + ?: transaction.throwUnsupportedException( + "H2 doesn't support UPDATE with join clause that uses multiple tables to join." + ) if (joinPart.joinType != JoinType.INNER) { exposedLogger.warn("All tables in UPDATE statement will be joined with inner join") } diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt index 04b01c956b..f506ee1d2b 100644 --- a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt +++ b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/UpdateTests.kt @@ -92,20 +92,6 @@ class UpdateTests : DatabaseTestsBase() { assertEquals(it[users.name], it[userData.comment]) assertEquals(0, it[userData.value]) } - - val userAlias = users.selectAll().where { users.cityId neq 1 }.alias("u2") - val joinWithSubQuery = userData.innerJoin(userAlias, { userData.user_id }, { userAlias[users.id] }) - joinWithSubQuery.update { - it[userData.value] = 123 - } - - joinWithSubQuery.selectAll().forEach { - assertEquals(123, it[userData.value]) - } - - if (currentTestDB in TestDB.ALL_H2_V1) { // h2_v1 treats 'U2' query as a table/view dependent on USERS - exec("${users.dropStatement().single()} CASCADE") - } } } @@ -122,22 +108,6 @@ class UpdateTests : DatabaseTestsBase() { assertEquals(it[users.name], it[userData.comment]) assertEquals(123, it[userData.value]) } - - val singleJoinQuery = userData.joinQuery( - on = { userData.user_id eq it[users.id] }, - joinPart = { users.selectAll().where { users.cityId neq 1 } } - ) - val doubleJoinQuery = singleJoinQuery.joinQuery( - on = { userData.user_id eq it[users.id] }, - joinPart = { users.selectAll().where { users.name like "%ey" } } - ) - doubleJoinQuery.update { - it[userData.value] = 0 - } - - doubleJoinQuery.selectAll().forEach { - assertEquals(0, it[userData.value]) - } } } @@ -175,16 +145,6 @@ class UpdateTests : DatabaseTestsBase() { it[tableB.bar] = "baz" } assertEquals(0, joinWithConstraint.selectAll().count()) - - val subquery = tableA.selectAll().where { tableA.foo eq "foo" }.alias("sq") - val joinWithSubquery = tableB.innerJoin(subquery, { tableB.tableAId }, { subquery[tableA.id] }) - joinWithSubquery.update({ tableB.bar eq "baz" }) { - it[tableB.bar] = "zip" - } - - joinWithSubquery.selectAll().single().also { - assertEquals("zip", it[tableB.bar]) - } } else { expectException { join.update({ tableA.foo eq "foo" }) { @@ -195,6 +155,50 @@ class UpdateTests : DatabaseTestsBase() { } } + @Test + fun testUpdateWithJoinQuery() { + withCitiesAndUsers(exclude = TestDB.ALL_H2_V1 + TestDB.SQLITE) { _, users, userData -> + // single join query using join() + val userAlias = users.selectAll().where { users.cityId neq 1 }.alias("u2") + val joinWithSubQuery = userData.innerJoin(userAlias, { userData.user_id }, { userAlias[users.id] }) + joinWithSubQuery.update { + it[userData.value] = 123 + } + + joinWithSubQuery.selectAll().forEach { + assertEquals(123, it[userData.value]) + } + + if (currentTestDB !in TestDB.ALL_H2) { // does not support either multi-table joins or update(where) + // single join query using join() with update(where) + joinWithSubQuery.update({ userData.comment like "Comment%" }) { + it[userData.value] = 0 + } + + joinWithSubQuery.selectAll().forEach { + assertEquals(0, it[userData.value]) + } + + // multiple join queries using joinQuery() + val singleJoinQuery = userData.joinQuery( + on = { userData.user_id eq it[users.id] }, + joinPart = { users.selectAll().where { users.cityId neq 1 } } + ) + val doubleJoinQuery = singleJoinQuery.joinQuery( + on = { userData.user_id eq it[users.id] }, + joinPart = { users.selectAll().where { users.name like "%ey" } } + ) + doubleJoinQuery.update { + it[userData.value] = 99 + } + + doubleJoinQuery.selectAll().forEach { + assertEquals(99, it[userData.value]) + } + } + } + } + @Test fun `test that column length checked in update `() { val stringTable = object : IntIdTable("StringTable") {