Review issues: prevent using both withDistinct and withDistinctOn, fi…

…x Query.copy()
JetBrains · Oct 17, 2024 · 1890d3b · 1890d3b
1 parent 667fe1d
commit 1890d3b
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 1 deletion.
diff --git a/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Query.kt b/exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Query.kt
@@ -69,6 +69,7 @@ open class Query(override var set: FieldSet, where: Op<Boolean>?) : AbstractQuer
     override fun copyTo(other: Query) {
         super.copyTo(other)
         other.distinct = distinct
+        other.distinctOn = distinctOn
         other.groupedByColumns = groupedByColumns.toMutableList()
         other.having = having
         other.forUpdate = forUpdate
@@ -86,6 +87,9 @@ open class Query(override var set: FieldSet, where: Op<Boolean>?) : AbstractQuer
     }
 
     override fun withDistinct(value: Boolean): Query = apply {
+        if (value) {
+            require(distinctOn == null) { "DISTINCT cannot be used with the DISTINCT ON modifier. Only one of them should be applied."  }
+        }
         distinct = value
     }
 
@@ -99,6 +103,7 @@ open class Query(override var set: FieldSet, where: Op<Boolean>?) : AbstractQuer
      * @return The current `Query` instance with the `DISTINCT ON` clause applied.
      */
     fun withDistinctOn(vararg columns: Column<*>): Query = apply {
+        require(!distinct) { "DISTINCT ON cannot be used with the DISTINCT modifier. Only one of them should be applied."  }
         distinctOn = (distinctOn ?: emptyList()) + columns
     }
 
@@ -113,6 +118,7 @@ open class Query(override var set: FieldSet, where: Op<Boolean>?) : AbstractQuer
      * @return The current `Query` instance with the `DISTINCT ON` clause and reordering applied.
      */
     fun withDistinctOn(vararg columns: Pair<Column<*>, SortOrder>): Query = apply {
+        require(!distinct) { "DISTINCT ON cannot be used with the DISTINCT modifier. Only one of them should be applied."  }
         @Suppress("SpreadOperator")
         withDistinctOn(*columns.map { it.first }.toTypedArray())
         return orderBy(*columns)

diff --git a/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/DistinctOnTests.kt b/exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/DistinctOnTests.kt
@@ -1,23 +1,28 @@
 package org.jetbrains.exposed.sql.tests.shared.dml
 
 import org.jetbrains.exposed.dao.id.IntIdTable
+import org.jetbrains.exposed.exceptions.UnsupportedByDialectException
 import org.jetbrains.exposed.sql.SortOrder
 import org.jetbrains.exposed.sql.batchInsert
 import org.jetbrains.exposed.sql.selectAll
 import org.jetbrains.exposed.sql.tests.DatabaseTestsBase
 import org.jetbrains.exposed.sql.tests.TestDB
 import org.jetbrains.exposed.sql.tests.shared.assertEqualLists
+import org.jetbrains.exposed.sql.tests.shared.expectException
 import org.junit.Test
 
 class DistinctOnTests : DatabaseTestsBase() {
+
+    private val distinctOnSupportedDb = TestDB.ALL_POSTGRES + TestDB.ALL_H2
+
     @Test
     fun testDistinctOn() {
         val tester = object : IntIdTable("distinct_function_test") {
             val value1 = integer("value1")
             val value2 = integer("value2")
         }
 
-        withTables(excludeSettings = TestDB.ALL - TestDB.ALL_POSTGRES - TestDB.ALL_H2, tester) {
+        withTables(excludeSettings = TestDB.ALL - distinctOnSupportedDb, tester) {
             tester.batchInsert(
                 listOf(
                     listOf(1, 1), listOf(1, 2), listOf(1, 2),
@@ -54,4 +59,25 @@ class DistinctOnTests : DatabaseTestsBase() {
             assertEqualLists(distinctBoth, distinctSequential)
         }
     }
+
+    @Test
+    fun testExceptions() {
+        val tester = object : IntIdTable("distinct_function_test") {
+            val value = integer("value1")
+        }
+
+        withTables(excludeSettings = TestDB.ALL - distinctOnSupportedDb, tester) {
+            val query1 = tester.selectAll()
+                .withDistinct()
+            expectException<IllegalArgumentException> {
+                query1.withDistinctOn(tester.value)
+            }
+
+            val query2 = tester.selectAll()
+                .withDistinctOn(tester.value)
+            expectException<IllegalArgumentException> {
+                query2.withDistinct()
+            }
+        }
+    }
 }