Skip to content

fix: EXPOSED-987 Add withParentheses to set operations for correct precedence #2776

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

Open
Christopher Jenkins (chrisjenx) wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: EXPOSED-987 Add withParentheses to set operations for correct precedence #2776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
30 changes: 20 additions & 10 deletions exposed-jdbc/api/exposed-jdbc.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ public final class org/jetbrains/exposed/v1/jdbc/EmptySizedIterable : java/util/
}

public final class org/jetbrains/exposed/v1/jdbc/Except : org/jetbrains/exposed/v1/jdbc/SetOperation {
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;)V
public fun copy ()Lorg/jetbrains/exposed/v1/jdbc/Intersect;
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Z)V
public synthetic fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun copy ()Lorg/jetbrains/exposed/v1/jdbc/Except;
public synthetic fun copy ()Lorg/jetbrains/exposed/v1/jdbc/SizedIterable;
public fun getOperationName ()Ljava/lang/String;
public synthetic fun withDistinct (Z)Lorg/jetbrains/exposed/v1/core/AbstractQuery;
Expand Down Expand Up @@ -86,7 +87,8 @@ public final class org/jetbrains/exposed/v1/jdbc/ExposedConnectionImpl : org/jet
}

public final class org/jetbrains/exposed/v1/jdbc/Intersect : org/jetbrains/exposed/v1/jdbc/SetOperation {
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;)V
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Z)V
public synthetic fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun copy ()Lorg/jetbrains/exposed/v1/jdbc/Intersect;
public synthetic fun copy ()Lorg/jetbrains/exposed/v1/jdbc/SizedIterable;
public synthetic fun withDistinct (Z)Lorg/jetbrains/exposed/v1/core/AbstractQuery;
Expand Down Expand Up @@ -332,7 +334,8 @@ public final class org/jetbrains/exposed/v1/jdbc/SchemaUtils : org/jetbrains/exp

