fix!: EXPOSED-569 groupConcat uses wrong SQLite syntax & ignores DIST…

…INCT in Oracle & SQL Server (#2257)

* fix!: EXPOSED-569 groupConcat uses wrong SQLite syntax & ignores DISTINCT in
 Oracle & SQL Server

- SQLite syntax now uses correct separator syntax and allows ORDER BY clause
- Oracle & SQL Server now fail early with unsupported by dialect exception if
distinct = true (before it would just be ignored by SQL builder).

documentation-website/Writerside/topics/Breaking-Changes.md

@@ -1,5 +1,9 @@
 # Breaking Changes
 
+## 0.56.0
+* If the `distinct` parameter of `groupConcat()` is set to `true`, when using Oracle or SQL Server, this will now fail early with an
+  `UnsupportedByDialectException`. Previously, the setting would be ignored and SQL function generation would not include a `DISTINCT` clause.
+
 ## 0.55.0
 * The `DeleteStatement` property `table` is now deprecated in favor of `targetsSet`, which holds a `ColumnSet` that may be a `Table` or `Join`.
   This enables the use of the new `Join.delete()` function, which performs a delete operation on a specific table from the join relation.

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/SQLExpressionBuilder.kt

@@ -29,9 +29,9 @@ fun <T : String?> Expression<T>.upperCase(): UpperCase<T> = UpperCase(this)
 /**
  * Concatenates all non-null input values of each group from [this] string expression, separated by [separator].
  *
- * When [distinct] is set to `true`, duplicate values will be eliminated.
- * [orderBy] can be used to sort values in the concatenated string.
- *
+ * @param separator The separator to use between concatenated values. If left `null`, the database default will be used.
+ * @param distinct If set to `true`, duplicate values will be eliminated.
+ * @param orderBy If specified, values will be sorted in the concatenated string.
  * @sample org.jetbrains.exposed.sql.tests.shared.dml.GroupByTests.testGroupConcat
  */
 fun <T : String?> Expression<T>.groupConcat(
@@ -43,9 +43,9 @@ fun <T : String?> Expression<T>.groupConcat(
 /**
  * Concatenates all non-null input values of each group from [this] string expression, separated by [separator].
  *
- * When [distinct] is set to `true`, duplicate values will be eliminated.
- * [orderBy] can be used to sort values in the concatenated string by one or more expressions.
- *
+ * @param separator The separator to use between concatenated values. If left `null`, the database default will be used.
+ * @param distinct If set to `true`, duplicate values will be eliminated.
+ * @param orderBy If specified, values will be sorted in the concatenated string.
  * @sample org.jetbrains.exposed.sql.tests.shared.dml.GroupByTests.testGroupConcat
  */
 fun <T : String?> Expression<T>.groupConcat(

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/OracleDialect.kt

@@ -120,17 +120,20 @@ internal object OracleFunctionProvider : FunctionProvider() {
         expr: GroupConcat<T>,
         queryBuilder: QueryBuilder
     ): Unit = queryBuilder {
-        if (expr.orderBy.size != 1) {
-            TransactionManager.current().throwUnsupportedException("SQLServer supports only single column in ORDER BY clause in LISTAGG")
+        val tr = TransactionManager.current()
+        if (expr.distinct) tr.throwUnsupportedException("Oracle doesn't support DISTINCT in LISTAGG")
+        if (expr.orderBy.size > 1) {
+            tr.throwUnsupportedException("Oracle supports only single column in ORDER BY clause in LISTAGG")
         }
         append("LISTAGG(")
         append(expr.expr)
         expr.separator?.let {
             append(", '$it'")
         }
-        append(") WITHIN GROUP (ORDER BY ")
-        val (col, order) = expr.orderBy.single()
-        append(col, " ", order.name, ")")
+        +")"
+        expr.orderBy.singleOrNull()?.let { (col, order) ->
+            append(" WITHIN GROUP (ORDER BY ", col, " ", order.name, ")")
+        }
     }
 
     override fun <T : String?> locate(

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLServerDialect.kt

@@ -95,8 +95,9 @@ internal object SQLServerFunctionProvider : FunctionProvider() {
     override fun <T : String?> groupConcat(expr: GroupConcat<T>, queryBuilder: QueryBuilder) {
         val tr = TransactionManager.current()
         return when {
-            expr.separator == null -> tr.throwUnsupportedException("SQLServer requires explicit separator in STRING_AGG.")
-            expr.orderBy.size > 1 -> tr.throwUnsupportedException("SQLServer supports only single column in ORDER BY clause in STRING_AGG.")
+            expr.separator == null -> tr.throwUnsupportedException("SQL Server requires explicit separator in STRING_AGG")
+            expr.distinct -> tr.throwUnsupportedException("SQL Server doesn't support DISTINCT in STRING_AGG")
+            expr.orderBy.size > 1 -> tr.throwUnsupportedException("SQL Server supports only single column in ORDER BY clause in STRING_AGG")
             else -> queryBuilder {
                 append("STRING_AGG(")
                 append(expr.expr)

exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/vendors/SQLiteDialect.kt

@@ -46,11 +46,21 @@ internal object SQLiteFunctionProvider : FunctionProvider() {
     }
 
     override fun <T : String?> groupConcat(expr: GroupConcat<T>, queryBuilder: QueryBuilder) {
-        val tr = TransactionManager.current()
-        return when {
-            expr.orderBy.isNotEmpty() -> tr.throwUnsupportedException("SQLite doesn't support ORDER BY in GROUP_CONCAT function.")
-            expr.distinct -> tr.throwUnsupportedException("SQLite doesn't support DISTINCT in GROUP_CONCAT function.")
-            else -> super.groupConcat(expr, queryBuilder) // .replace(" SEPARATOR ", ", ")
+        if (expr.distinct) {
+            TransactionManager.current().throwUnsupportedException("SQLite doesn't support DISTINCT in GROUP_CONCAT function")
+        }
+        queryBuilder {
+            +"GROUP_CONCAT("
+            +expr.expr
+            expr.separator?.let {
+                +", '$it'"
+            }
+            if (expr.orderBy.isNotEmpty()) {
+                expr.orderBy.appendTo(prefix = " ORDER BY ") { (expression, sortOrder) ->
+                    currentDialect.dataTypeProvider.precessOrderByClause(this, expression, sortOrder)
+                }
+            }
+            +")"
         }
     }
 

exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/GroupByTests.kt

@@ -3,7 +3,6 @@ package org.jetbrains.exposed.sql.tests.shared.dml
 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.currentDialectTest
 import org.jetbrains.exposed.sql.tests.shared.assertEquals
 import org.jetbrains.exposed.sql.vendors.*
@@ -158,7 +157,7 @@ class GroupByTests : DatabaseTestsBase() {
 
     @Test
     fun testGroupConcat() {
-        withCitiesAndUsers(exclude = listOf(TestDB.SQLITE)) { cities, users, _ ->
+        withCitiesAndUsers { cities, users, _ ->
             fun <T : String?> GroupConcat<T>.checkExcept(vararg dialects: VendorDialect.DialectNameProvider, assert: (Map<String, String?>) -> Unit) {
                 try {
                     val result = cities.leftJoin(users)
@@ -179,29 +178,30 @@ class GroupByTests : DatabaseTestsBase() {
                 }
             }
 
-            users.name.groupConcat().checkExcept(PostgreSQLDialect, PostgreSQLNGDialect, SQLServerDialect, OracleDialect) {
+            // separator must be specified by PostgreSQL and SQL Server
+            users.name.groupConcat().checkExcept(PostgreSQLDialect, PostgreSQLNGDialect, SQLServerDialect) {
                 assertEquals(3, it.size)
             }
 
-            users.name.groupConcat(separator = ", ").checkExcept(OracleDialect) {
+            users.name.groupConcat(separator = ", ").checkExcept {
                 assertEquals(3, it.size)
                 assertEquals("Andrey", it["St. Petersburg"])
                 when (currentDialectTest) {
-                    is MariaDBDialect -> assertEquals(true, it["Munich"] in listOf("Sergey, Eugene", "Eugene, Sergey"))
+                    // return order is arbitrary if no ORDER BY is specified
+                    is MariaDBDialect, is SQLiteDialect -> assertTrue(it["Munich"] in listOf("Sergey, Eugene", "Eugene, Sergey"))
                     is MysqlDialect, is SQLServerDialect -> assertEquals("Eugene, Sergey", it["Munich"])
                     else -> assertEquals("Sergey, Eugene", it["Munich"])
                 }
 
                 assertNull(it["Prague"])
             }
 
-            users.name.groupConcat(separator = " | ", distinct = true).checkExcept(OracleDialect) {
+            users.name.groupConcat(separator = " | ", distinct = true).checkExcept(OracleDialect, SQLiteDialect, SQLServerDialect) {
                 assertEquals(3, it.size)
                 assertEquals("Andrey", it["St. Petersburg"])
                 when (currentDialectTest) {
                     is MariaDBDialect -> assertEquals(true, it["Munich"] in listOf("Sergey | Eugene", "Eugene | Sergey"))
-                    is MysqlDialect, is SQLServerDialect, is PostgreSQLDialect ->
-                        assertEquals("Eugene | Sergey", it["Munich"])
+                    is MysqlDialect, is PostgreSQLDialect -> assertEquals("Eugene | Sergey", it["Munich"])
                     is H2Dialect -> {
                         if (currentDialect.h2Mode == H2Dialect.H2CompatibilityMode.SQLServer) {
                             assertEquals("Sergey | Eugene", it["Munich"])