-
-
Notifications
You must be signed in to change notification settings - Fork 237
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
(Accidental?) Change in ForceDelete()
behavior vs. go-pg/pg
#673
Comments
maximerety
added a commit
to maximerety/bun
that referenced
this issue
Sep 22, 2022
```go db.NewDelete().Model((*Model)(nil)).Where("1 = 1").WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: DELETE FROM model WHERE 1 = 1 // After this commit: DELETE FROM model WHERE 1 = 1 AND deleted_at IS NOT NULL ``` Additionally, the `Where("1 = 1")` trick is no longer required as `WhereDeleted()` counts as a WHERE clause in mustAppendWhere. ```go db.NewDelete().Model((*Model)(nil)).WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: Errors with "bun: Update and Delete queries require at least one Where" // After this commit: DELETE FROM model WHERE deleted_at IS NOT NULL ``` This gives us an easy and natural way to hard delete all soft deleted records. Fixes issue uptrace#673.
maximerety
added a commit
to maximerety/bun
that referenced
this issue
Sep 22, 2022
```go db.NewDelete().Model((*Model)(nil)).Where("1 = 1").WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: DELETE FROM model WHERE 1 = 1 // After this commit: DELETE FROM model WHERE 1 = 1 AND deleted_at IS NOT NULL ``` Additionally, the `Where("1 = 1")` trick is no longer required as `WhereDeleted()` counts as a WHERE clause in mustAppendWhere. ```go db.NewDelete().Model((*Model)(nil)).WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: Errors with "bun: Update and Delete queries require at least one Where" // After this commit: DELETE FROM model WHERE deleted_at IS NOT NULL ``` This gives us an easy and natural way to hard delete all soft deleted records. Fixes issue uptrace#673.
maximerety
added a commit
to maximerety/bun
that referenced
this issue
Sep 22, 2022
As of Bun 1.1.8, `WhereDeleted()` is ignored when used with `ForceDelete()`. This commit fixes that problem: ```go db.NewDelete().Model((*Model)(nil)).Where("1 = 1").WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: DELETE FROM model WHERE 1 = 1 // After this commit: DELETE FROM model WHERE 1 = 1 AND deleted_at IS NOT NULL ``` Additionally, the `Where("1 = 1")` trick is no longer required as `WhereDeleted()` now counts as a user-defined `WHERE` clause in `mustAppendWhere`: ```go db.NewDelete().Model((*Model)(nil)).WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: Errors with "bun: Update and Delete queries require at least one Where" // After this commit: DELETE FROM model WHERE deleted_at IS NOT NULL ``` This gives us an easy and natural way to hard delete all soft deleted records. Fixes uptrace#673.
maximerety
added a commit
to maximerety/bun
that referenced
this issue
Sep 22, 2022
As of Bun 1.1.8, `WhereDeleted()` is ignored when used with `ForceDelete()`. This commit fixes that problem: ```go db.NewDelete().Model((*Model)(nil)).Where("1 = 1").WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: DELETE FROM model WHERE 1 = 1 // After this commit: DELETE FROM model WHERE 1 = 1 AND deleted_at IS NOT NULL ``` Additionally, the `Where("1 = 1")` trick is no longer required as `WhereDeleted()` now counts as a user-defined `WHERE` clause in `mustAppendWhere`: ```go db.NewDelete().Model((*Model)(nil)).WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: Errors with "bun: Update and Delete queries require at least one Where" // After this commit: DELETE FROM model WHERE deleted_at IS NOT NULL ``` This gives us an easy and natural way to hard delete all soft deleted records. Fixes uptrace#673.
maximerety
added a commit
to maximerety/bun-docs
that referenced
this issue
Sep 27, 2022
maximerety
added a commit
to maximerety/bun-docs
that referenced
this issue
Sep 27, 2022
maximerety
added a commit
to maximerety/bun
that referenced
this issue
Nov 22, 2022
As of Bun 1.1.8, `WhereDeleted()` is ignored when used with `ForceDelete()`. This commit fixes that problem: ```go db.NewDelete().Model((*Model)(nil)).Where("1 = 1").WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: DELETE FROM model WHERE 1 = 1 // After this commit: DELETE FROM model WHERE 1 = 1 AND deleted_at IS NOT NULL ``` Additionally, the `Where("1 = 1")` trick is no longer required as `WhereDeleted()` now counts as a user-defined `WHERE` clause in `mustAppendWhere`: ```go db.NewDelete().Model((*Model)(nil)).WhereDeleted().ForceDelete().Exec(ctx) // Before this commit: Errors with "bun: Update and Delete queries require at least one Where" // After this commit: DELETE FROM model WHERE deleted_at IS NOT NULL ``` This gives us an easy and natural way to hard delete all soft deleted records. Fixes uptrace#673.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Given a
Model
using the soft deletion feature:Previously in go-pg/pg
The query
db.Model((*Model)(nil)).ForceDelete()
generatedDELETE FROM model WHERE deleted_at IS NOT NULL
, i.e. performed the hard deletion of all soft deleted rows.This is because the
deletedFlag
was forcefully set and taken into account when generating the WHERE clause (isSoftDelete returns true, appendSoftDelete is called, and the clause is added).Now with bun
The query
db.NewDelete().Model((*Model)(nil)).ForceDelete().Exec(ctx)
generatesDELETE FROM model
, i.e. performs the hard deletion of all rows, whether soft deleted or not.Indeed:
(q *DeleteQuery) AppendQuery
, thedeletedFlag
is still forcefully applied to the query (query_delete.go#L161), butdeletedFlag
is then ignored by(q *whereBaseQuery) appendWhere
(query_base.go#L778) because the new implementation of(q *DeleteQuery) isSoftDelete()
returnsfalse
(query_delete.go#L205 due toforceDeleteFlag
being set).I wonder if that change was intentional or accidental, as forcing a flag to be set that is not then used seems to be a mistake.
It is possible to restore previous behavior by manually adding a
Where("deleted_at IS NOT NULL")
clause to the query, but usingWhereDeleted()
explicitly is not an option since thedeletedFlag
would still be ignored.IMHO, keeping the new behavior but letting
WhereDeleted()
work in conjunction withForceDelete()
would be the nicest option here:What do you think?
The text was updated successfully, but these errors were encountered: