Skip to content

Commit

Permalink
fix: EXPOSED-550 DeleteStatement holds unused offset property (#2243)
Browse files Browse the repository at this point in the history
The 'offset' property in the class 'DeleteStatement' is not used anywhere nor to
generate SQL as no supported databases allow the OFFSET syntax in this operation.

The original class constructor and the table extension functions that accept an offset
are now deprecated in favor of ones that don't.
  • Loading branch information
bog-walk authored Sep 19, 2024
1 parent 6e840af commit 888ffd8
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 16 deletions.
2 changes: 2 additions & 0 deletions documentation-website/Writerside/topics/Breaking-Changes.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
This enables the use of the new `Join.delete()` function, which performs a delete operation on a specific table from the join relation.
The original statement class constructor has also been deprecated in favor of the constructor that accepts `targetsSet`, as well as another
additional parameter `targetTables` (for specifying which table from the join relation, if applicable, to delete from).
* The `DeleteStatement` property `offset` was not being used and is now deprecated, as are the extension functions that have an `offset` parameter.
`deleteWhere()` and `deleteIgnoreWhere()`, as well as the original statement class constructor, no longer accept an argument for `offset`.
* `SizedIterable.limit(n, offset)` is now deprecated in favor of 2 independent methods, `limit()` and `offset()`.
In supporting databases, this allows the generation of an OFFSET clause in the SELECT statement without any LIMIT clause.
Any custom implementations of the `SizedIterable` interface with a `limit()` override will now show a warning that the declaration overrides
Expand Down
10 changes: 8 additions & 2 deletions exposed-core/api/exposed-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -1822,11 +1822,15 @@ public final class org/jetbrains/exposed/sql/QueriesKt {
public static synthetic fun delete$default (Lorg/jetbrains/exposed/sql/Join;Lorg/jetbrains/exposed/sql/Table;[Lorg/jetbrains/exposed/sql/Table;ZLjava/lang/Integer;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)I
public static final fun deleteAll (Lorg/jetbrains/exposed/sql/Table;)I
public static final fun deleteIgnoreWhere (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Ljava/lang/Long;Lkotlin/jvm/functions/Function2;)I
public static final fun deleteIgnoreWhere (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;)I
public static synthetic fun deleteIgnoreWhere$default (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Ljava/lang/Long;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)I
public static synthetic fun deleteIgnoreWhere$default (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)I
public static final fun deleteReturning (Lorg/jetbrains/exposed/sql/Table;Ljava/util/List;Lkotlin/jvm/functions/Function1;)Lorg/jetbrains/exposed/sql/statements/ReturningStatement;
public static synthetic fun deleteReturning$default (Lorg/jetbrains/exposed/sql/Table;Ljava/util/List;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lorg/jetbrains/exposed/sql/statements/ReturningStatement;
public static final fun deleteWhere (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Ljava/lang/Long;Lkotlin/jvm/functions/Function2;)I
public static final fun deleteWhere (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;)I
public static synthetic fun deleteWhere$default (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Ljava/lang/Long;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)I
public static synthetic fun deleteWhere$default (Lorg/jetbrains/exposed/sql/Table;Ljava/lang/Integer;Lkotlin/jvm/functions/Function2;ILjava/lang/Object;)I
public static final fun exists (Lorg/jetbrains/exposed/sql/Table;)Z
public static final fun insert (Lorg/jetbrains/exposed/sql/Table;Lkotlin/jvm/functions/Function2;)Lorg/jetbrains/exposed/sql/statements/InsertStatement;
public static final fun insert (Lorg/jetbrains/exposed/sql/Table;Lorg/jetbrains/exposed/sql/AbstractQuery;Ljava/util/List;)Ljava/lang/Integer;
Expand Down Expand Up @@ -3110,8 +3114,8 @@ public class org/jetbrains/exposed/sql/statements/BatchUpsertStatement : org/jet

public class org/jetbrains/exposed/sql/statements/DeleteStatement : org/jetbrains/exposed/sql/statements/Statement {
public static final field Companion Lorg/jetbrains/exposed/sql/statements/DeleteStatement$Companion;
public fun <init> (Lorg/jetbrains/exposed/sql/ColumnSet;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/lang/Long;Ljava/util/List;)V
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/ColumnSet;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/lang/Long;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lorg/jetbrains/exposed/sql/ColumnSet;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/util/List;)V
public synthetic fun <init> (Lorg/jetbrains/exposed/sql/ColumnSet;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/util/List;ILkotlin/jvm/internal/DefaultConstructorMarker;)V
public fun <init> (Lorg/jetbrains/exposed/sql/Table;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/lang/Long;)V
public fun arguments ()Ljava/lang/Iterable;
public fun executeInternal (Lorg/jetbrains/exposed/sql/statements/api/PreparedStatementApi;Lorg/jetbrains/exposed/sql/Transaction;)Ljava/lang/Integer;
Expand All @@ -3128,7 +3132,9 @@ public class org/jetbrains/exposed/sql/statements/DeleteStatement : org/jetbrain

public final class org/jetbrains/exposed/sql/statements/DeleteStatement$Companion {
public final fun all (Lorg/jetbrains/exposed/sql/Transaction;Lorg/jetbrains/exposed/sql/Table;)I
public final fun where (Lorg/jetbrains/exposed/sql/Transaction;Lorg/jetbrains/exposed/sql/Table;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;)I
public final fun where (Lorg/jetbrains/exposed/sql/Transaction;Lorg/jetbrains/exposed/sql/Table;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/lang/Long;)I
public static synthetic fun where$default (Lorg/jetbrains/exposed/sql/statements/DeleteStatement$Companion;Lorg/jetbrains/exposed/sql/Transaction;Lorg/jetbrains/exposed/sql/Table;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;ILjava/lang/Object;)I
public static synthetic fun where$default (Lorg/jetbrains/exposed/sql/statements/DeleteStatement$Companion;Lorg/jetbrains/exposed/sql/Transaction;Lorg/jetbrains/exposed/sql/Table;Lorg/jetbrains/exposed/sql/Op;ZLjava/lang/Integer;Ljava/lang/Long;ILjava/lang/Object;)I
}

Expand Down
36 changes: 28 additions & 8 deletions exposed-core/src/main/kotlin/org/jetbrains/exposed/sql/Queries.kt
Original file line number Diff line number Diff line change
Expand Up @@ -99,17 +99,38 @@ fun Query.selectAllBatched(
return fetchBatchedResults(batchSize)
}

@Deprecated(
"This `offset` parameter is not being used and will be removed in future releases. Please leave a comment on " +
"[YouTrack](https://youtrack.jetbrains.com/issue/EXPOSED-550/DeleteStatement-holds-unused-offset-property) " +
"with a use-case if your database supports the OFFSET clause in a DELETE statement.",
ReplaceWith("deleteWhere(limit) { op.invoke() }"),
DeprecationLevel.WARNING
)
@Suppress("UnusedParameter")
fun <T : Table> T.deleteWhere(limit: Int? = null, offset: Long? = null, op: T.(ISqlExpressionBuilder) -> Op<Boolean>): Int =
deleteWhere(limit, op)

/**
* Represents the SQL statement that deletes only rows in a table that match the provided [op].
*
* @param limit Maximum number of rows to delete.
* @param offset The number of rows to skip.
* @param op Condition that determines which rows to delete.
* @return Count of deleted rows.
* @sample org.jetbrains.exposed.sql.tests.shared.dml.DeleteTests.testDelete01
*/
fun <T : Table> T.deleteWhere(limit: Int? = null, offset: Long? = null, op: T.(ISqlExpressionBuilder) -> Op<Boolean>): Int =
DeleteStatement.where(TransactionManager.current(), this@deleteWhere, op(SqlExpressionBuilder), false, limit, offset)
fun <T : Table> T.deleteWhere(limit: Int? = null, op: T.(ISqlExpressionBuilder) -> Op<Boolean>): Int =
DeleteStatement.where(TransactionManager.current(), this@deleteWhere, op(SqlExpressionBuilder), false, limit)

@Deprecated(
"This `offset` parameter is not being used and will be removed in future releases. Please leave a comment on " +
"[YouTrack](https://youtrack.jetbrains.com/issue/EXPOSED-550/DeleteStatement-holds-unused-offset-property) " +
"with a use-case if your database supports the OFFSET clause in a DELETE statement.",
ReplaceWith("deleteIgnoreWhere(limit) { op.invoke() }"),
DeprecationLevel.WARNING
)
@Suppress("UnusedParameter")
fun <T : Table> T.deleteIgnoreWhere(limit: Int? = null, offset: Long? = null, op: T.(ISqlExpressionBuilder) -> Op<Boolean>): Int =
deleteIgnoreWhere(limit, op)

/**
* Represents the SQL statement that deletes only rows in a table that match the provided [op], while ignoring any
Expand All @@ -118,12 +139,11 @@ fun <T : Table> T.deleteWhere(limit: Int? = null, offset: Long? = null, op: T.(I
* **Note:** `DELETE IGNORE` is not supported by all vendors. Please check the documentation.
*
* @param limit Maximum number of rows to delete.
* @param offset The number of rows to skip.
* @param op Condition that determines which rows to delete.
* @return Count of deleted rows.
*/
fun <T : Table> T.deleteIgnoreWhere(limit: Int? = null, offset: Long? = null, op: T.(ISqlExpressionBuilder) -> Op<Boolean>): Int =
DeleteStatement.where(TransactionManager.current(), this@deleteIgnoreWhere, op(SqlExpressionBuilder), true, limit, offset)
fun <T : Table> T.deleteIgnoreWhere(limit: Int? = null, op: T.(ISqlExpressionBuilder) -> Op<Boolean>): Int =
DeleteStatement.where(TransactionManager.current(), this@deleteIgnoreWhere, op(SqlExpressionBuilder), true, limit)

/**
* Represents the SQL statement that deletes all rows in a table.
Expand All @@ -147,7 +167,7 @@ fun <T : Table> T.deleteReturning(
returning: List<Expression<*>> = columns,
where: (SqlExpressionBuilder.() -> Op<Boolean>)? = null
): ReturningStatement {
val delete = DeleteStatement(targetsSet = this, where?.let { SqlExpressionBuilder.it() }, false, null, null)
val delete = DeleteStatement(this, where?.let { SqlExpressionBuilder.it() }, false, null)
return ReturningStatement(this, returning, delete)
}

Expand All @@ -173,7 +193,7 @@ fun Join.delete(
where: (SqlExpressionBuilder.() -> Op<Boolean>)? = null
): Int {
val targets = listOf(targetTable) + targetTables
val delete = DeleteStatement(this, where?.let { SqlExpressionBuilder.it() }, ignore, limit, null, targets)
val delete = DeleteStatement(this, where?.let { SqlExpressionBuilder.it() }, ignore, limit, targets)
return delete.execute(TransactionManager.current()) ?: 0
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,29 +15,28 @@ import org.jetbrains.exposed.sql.vendors.h2Mode
* @param isIgnore Whether to ignore errors or not.
* **Note** [isIgnore] is not supported by all vendors. Please check the documentation.
* @param limit Maximum number of rows to delete.
* @param offset The number of rows to skip.
* @param targetTables List of specific tables from [targetsSet] to delete rows from.
*/
open class DeleteStatement(
val targetsSet: ColumnSet,
val where: Op<Boolean>? = null,
val isIgnore: Boolean = false,
val limit: Int? = null,
val offset: Long? = null,
val targetTables: List<Table> = emptyList(),
) : Statement<Int>(StatementType.DELETE, targetsSet.targetTables()) {
@Deprecated(
"This constructor will be removed in future releases.",
ReplaceWith("DeleteStatement(targetsSet = table, where, isIgnore, limit, offset, emptyList())"),
ReplaceWith("DeleteStatement(targetsSet = table, where, isIgnore, limit, emptyList())"),
DeprecationLevel.WARNING
)
@Suppress("UnusedPrivateProperty")
constructor(
table: Table,
where: Op<Boolean>?,
isIgnore: Boolean,
limit: Int?,
offset: Long?
) : this(table, where, isIgnore, limit, offset, emptyList())
) : this(table, where, isIgnore, limit, emptyList())

@Deprecated(
"This property will be removed in future releases and replaced with a property that stores a `ColumnSet`," +
Expand All @@ -47,6 +46,14 @@ open class DeleteStatement(
)
val table: Table = targets.first()

@Deprecated(
"This property is not being used and will be removed in future releases. Please leave a comment on " +
"[YouTrack](https://youtrack.jetbrains.com/issue/EXPOSED-550/DeleteStatement-holds-unused-offset-property) " +
"with a use-case if your database supports the OFFSET clause in a DELETE statement.",
level = DeprecationLevel.WARNING
)
val offset: Long? = null

override fun PreparedStatementApi.executeInternal(transaction: Transaction): Int {
return executeUpdate()
}
Expand Down Expand Up @@ -80,13 +87,24 @@ open class DeleteStatement(
}

companion object {
@Deprecated(
"This function that accepts an 'offset' argument will be removed in future releases. Please leave a comment on " +
"[YouTrack](https://youtrack.jetbrains.com/issue/EXPOSED-550/DeleteStatement-holds-unused-offset-property) " +
"with a use-case if your database supports the OFFSET clause in a DELETE statement.",
ReplaceWith("where(transaction, table, op, isIgnore, limit)"),
DeprecationLevel.WARNING
)
@Suppress("UnusedParameter")
fun where(transaction: Transaction, table: Table, op: Op<Boolean>, isIgnore: Boolean = false, limit: Int? = null, offset: Long? = null): Int =
where(transaction, table, op, isIgnore, limit)

/**
* Creates a [DeleteStatement] that deletes only rows in [table] that match the provided [op].
*
* @return Count of deleted rows.
*/
fun where(transaction: Transaction, table: Table, op: Op<Boolean>, isIgnore: Boolean = false, limit: Int? = null, offset: Long? = null): Int =
DeleteStatement(targetsSet = table, op, isIgnore, limit, offset).execute(transaction) ?: 0
fun where(transaction: Transaction, table: Table, op: Op<Boolean>, isIgnore: Boolean = false, limit: Int? = null): Int =
DeleteStatement(table, op, isIgnore, limit, emptyList()).execute(transaction) ?: 0

/**
* Creates a [DeleteStatement] that deletes all rows in [table].
Expand Down

0 comments on commit 888ffd8

Please sign in to comment.