Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: EXPOSED-560 Support DISTINCT ON from Postgres #2275

Merged
merged 4 commits into from
Oct 30, 2024
Merged

feat: EXPOSED-560 Support DISTINCT ON from Postgres #2275

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7a67160
feat: EXPOSED-560 Support DISTINCT ON from Postgres
obabichevjb Oct 15, 2024
667fe1d
Update exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Query.kt
obabichevjb Oct 15, 2024
7cc0529
Review issues: prevent using both withDistinct and withDistinctOn, fi…
obabichevjb Oct 17, 2024
4f41be1
Review issues: add test with empty distinctOn, update query so it doe…
obabichevjb Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion documentation-website/Writerside/topics/DSL-CRUD-operations.topic
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,28 @@

<code-block lang="kotlin">
val directors =
StarWarsFilms.select(StarWarsFilms.director).where { StarWarsFilms.sequelId less 5 }.withDistinct()
StarWarsFilms.select(StarWarsFilms.director)
.where { StarWarsFilms.sequelId less 5 }.withDistinct()
.map {
it[StarWarsFilms.director]
}
</code-block>

<p>Some SQL dialects, such as PostgreSQL and H2, also support the <code>DISTINCT ON</code> clause.
You can use this clause with the <code>withDistinctOn()</code> function:</p>