public abstract class org/jetbrains/exposed/v1/jdbc/SetOperation : org/jetbrains/exposed/v1/core/AbstractQuery, org/jetbrains/exposed/v1/jdbc/SizedIterable, org/jetbrains/exposed/v1/jdbc/statements/BlockingExecutable {
public static final field Companion Lorg/jetbrains/exposed/v1/jdbc/SetOperation$Companion;
public synthetic fun <init> (Ljava/lang/String;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/String;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun <init> (Ljava/lang/String;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;ZLkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun count ()J
public fun empty ()Z
public synthetic fun execute (Lorg/jetbrains/exposed/v1/jdbc/JdbcTransaction;)Ljava/lang/Object;
Expand All @@ -347,6 +350,7 @@ public abstract class org/jetbrains/exposed/v1/jdbc/SetOperation : org/jetbrains
public synthetic fun getStatement ()Lorg/jetbrains/exposed/v1/core/statements/Statement;
public fun getStatement ()Lorg/jetbrains/exposed/v1/jdbc/SetOperation;
protected final fun getTransaction ()Lorg/jetbrains/exposed/v1/jdbc/JdbcTransaction;
public final fun getWrapInParentheses ()Z
public fun isAlwaysBatch ()Z
public fun iterator ()Ljava/util/Iterator;
public fun limit (I)Lorg/jetbrains/exposed/v1/jdbc/SetOperation;
Expand All @@ -367,10 +371,14 @@ public final class org/jetbrains/exposed/v1/jdbc/SetOperation$Companion {
}

public final class org/jetbrains/exposed/v1/jdbc/SetOperationsKt {
public static final fun except (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;)Lorg/jetbrains/exposed/v1/jdbc/Except;
public static final fun intersect (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;)Lorg/jetbrains/exposed/v1/jdbc/Intersect;
public static final fun union (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;)Lorg/jetbrains/exposed/v1/jdbc/Union;
public static final fun unionAll (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;)Lorg/jetbrains/exposed/v1/jdbc/UnionAll;
public static final fun except (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;Z)Lorg/jetbrains/exposed/v1/jdbc/Except;
public static synthetic fun except$default (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;ZILjava/lang/Object;)Lorg/jetbrains/exposed/v1/jdbc/Except;
public static final fun intersect (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;Z)Lorg/jetbrains/exposed/v1/jdbc/Intersect;
public static synthetic fun intersect$default (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;ZILjava/lang/Object;)Lorg/jetbrains/exposed/v1/jdbc/Intersect;
public static final fun union (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;Z)Lorg/jetbrains/exposed/v1/jdbc/Union;
public static synthetic fun union$default (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;ZILjava/lang/Object;)Lorg/jetbrains/exposed/v1/jdbc/Union;
public static final fun unionAll (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;Z)Lorg/jetbrains/exposed/v1/jdbc/UnionAll;
public static synthetic fun unionAll$default (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/jdbc/Query;ZILjava/lang/Object;)Lorg/jetbrains/exposed/v1/jdbc/UnionAll;
}

public final class org/jetbrains/exposed/v1/jdbc/SizedCollection : org/jetbrains/exposed/v1/jdbc/SizedIterable {
Expand Down Expand Up @@ -407,15 +415,17 @@ public final class org/jetbrains/exposed/v1/jdbc/SizedIterable$DefaultImpls {
}

public final class org/jetbrains/exposed/v1/jdbc/Union : org/jetbrains/exposed/v1/jdbc/SetOperation {
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;)V
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Z)V
public synthetic fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun copy ()Lorg/jetbrains/exposed/v1/jdbc/SizedIterable;
public fun copy ()Lorg/jetbrains/exposed/v1/jdbc/Union;
public synthetic fun withDistinct (Z)Lorg/jetbrains/exposed/v1/core/AbstractQuery;
public fun withDistinct (Z)Lorg/jetbrains/exposed/v1/jdbc/SetOperation;
}

public final class org/jetbrains/exposed/v1/jdbc/UnionAll : org/jetbrains/exposed/v1/jdbc/SetOperation {
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;)V
public fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;Z)V
public synthetic fun <init> (Lorg/jetbrains/exposed/v1/core/AbstractQuery;Lorg/jetbrains/exposed/v1/core/AbstractQuery;ZILkotlin/jvm/internal/DefaultConstructorMarker;)V
public synthetic fun copy ()Lorg/jetbrains/exposed/v1/jdbc/SizedIterable;
public fun copy ()Lorg/jetbrains/exposed/v1/jdbc/UnionAll;
public synthetic fun withDistinct (Z)Lorg/jetbrains/exposed/v1/core/AbstractQuery;
Expand Down
59 changes: 39 additions & 20 deletions exposed-jdbc/src/main/kotlin/org/jetbrains/exposed/v1/jdbc/SetOperations.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,14 @@ import java.sql.ResultSet
* Represents an SQL operation that combines the results of multiple queries into a single result.
*
* @param secondStatement The SQL statement on the right-hand side of the set operator.
* @param wrapInParentheses If `true`, wraps any preceding set operation in parentheses to ensure the
* DSL chain order matches the SQL evaluation order. If `false`, relies on SQL operator precedence.
*/
sealed class SetOperation(
operationName: String,
_firstStatement: AbstractQuery<*>,
val secondStatement: AbstractQuery<*>
val secondStatement: AbstractQuery<*>,
val wrapInParentheses: Boolean = true
) : AbstractQuery<SetOperation>((_firstStatement.targets + secondStatement.targets).distinct()),
BlockingExecutable<ResultApi, SetOperation>,
SizedIterable<ResultRow> {
Expand Down Expand Up @@ -134,7 +137,11 @@ sealed class SetOperation(
it.prepareSQL(this)
if (isSubQuery) append(")")
}
is SetOperation -> it.prepareSQL(this)
is SetOperation -> {
if (wrapInParentheses) append("(")
it.prepareSQL(this)
if (wrapInParentheses) append(")")
}
}
}
}
Expand Down Expand Up @@ -193,48 +200,51 @@ sealed class SetOperation(
/** Represents an SQL operation that combines all results from two queries, without any duplicates. */
class Union(
firstStatement: AbstractQuery<*>,
secondStatement: AbstractQuery<*>
) : SetOperation("UNION", firstStatement, secondStatement) {
secondStatement: AbstractQuery<*>,
wrapInParentheses: Boolean = true
) : SetOperation("UNION", firstStatement, secondStatement, wrapInParentheses) {
override fun withDistinct(value: Boolean): SetOperation {
return if (!value) {
UnionAll(firstStatement, secondStatement).also {
UnionAll(firstStatement, secondStatement, wrapInParentheses).also {
copyTo(it)
}
} else {
this
}
}

override fun copy() = Union(firstStatement, secondStatement).also {
override fun copy() = Union(firstStatement, secondStatement, wrapInParentheses).also {
copyTo(it)
}
}

/** Represents an SQL operation that combines all results from two queries, with duplicates included. */
class UnionAll(
firstStatement: AbstractQuery<*>,
secondStatement: AbstractQuery<*>
) : SetOperation("UNION ALL", firstStatement, secondStatement) {
secondStatement: AbstractQuery<*>,
wrapInParentheses: Boolean = true
) : SetOperation("UNION ALL", firstStatement, secondStatement, wrapInParentheses) {

override fun withDistinct(value: Boolean): SetOperation {
return if (value) {
Union(firstStatement, secondStatement)
Union(firstStatement, secondStatement, wrapInParentheses)
} else {
this
}
}

override fun copy() = UnionAll(firstStatement, secondStatement).also {
override fun copy() = UnionAll(firstStatement, secondStatement, wrapInParentheses).also {
copyTo(it)
}
}

/** Represents an SQL operation that returns only the common rows from two query results, without any duplicates. */
class Intersect(
firstStatement: AbstractQuery<*>,
secondStatement: AbstractQuery<*>
) : SetOperation("INTERSECT", firstStatement, secondStatement) {
override fun copy() = Intersect(firstStatement, secondStatement).also {
secondStatement: AbstractQuery<*>,
wrapInParentheses: Boolean = true
) : SetOperation("INTERSECT", firstStatement, secondStatement, wrapInParentheses) {
override fun copy() = Intersect(firstStatement, secondStatement, wrapInParentheses).also {
copyTo(it)
}

Expand All @@ -254,16 +264,17 @@ class Intersect(
*/
class Except(
firstStatement: AbstractQuery<*>,
secondStatement: AbstractQuery<*>
) : SetOperation("EXCEPT", firstStatement, secondStatement) {
secondStatement: AbstractQuery<*>,
wrapInParentheses: Boolean = true
) : SetOperation("EXCEPT", firstStatement, secondStatement, wrapInParentheses) {

override val operationName: String
get() = when {
currentDialect is OracleDialect || currentDialect.h2Mode == H2Dialect.H2CompatibilityMode.Oracle -> "MINUS"
else -> "EXCEPT"
}

override fun copy() = Intersect(firstStatement, secondStatement).also {
override fun copy() = Except(firstStatement, secondStatement, wrapInParentheses).also {
copyTo(it)
}

Expand All @@ -281,27 +292,35 @@ class Except(
/**
* Combines all results from [this] query with the results of [other], WITHOUT including duplicates.
*
* @param withParentheses If `true`, wraps any preceding set operation in parentheses to ensure the
* DSL chain order matches the SQL evaluation order. If `false`, relies on SQL operator precedence.
* @sample org.jetbrains.exposed.v1.tests.shared.dml.UnionTests.testUnionWithLimit
*/
fun AbstractQuery<*>.union(other: Query): Union = Union(this, other)
fun AbstractQuery<*>.union(other: Query, withParentheses: Boolean = true): Union = Union(this, other, withParentheses)

/**
* Combines all results from [this] query with the results of [other], WITH duplicates included.
*
* @param withParentheses If `true`, wraps any preceding set operation in parentheses to ensure the
* DSL chain order matches the SQL evaluation order. If `false`, relies on SQL operator precedence.
* @sample org.jetbrains.exposed.v1.tests.shared.dml.UnionTests.testUnionWithAllResults
*/
fun AbstractQuery<*>.unionAll(other: Query): UnionAll = UnionAll(this, other)
fun AbstractQuery<*>.unionAll(other: Query, withParentheses: Boolean = true): UnionAll = UnionAll(this, other, withParentheses)

/**
* Returns only results from [this] query that are common to the results of [other], WITHOUT including any duplicates.
*
* @param withParentheses If `true`, wraps any preceding set operation in parentheses to ensure the
* DSL chain order matches the SQL evaluation order. If `false`, relies on SQL operator precedence.
* @sample org.jetbrains.exposed.v1.tests.shared.dml.UnionTests.testIntersectWithThreeQueries
*/
fun AbstractQuery<*>.intersect(other: Query): Intersect = Intersect(this, other)
fun AbstractQuery<*>.intersect(other: Query, withParentheses: Boolean = true): Intersect = Intersect(this, other, withParentheses)

/**
* Returns only distinct results from [this] query that are NOT common to the results of [other].
*
* @param withParentheses If `true`, wraps any preceding set operation in parentheses to ensure the
* DSL chain order matches the SQL evaluation order. If `false`, relies on SQL operator precedence.
* @sample org.jetbrains.exposed.v1.tests.shared.dml.UnionTests.testExceptWithTwoQueries
*/
fun AbstractQuery<*>.except(other: Query): Except = Except(this, other)
fun AbstractQuery<*>.except(other: Query, withParentheses: Boolean = true): Except = Except(this, other, withParentheses)
38 changes: 36 additions & 2 deletions ...c-tests/src/test/kotlin/org/jetbrains/exposed/v1/r2dbc/sql/tests/shared/dml/UnionTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.jetbrains.exposed.v1.r2dbc.sql.tests.shared.dml

import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.toList
import org.jetbrains.exposed.v1.core.QueryBuilder
import org.jetbrains.exposed.v1.core.SortOrder
import org.jetbrains.exposed.v1.core.alias
import org.jetbrains.exposed.v1.core.eq
Expand All @@ -27,6 +28,7 @@ import org.jetbrains.exposed.v1.r2dbc.tests.shared.expectException
import org.jetbrains.exposed.v1.r2dbc.union
import org.jetbrains.exposed.v1.r2dbc.unionAll
import org.junit.jupiter.api.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class UnionTests : R2dbcDatabaseTestsBase() {
Expand Down Expand Up @@ -94,6 +96,20 @@ class UnionTests : R2dbcDatabaseTestsBase() {

@Test
fun testIntersectWithThreeQueries() {
withCitiesAndUsers(TestDB.ALL_MYSQL) { _, users, _ ->
val usersQuery = users.selectAll()
val sergeyQuery = users.selectAll().where { users.id eq "sergey" }

// Default withParentheses=true: (UNION ALL) INTERSECT ensures consistent results
usersQuery.unionAll(usersQuery).intersect(sergeyQuery).map { it[users.id] }.toList().apply {
assertEquals(1, size)
assertEquals("sergey", single())
}
}
}

@Test
fun testIntersectWithoutParentheses() {
withCitiesAndUsers(TestDB.ALL_MYSQL) { _, users, _ ->
val usersQuery = users.selectAll()
val sergeyQuery = users.selectAll().where { users.id eq "sergey" }
Expand All @@ -102,18 +118,36 @@ class UnionTests : R2dbcDatabaseTestsBase() {
is PostgreSQLDialect, is SQLServerDialect, is MariaDBDialect, is H2Dialect -> true
else -> false
}
usersQuery.unionAll(usersQuery).intersect(sergeyQuery).map { it[users.id] }.toList().apply {
// withParentheses=false preserves legacy behavior (no wrapping)
usersQuery.unionAll(usersQuery).intersect(sergeyQuery, withParentheses = false).map { it[users.id] }.toList().apply {
if (intersectAppliedFirst) {
assertEquals(6, size)
assertEqualCollections(this, expectedUsers)
} else {
assertEquals(1, size)
assertEquals("sergey", this.single())
assertEquals("sergey", single())
}
}
}
}

@Test
fun testSetOperationGeneratesParentheses() {
withCitiesAndUsers { _, users, _ ->
val q1 = users.selectAll().where { users.id eq "andrey" }
val q2 = users.selectAll().where { users.id eq "sergey" }
val q3 = users.selectAll().where { users.id eq "eugene" }

val withParens = q1.union(q2).intersect(q3)
val sqlWithParens = withParens.prepareSQL(QueryBuilder(false))
assertTrue(sqlWithParens.startsWith("("))

val withoutParens = q1.union(q2).intersect(q3, withParentheses = false)
val sqlWithoutParens = withoutParens.prepareSQL(QueryBuilder(false))
assertFalse(sqlWithoutParens.startsWith("("))
}
}

@Test
fun testExceptWithTwoQueries() {
withCitiesAndUsers(TestDB.ALL_MYSQL) { _, users, _ ->
Expand Down
Loading
Loading