ReviewOS

stacks/bun-query-builder

ORM: onlyTrashed().delete() removes every row, and destructive paths ignore the soft-delete scope

#1111
Closed glennmichael123 opened this 23 days ago · 0 comments
23 days ago

Found on main @ a258137, verified by execution (sqlite).

The ORM's destructive paths build their WHERE with buildWhereClauses(), guarded by this._wheres.length > 0. Every read path uses composeWhere() (orm.ts:2593) — which is the only place softDeleteClause() is added.

So a scope that exists purely as a soft-delete predicate contributes nothing to a write, and when it is the only predicate the statement goes out with no WHERE at all.

Repro

const Post = createModel({
  name: 'Sdpost', table: 'sd_posts', primaryKey: 'id', autoIncrement: true,
  traits: { useSoftDeletes: true },
  attributes: { title: { type: 'string', fillable: true }, status: { type: 'string', fillable: true } },
} as const)

// 4 rows, two of them soft-deleted
await Post.query().onlyTrashed().delete()

Observed:

after onlyTrashed().delete(): total=0   alive=0      <- every row gone, including live ones
                       want: total=2   alive=2

The intent — "purge the trash" — is the exact call that destroys the live rows.

Second finding in the same probe

Post.query().where('title','a').delete() hard-deletes on a model declaring useSoftDeletes:

after 2 x .where(...).delete(): total=2 alive=2
                          want: total=4 alive=2   (rows soft-deleted, not removed)

createTableFromModel does create the deleted_at column, so the trait is recognised elsewhere — the builder's delete() simply does not consult it. Worth confirming whether this is intended (an explicit hard delete) or the same root cause; if intended, forceDelete() exists for that and this call should soft-delete.

  • update() and increment()/decrement() take the same buildWhereClauses path, so onlyTrashed().update({...}) rewrites every row.
  • limit(), offset() and orderBy() are read by none of delete(), update() or increment(), so query().where(...).orderBy('id').limit(1).delete() deletes every matching row rather than one.

Suggested fix

Route the destructive paths through composeWhere(), the same function every read path uses — the divergence between "how a read scopes itself" and "how a write scopes itself" is the defect, and any fix that leaves two implementations will drift again.

A write whose composed WHERE is empty is worth refusing outright, in the spirit of #1101.

Relevant code: orm.ts:3105-3115 (delete), :3117-3134 (update), :2939-2957 (increment), versus :2593 (composeWhere).

Found by an automated sweep; the row counts above are from my own reproduction.

Sign in to comment on this issue.