fix: EXPOSED-551 [SQL Server] Update from join with limit throws synt…

…ax exception (#2244) * fix: EXPOSED-551 [SQL Server] Update from join with limit throws syntax exception Attempting to perform an update from a join, with a provided limit, throws a syntax exception if either SQL Server or MySQL is used. The former because of an error in generated SQL and the latter because it is not allowed (when it should fail early).
JetBrains · Sep 19, 2024 · d3c60f2 · d3c60f2
1 parent 888ffd8
commit d3c60f2
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 72 deletions.
diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MariaDBDialect.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MariaDBDialect.kt
@@ -24,6 +24,17 @@ internal object MariaDBFunctionProvider : MysqlFunctionProvider() {
         append("LOCATE(\'", substring, "\',", expr, ")")
     }
 
+    override fun update(
+        targets: Join,
+        columnsAndValues: List<Pair<Column<*>, Any?>>,
+        limit: Int?,
+        where: Op<Boolean>?,
+        transaction: Transaction
+    ): String {
+        val sql = super.update(targets, columnsAndValues, null, where, transaction)
+        return if (limit != null) "$sql LIMIT $limit" else sql
+    }
+
     override fun explain(
         analyze: Boolean,
         options: String?,

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/MysqlDialect.kt
@@ -219,21 +219,26 @@ internal open class MysqlFunctionProvider : FunctionProvider() {
         limit: Int?,
         where: Op<Boolean>?,
         transaction: Transaction
-    ): String = with(QueryBuilder(true)) {
-        +"UPDATE "
-        targets.describe(transaction, this)
-        +" SET "
-        columnsAndValues.appendTo(this) { (col, value) ->
-            append("${transaction.fullIdentity(col)}=")
-            registerArgument(col, value)
+    ): String {
+        if (limit != null) {
+            transaction.throwUnsupportedException("MySQL doesn't support LIMIT in UPDATE with join clause.")
         }
 
-        where?.let {
-            +" WHERE "
-            +it
+        return with(QueryBuilder(true)) {
+            +"UPDATE "
+            targets.describe(transaction, this)
+            +" SET "
+            columnsAndValues.appendTo(this) { (col, value) ->
+                append("${transaction.fullIdentity(col)}=")
+                registerArgument(col, value)
+            }
+
+            where?.let {
+                +" WHERE "
+                +it
+            }
+            toString()
         }
-        limit?.let { +" LIMIT $it" }
-        toString()
     }
 
     override fun upsert(

diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLServerDialect.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLServerDialect.kt
@@ -212,7 +212,7 @@ internal object SQLServerFunctionProvider : FunctionProvider() {
 
         targets.checkJoinTypes(StatementType.UPDATE)
         if (limit != null) {
-            +"UPDATE TOP($limit)"
+            +"UPDATE TOP($limit) "
         } else {
             +"UPDATE "
         }
@@ -229,7 +229,6 @@ internal object SQLServerFunctionProvider : FunctionProvider() {
             +" AND "
             +it
         }
-        limit?.let { +" LIMIT $it" }
         toString()
     }
 

diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/DeleteTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/DeleteTests.kt
@@ -9,6 +9,7 @@ import org.jetbrains.exposed.sql.SqlExpressionBuilder.like
 import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
 import org.jetbrains.exposed.sql.tests.TestDB
 import org.jetbrains.exposed.sql.tests.currentDialectTest
+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.H2Dialect
@@ -19,18 +20,9 @@ import kotlin.test.assertTrue
 import kotlin.test.expect
 
 class DeleteTests : DatabaseTestsBase() {
-    private val notSupportLimit by lazy {
-        val exclude = arrayListOf(
-            TestDB.POSTGRESQL,
-            TestDB.POSTGRESQLNG,
-            TestDB.ORACLE,
-            TestDB.H2_V2_PSQL,
-            TestDB.H2_V2_ORACLE
-        )
-        if (!SQLiteDialect.ENABLE_UPDATE_DELETE_LIMIT) {
-            exclude.add(TestDB.SQLITE)
-        }
-        exclude
+    private val limitNotSupported by lazy {
+        val extra = setOf(TestDB.SQLITE).takeUnless { SQLiteDialect.ENABLE_UPDATE_DELETE_LIMIT }.orEmpty()
+        TestDB.ALL_POSTGRES_LIKE + TestDB.ALL_ORACLE_LIKE + extra
     }
 
     @Test
@@ -82,24 +74,18 @@ class DeleteTests : DatabaseTestsBase() {
     }
 
     @Test
-    fun testDeleteWithLimitAndOffset01() {
-        withCitiesAndUsers(exclude = notSupportLimit) { _, _, userData ->
-            userData.deleteWhere(limit = 1) { userData.value eq 20 }
-            userData.select(userData.user_id, userData.value).where { userData.value eq 20 }.let {
-                assertEquals(1L, it.count())
-                val expected = if (currentDialectTest is H2Dialect) "smth" else "eugene"
-                assertEquals(expected, it.single()[userData.user_id])
-            }
-        }
-    }
-
-    @Test
-    fun testDeleteWithLimit02() {
-        val dialects = TestDB.entries - notSupportLimit
-        withCitiesAndUsers(dialects) { _, _, userData ->
-            expectException<UnsupportedByDialectException> {
-                userData.deleteWhere(limit = 1) {
-                    userData.value eq 20
+    fun testDeleteWithLimit() {
+        withCitiesAndUsers { _, _, userData ->
+            if (currentTestDB in limitNotSupported) {
+                expectException<UnsupportedByDialectException> {
+                    userData.deleteWhere(limit = 1) { userData.value eq 20 }
+                }
+            } else {
+                userData.deleteWhere(limit = 1) { userData.value eq 20 }
+                userData.select(userData.user_id, userData.value).where { userData.value eq 20 }.let {
+                    assertEquals(1L, it.count())
+                    val expected = if (currentDialectTest is H2Dialect) "smth" else "eugene"
+                    assertEquals(expected, it.single()[userData.user_id])
                 }
             }
         }

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
@@ -11,15 +11,12 @@ 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 kotlin.test.assertTrue
 
 class UpdateTests : DatabaseTestsBase() {
-    private val notSupportLimit by lazy {
-        val exclude = TestDB.ALL_POSTGRES_LIKE
-        if (!SQLiteDialect.ENABLE_UPDATE_DELETE_LIMIT) {
-            exclude + TestDB.SQLITE
-        } else {
-            exclude
-        }
+    private val limitNotSupported by lazy {
+        val extra = setOf(TestDB.SQLITE).takeUnless { SQLiteDialect.ENABLE_UPDATE_DELETE_LIMIT }.orEmpty()
+        TestDB.ALL_POSTGRES_LIKE + extra
     }
 
     @Test
@@ -40,30 +37,26 @@ class UpdateTests : DatabaseTestsBase() {
     }
 
     @Test
-    fun testUpdateWithLimit01() {
-        withCitiesAndUsers(exclude = notSupportLimit) { _, users, _ ->
-            val aNames = users.select(users.name).where { users.id like "a%" }.map { it[users.name] }
-            assertEquals(2, aNames.size)
-
-            users.update({ users.id like "a%" }, 1) {
-                it[users.id] = "NewName"
-            }
-
-            val unchanged = users.select(users.name).where { users.id like "a%" }.count()
-            val changed = users.select(users.name).where { users.id eq "NewName" }.count()
-            assertEquals(1, unchanged)
-            assertEquals(1, changed)
-        }
-    }
+    fun testUpdateWithLimit() {
+        withCitiesAndUsers { _, users, _ ->
+            if (currentTestDB in limitNotSupported) {
+                expectException<UnsupportedByDialectException> {
+                    users.update({ users.id like "a%" }, limit = 1) {
+                        it[users.id] = "NewName"
+                    }
+                }
+            } else {
+                val aNames = users.select(users.name).where { users.id like "a%" }.map { it[users.name] }
+                assertEquals(2, aNames.size)
 
-    @Test
-    fun testUpdateWithLimit02() {
-        val dialects = TestDB.entries - notSupportLimit
-        withCitiesAndUsers(dialects) { _, users, _ ->
-            expectException<UnsupportedByDialectException> {
-                users.update({ users.id like "a%" }, 1) {
+                users.update({ users.id like "a%" }, limit = 1) {
                     it[users.id] = "NewName"
                 }
+
+                val unchanged = users.select(users.name).where { users.id like "a%" }.count()
+                val changed = users.select(users.name).where { users.id eq "NewName" }.count()
+                assertEquals(1, unchanged)
+                assertEquals(1, changed)
             }
         }
     }
@@ -95,6 +88,27 @@ class UpdateTests : DatabaseTestsBase() {
         }
     }
 
+    @Test
+    fun testUpdateWithJoinAndLimit() {
+        val supportsUpdateWithJoinAndLimit = TestDB.ALL_MARIADB + TestDB.ORACLE + TestDB.SQLSERVER
+        withCitiesAndUsers(exclude = TestDB.ALL - supportsUpdateWithJoinAndLimit) { _, users, userData ->
+            val join = users.innerJoin(userData)
+
+            val maxToUpdate = 2
+            assertTrue { join.selectAll().count() > maxToUpdate }
+
+            val updatedValue = 123
+            val valueQuery = join.selectAll().where { userData.value eq updatedValue }
+            assertEquals(0, valueQuery.count())
+
+            join.update(limit = maxToUpdate) {
+                it[userData.value] = updatedValue
+            }
+
+            assertEquals(maxToUpdate, valueQuery.count().toInt())
+        }
+    }
+
     @Test
     fun testUpdateWithMultipleJoins() {
         withCitiesAndUsers(exclude = TestDB.ALL_H2 + TestDB.SQLITE) { cities, users, userData ->