Closes #1153.
MySQL has no RETURNING clause. returning(cols) has carried a branch for that on insert, update and delete for a while: the insert reads back by the key the server assigned, the update reads the ids the predicate matched before the write, the delete reads the rows before removing them.
returningAll() was a second implementation of the same idea. It appended RETURNING * itself and never grew that branch, so on MySQL every returningAll() write raised errno 1064 near 'RETURNING *', and because the statement never ran it wrote nothing at all.
Measured
MySQL 8.4.5, a disposable database, through createQueryBuilder.
| 0.2.69 | this branch | |
|---|---|---|
updateTable(t).set(v).where(id).returningAll().executeTakeFirst() | errno 1064, row unchanged | {id:1,title:'ONE',…}, row written |
insertInto(t).values(v).returningAll().executeTakeFirst() | errno 1064 | {id:6,title:'six',…} |
deleteFrom(t).where(id).returningAll().executeTakeFirst() | errno 1064, row still there | {id:5,…}, row gone |
| predicate applied after the handle | errno 1064, table intact | removes only that row |
Beyond the dialect branch
The duplicate had shed everything returning() gained since it was written:
- delete hooks. It never fired
beforeDelete/afterDelete, so adding.returningAll()to a delete walked straight past an application delete guard. That is the same defectreturning()was fixed for, and the usual reason to implementbeforeDeleteat all. - filters (#1110). The handle had no
where()to delegate, so a predicate applied after it was taken could not reach the statement. - row accessors. The declared return type is
SelectQueryBuilder, which hasget/first/firstOrFail, and the update and delete handles carried four methods and none of them, so.returningAll().first()threw "not a function".
So this deletes the second implementation rather than patching it, and the two cannot drift again. mysqlReturningBuilder already reads '*' as every column, so the read-back needed nothing further.
Tests
test/client.returning-all.test.ts, 12 cases. They run on SQLite, which takes RETURNING natively, so what they pin is the delegation rather than the MySQL text; the MySQL behaviour is the table above. Besides the behaviours, they compare the surfaces of returningAll() and returning('*'), so a reintroduced bespoke returningAll() fails there rather than silently shedding whatever returning() gains next.
Restoring the old implementation fails 8 of the 12.
Full suite 2293 pass / 0 fail, tsc --noEmit clean, pickier at baseline (15 pre-existing warnings in client.ts, unchanged; the new test is clean).
Downstream
Reported as stacksjs/stacks#2637, where returningAll() writes appear 63 times across cms, commerce and the orm traits, four of them behind dashboard PATCH routes.
Unrelated defect noticed while measuring, not addressed here: where(col, 'is', null) binds the NULL as a parameter and emits col is ?, which MySQL rejects. It fails the same way on a plain update with no returning at all, on 0.2.69 and on this branch. whereNull() is fine.
🤖 Generated with Claude Code