<code-block lang="kotlin">
val directors =
StarWarsFilms.select(StarWarsFilms.director, StarWarsFilms.name)
.withDistinctOn(StarWarsFilms.director)
.orderBy(
StarWarsFilms.director to SortOrder.ASC,
StarWarsFilms.name to SortOrder.ASC
)
.map {
it[StarWarsFilms.name]
}
</code-block>
</chapter>
<chapter id="selectAll">
<title><code>selectAll</code></title>
Expand Down
4 changes: 4 additions & 0 deletions exposed-core/api/exposed-core.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1905,6 +1905,7 @@ public class org/jetbrains/exposed/sql/Query : org/jetbrains/exposed/sql/Abstrac
public synthetic fun forUpdate (Lorg/jetbrains/exposed/sql/vendors/ForUpdateOption;)Lorg/jetbrains/exposed/sql/SizedIterable;
public final fun getComments ()Ljava/util/Map;
public final fun getDistinct ()Z
public final fun getDistinctOn ()Ljava/util/List;
public final fun getGroupedByColumns ()Ljava/util/List;
public final fun getHaving ()Lorg/jetbrains/exposed/sql/Op;
protected fun getQueryToExecute ()Lorg/jetbrains/exposed/sql/statements/Statement;
Expand All @@ -1918,11 +1919,14 @@ public class org/jetbrains/exposed/sql/Query : org/jetbrains/exposed/sql/Abstrac
public synthetic fun notForUpdate ()Lorg/jetbrains/exposed/sql/SizedIterable;
public fun prepareSQL (Lorg/jetbrains/exposed/sql/QueryBuilder;)Ljava/lang/String;
protected final fun setDistinct (Z)V
protected final fun setDistinctOn (Ljava/util/List;)V
public fun setSet (Lorg/jetbrains/exposed/sql/FieldSet;)V
public final fun where (Lkotlin/jvm/functions/Function1;)Lorg/jetbrains/exposed/sql/Query;
public final fun where (Lorg/jetbrains/exposed/sql/Op;)Lorg/jetbrains/exposed/sql/Query;
public synthetic fun withDistinct (Z)Lorg/jetbrains/exposed/sql/AbstractQuery;
public fun withDistinct (Z)Lorg/jetbrains/exposed/sql/Query;
public final fun withDistinctOn ([Lkotlin/Pair;)Lorg/jetbrains/exposed/sql/Query;
public final fun withDistinctOn ([Lorg/jetbrains/exposed/sql/Column;)Lorg/jetbrains/exposed/sql/Query;
}

public final class org/jetbrains/exposed/sql/Query$CommentPosition : java/lang/Enum {
Expand Down
47 changes: 47 additions & 0 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Query.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,15 @@ open class Query(override var set: FieldSet, where: Op<Boolean>?) : AbstractQuer
var distinct: Boolean = false
protected set

/**
* List of columns on which the query should be distinct.
*
* This parameter specifies columns for the `DISTINCT ON` clause, which allows selecting distinct rows based on
* the specified columns and is supported by some SQL dialects (e.g., PostgreSQL, H2).
*/
var distinctOn: List<Column<*>>? = null
protected set

/** The stored list of columns for a `GROUP BY` clause in this `SELECT` query. */
var groupedByColumns: List<Expression<*>> = mutableListOf()
private set
Expand Down Expand Up @@ -60,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
Expand All @@ -77,9 +87,43 @@ 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
}

/**
* Specifies that the `SELECT` query should retrieve distinct results based on the given list of columns.
*
* This method can be used to set a `DISTINCT ON` clause for the query, which is supported by some SQL dialects
* (e.g., PostgreSQL, H2). The resulting query will retrieve rows that are distinct based on the specified columns.
*
* @param columns The columns to apply the `DISTINCT ON` clause.
* @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
}

/**
* Specifies that the `SELECT` query should retrieve distinct results based on the given list of columns with sort orders.
* This method sets a `DISTINCT ON` clause and may reorder the results as indicated.
*
* This method can be used to set a `DISTINCT ON` clause for the query, which is supported by some SQL dialects
* (e.g., PostgreSQL, H2), along with an `ORDER BY` clause for the specified columns.
*
* @param columns The columns and their sort orders to apply the `DISTINCT ON` clause.
* @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)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(minor) with this implementation it would currently be allowed to -- arguably unintentionally -- add an entry to orderByExpressions twice for a DISTINCT ON column (since that attribute is a list and not a distinct set), e.g.

table
  .selectAll()
  .withDistinctOn(table.col to SortOrder.ASC)
  .orderBy(table.col, SortOrder.DESC)

postgresql allows for this and uses the last specified sorting predicate specified for a column but i'm not sure if you're intentionally supporting this use case; i don't personally know if the behaviour of having a column mentioned twice in ORDER BY is consistent across all sql dialects.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback,

when I was adding the option of withDistinctOn with ordering I also thought that it could cause misunderstanding,

In general, the main purpose of that variant is make a use case when there is only distinctOn without extra ordering simpler, and prevent necessity to call orderBy right after withDistinctOn() with the same columns order, what could be broken on any refactoring.

So my expectation at the current moment that for any complex logic of adding ordering, the version withDistinctOn(vararg columns: Column<*>) (without ordering) should be used.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note that if withDistinctOn() is called after orderBy() (which i think is technically possible) then the ordering may be incorrect (DISTINCT ON at least in postgresql requires the specified columns to appear in the ORDER BY expression in the same order), e.g.

table
  .selectAll()
  .orderBy(table.colThree)
  .withDistinctOn(table.colOne, table.colTwo)

i think results in ORDER BY table.col_three, table.col_one, table.col_two which generates a SQL error.

a possible solution would be to manipulate the underlying orderByExpressions mutable list directly to prepend the list of columns passed in, instead of calling orderBy(). something like:

columns.forEachIndexed { index, column -> (orderByExpressions as MutableList).add(index, column) }

(since private set on the declaration orderByExpressions prohibits redefining the var)

}

@Deprecated(
message = "As part of SELECT DSL design changes, this will be removed in future releases.",
replaceWith = ReplaceWith("adjustSelect { body.invoke() }"),
Expand Down Expand Up @@ -174,6 +218,9 @@ open class Query(override var set: FieldSet, where: Op<Boolean>?) : AbstractQuer
if (distinct) {
append("DISTINCT ")
}
distinctOn?.let { columns ->
columns.appendTo(prefix = "DISTINCT ON (", postfix = ") ") { +"${it.table.tableName}.${it.name}" }
}
set.realFields.appendTo { +it }
}
if (set.source != Table.Dual || currentDialect.supportsDualTableConcept) {
Expand Down
82 changes: 82 additions & 0 deletions exposed-tests/src/test/kotlin/org/jetbrains/exposed/sql/tests/shared/dml/DistinctOnTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package org.jetbrains.exposed.sql.tests.shared.dml

import org.jetbrains.exposed.dao.id.IntIdTable
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 - distinctOnSupportedDb, tester) {
tester.batchInsert(
listOf(
listOf(1, 1), listOf(1, 2), listOf(1, 2),
listOf(2, 1), listOf(2, 2), listOf(2, 2),
listOf(4, 4), listOf(4, 4), listOf(4, 4),
)
) {
this[tester.value1] = it[0]
this[tester.value2] = it[1]
}

val distinctValue1 = tester.selectAll()
.withDistinctOn(tester.value1)
.orderBy(tester.value1 to SortOrder.ASC, tester.value2 to SortOrder.ASC)
.map { it[tester.value1] to it[tester.value2] }
assertEqualLists(listOf(1 to 1, 2 to 1, 4 to 4), distinctValue1)

val distinctValue2 = tester.selectAll()
.withDistinctOn(tester.value2)
.orderBy(tester.value2 to SortOrder.ASC, tester.value1 to SortOrder.ASC)
.map { it[tester.value1] to it[tester.value2] }
assertEqualLists(listOf(1 to 1, 1 to 2, 4 to 4), distinctValue2)

val distinctBoth = tester.selectAll()
.withDistinctOn(tester.value1, tester.value2)
.orderBy(tester.value1 to SortOrder.ASC, tester.value2 to SortOrder.ASC)
.map { it[tester.value1] to it[tester.value2] }
assertEqualLists(listOf(1 to 1, 1 to 2, 2 to 1, 2 to 2, 4 to 4), distinctBoth)

val distinctSequential = tester.selectAll()
.withDistinctOn(tester.value1 to SortOrder.ASC)
.withDistinctOn(tester.value2 to SortOrder.ASC)
.map { it[tester.value1] to it[tester.value2] }
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()
}
}
}
}
Loading