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.

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

deprecate!: EXPOSED-550 DeleteStatement holds unused offset property #2243

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

bog-walk
Copy link
Member

Description

Summary of the change:
DeleteStatement.offset is now deprecated, as well as both deleteWhere() and deleteIgnoreWhere() that accept an argument for offset parameter.

Detailed description:

  • Why:

The class DeleteStatement has a property offset that is not used anywhere in the library nor when generating SQL, as no supported databases allow the OFFSET syntax in this operation. This has now been confirmed by db documentation and by checking with plain sql.

The property was originally implemented wherever DELETE + LIMIT was allowed, but it was later removed since even databases that support LIMIT do not accept OFFSET in this statement. In spite of its removal from SQL generation, the property was left in both the class and table extension functions.

  • What:
    • DeleteStatement.offset property is deprecated.
    • DeleteStatement constructor that accepts offset value is deprecated.
    • Table.deleteWhere() and Table.deleteIgnoreWhere() are also deprecated in favor of overloads that do not have offset parameter.
    • No changes made to tests as no tests exist for it (since it wasn't actually implemented).

Note:
There is a possibility that DeleteStatement class is being extended and the offset property is being used with a prepareSQL() override (for example, to generate SQL that deletes from a select subquery with limit + offset). If a custom class uses this property, that means a custom table extension would also be used to instantiate the custom class. And the deprecated property could be set up as a custom property.
Any customizations that are no longer possible with these changes should be addressed in the related issue below.


Type of Change

Please mark the relevant options with an "X":

  • Deprecation

Updates/remove existing public API methods:

  • Is breaking change

Affected databases:

  • All

Checklist

  • The build is green (including the Detekt check)
  • All public methods affected by my PR has up to date API docs
  • Documentation for my change is up to date

Related Issues

EXPOSED-550

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.
@bog-walk bog-walk merged commit 888ffd8 into main Sep 19, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/remove-delete-offset branch September 19, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants