also looking at this
fix(client): apply the filters that follow returning(), instead of discarding them
#1115
2 files
+262
-44
| @@ -7115,26 +7115,47 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7115 | 7115 | return this |
| 7116 | 7116 | }, |
| 7117 | 7117 | returning(...cols) { |
| 7118 | const retText = `${sqlText} RETURNING ${cols.join(', ')}` | |
| 7119 | const q = params.length > 0 | |
| 7120 | ? _sql.unsafe(retText, params) | |
| 7121 | : _sql.unsafe(retText) | |
| 7118 | const parent: any = this | |
| 7119 | // Render at execute time, not here. | |
| 7120 | // | |
| 7121 | // This used to freeze the statement text immediately and return an | |
| 7122 | // object whose where/andWhere/orWhere/orderBy/limit/offset were | |
| 7123 | // literally `() => obj` — so every filter applied after returning() | |
| 7124 | // was discarded and the UPDATE ran against the whole table, silently, | |
| 7125 | // while `.executeTakeFirst()` handed back a row it had never filtered | |
| 7126 | // to. `returning()` is typed as SelectQueryBuilder, which declares all | |
| 7127 | // of those, so the broken order type-checks. See #1110. | |
| 7128 | // | |
| 7129 | // Deferring also fixes the aliasing form, where the returning handle | |
| 7130 | // is held while the parent gains a predicate. | |
| 7131 | const retText = () => `${sqlText} RETURNING ${cols.join(', ')}` | |
| 7132 | const build = () => (params.length > 0 ? _sql.unsafe(retText(), params) : _sql.unsafe(retText())) | |
| 7122 | 7133 | const runFirst = async () => { |
| 7123 | const rows = await runWithHooks<any[]>(q, 'update') | |
| 7134 | const rows = await runWithHooks<any[]>(build(), 'update') | |
| 7124 | 7135 | return Array.isArray(rows) ? rows[0] : rows |
| 7125 | 7136 | } |
| 7137 | // Not expressible on this builder. Silently ignoring them is what | |
| 7138 | // made a `.limit(1)` destructive write hit every row. | |
| 7139 | const unsupported = (name: string) => (): never => { | |
| 7140 | throw new TypeError(`[query-builder] updateTable.returning(...).${name}() is not supported — an UPDATE cannot be ordered or limited through this builder. Apply the filter with where() instead.`) | |
| 7141 | } | |
| 7126 | 7142 | const obj: any = { |
| 7127 | where: () => obj, | |
| 7128 | andWhere: () => obj, | |
| 7129 | orWhere: () => obj, | |
| 7130 | orderBy: () => obj, | |
| 7131 | limit: () => obj, | |
| 7132 | offset: () => obj, | |
| 7133 | toSQL: () => makeExecutableQuery(q, retText) as any, | |
| 7134 | execute: () => runWithHooks<any[]>(q, 'update'), | |
| 7143 | // Delegate to the builder that owns the statement, then keep | |
| 7144 | // chaining from here. Consecutive predicates join with AND, which | |
| 7145 | // is what andWhere means. | |
| 7146 | where: (...args: any[]) => { parent.where(...args); return obj }, | |
| 7147 | andWhere: (...args: any[]) => { parent.where(...args); return obj }, | |
| 7148 | whereNull: (...args: any[]) => { parent.whereNull(...args); return obj }, | |
| 7149 | whereNotNull: (...args: any[]) => { parent.whereNotNull(...args); return obj }, | |
| 7150 | orWhere: unsupported('orWhere'), | |
| 7151 | orderBy: unsupported('orderBy'), | |
| 7152 | limit: unsupported('limit'), | |
| 7153 | offset: unsupported('offset'), | |
| 7154 | toSQL: () => makeExecutableQuery(build(), retText()) as any, | |
| 7155 | execute: () => runWithHooks<any[]>(build(), 'update'), | |
| 7135 | 7156 | // returning() is typed as SelectQueryBuilder — expose the |
| 7136 | 7157 | // row-fetching methods so `.returning('id').first()` works. |
| 7137 | get: () => runWithHooks<any[]>(q, 'update'), | |
| 7158 | get: () => runWithHooks<any[]>(build(), 'update'), | |
| 7138 | 7159 | first: runFirst, |
| 7139 | 7160 | executeTakeFirst: runFirst, |
| 7140 | 7161 | async firstOrFail() { |
| @@ -7175,19 +7196,20 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7175 | 7196 | return { numUpdatedRows: result } |
| 7176 | 7197 | }, |
| 7177 | 7198 | returningAll() { |
| 7178 | const retAllText = `${sqlText} RETURNING *` | |
| 7179 | const q = params.length > 0 | |
| 7180 | ? _sql.unsafe(retAllText, params) | |
| 7181 | : _sql.unsafe(retAllText) | |
| 7199 | // Deferred for the same reason as returning(): holding this handle | |
| 7200 | // while the parent gains a predicate must not execute the statement | |
| 7201 | // as it stood beforehand. See #1110. | |
| 7202 | const retAllText = () => `${sqlText} RETURNING *` | |
| 7203 | const build = () => (params.length > 0 ? _sql.unsafe(retAllText(), params) : _sql.unsafe(retAllText())) | |
| 7182 | 7204 | return { |
| 7183 | toSQL: () => makeExecutableQuery(q, retAllText) as any, | |
| 7184 | execute: () => runWithHooks<any[]>(q, 'update'), | |
| 7205 | toSQL: () => makeExecutableQuery(build(), retAllText()) as any, | |
| 7206 | execute: () => runWithHooks<any[]>(build(), 'update'), | |
| 7185 | 7207 | async executeTakeFirst() { |
| 7186 | const result = await runWithHooks<any[]>(q, 'update') | |
| 7208 | const result = await runWithHooks<any[]>(build(), 'update') | |
| 7187 | 7209 | return Array.isArray(result) ? result[0] : result |
| 7188 | 7210 | }, |
| 7189 | 7211 | async executeTakeFirstOrThrow() { |
| 7190 | const result = await runWithHooks<any[]>(q, 'update') | |
| 7212 | const result = await runWithHooks<any[]>(build(), 'update') | |
| 7191 | 7213 | const first = Array.isArray(result) ? result[0] : result |
| 7192 | 7214 | if (!first) |
| 7193 | 7215 | throw new Error('Update with RETURNING failed') |
| @@ -7337,25 +7359,46 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7337 | 7359 | return this |
| 7338 | 7360 | }, |
| 7339 | 7361 | returning(...cols) { |
| 7340 | const retText = `${sqlText} RETURNING ${cols.join(', ')}` | |
| 7341 | const q = delParams.length > 0 | |
| 7342 | ? _sql.unsafe(retText, delParams) | |
| 7343 | : _sql.unsafe(retText) | |
| 7362 | const parent: any = this | |
| 7363 | // Rendered at execute time. The frozen-text form, with the no-op | |
| 7364 | // filter stubs below it, meant `.returning('id').where({ id: 1 })` | |
| 7365 | // emptied the table and returned every deleted row as though the | |
| 7366 | // filter had applied. See #1110. | |
| 7367 | const retText = () => `${sqlText} RETURNING ${cols.join(', ')}` | |
| 7368 | const build = () => (delParams.length > 0 ? _sql.unsafe(retText(), delParams) : _sql.unsafe(retText())) | |
| 7369 | // Fire the same delete hooks execute() does. Without this, adding | |
| 7370 | // `.returning(...)` to a delete skipped beforeDelete/afterDelete | |
| 7371 | // entirely, so an application-level delete guard — the usual reason | |
| 7372 | // to implement beforeDelete at all — could be walked straight past. | |
| 7373 | const runDelete = async (): Promise<any[]> => { | |
| 7374 | await activeHooks()?.beforeDelete?.({ table: String(table), where: whereCondition }) | |
| 7375 | const rows = await runWithHooks<any[]>(build(), 'delete') | |
| 7376 | try { | |
| 7377 | await activeHooks()?.afterDelete?.({ table: String(table), where: whereCondition, result: rows }) | |
| 7378 | } | |
| 7379 | catch {} | |
| 7380 | return rows | |
| 7381 | } | |
| 7344 | 7382 | const runFirst = async () => { |
| 7345 | const rows = await runWithHooks<any[]>(q, 'delete') | |
| 7383 | const rows = await runDelete() | |
| 7346 | 7384 | return Array.isArray(rows) ? rows[0] : rows |
| 7347 | 7385 | } |
| 7386 | const unsupported = (name: string) => (): never => { | |
| 7387 | throw new TypeError(`[query-builder] deleteFrom.returning(...).${name}() is not supported — a DELETE cannot be ordered or limited through this builder. Narrow it with where() instead.`) | |
| 7388 | } | |
| 7348 | 7389 | const obj: any = { |
| 7349 | where: () => obj, | |
| 7350 | andWhere: () => obj, | |
| 7351 | orWhere: () => obj, | |
| 7352 | orderBy: () => obj, | |
| 7353 | limit: () => obj, | |
| 7354 | offset: () => obj, | |
| 7355 | toSQL: () => makeExecutableQuery(q, retText) as any, | |
| 7356 | execute: () => runWithHooks<any[]>(q, 'delete'), | |
| 7390 | where: (...args: any[]) => { parent.where(...args); return obj }, | |
| 7391 | andWhere: (...args: any[]) => { parent.where(...args); return obj }, | |
| 7392 | whereNull: (...args: any[]) => { parent.whereNull(...args); return obj }, | |
| 7393 | whereNotNull: (...args: any[]) => { parent.whereNotNull(...args); return obj }, | |
| 7394 | orWhere: unsupported('orWhere'), | |
| 7395 | orderBy: unsupported('orderBy'), | |
| 7396 | limit: unsupported('limit'), | |
| 7397 | offset: unsupported('offset'), | |
| 7398 | toSQL: () => makeExecutableQuery(build(), retText()) as any, | |
| 7399 | execute: () => runDelete(), | |
| 7357 | 7400 | // returning() is typed as SelectQueryBuilder — expose row fetchers. |
| 7358 | get: () => runWithHooks<any[]>(q, 'delete'), | |
| 7401 | get: () => runDelete(), | |
| 7359 | 7402 | first: runFirst, |
| 7360 | 7403 | executeTakeFirst: runFirst, |
| 7361 | 7404 | async firstOrFail() { |
| @@ -7404,19 +7447,18 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 7404 | 7447 | return { numDeletedRows: result } |
| 7405 | 7448 | }, |
| 7406 | 7449 | returningAll() { |
| 7407 | const retAllText = `${sqlText} RETURNING *` | |
| 7408 | const q = delParams.length > 0 | |
| 7409 | ? _sql.unsafe(retAllText, delParams) | |
| 7410 | : _sql.unsafe(retAllText) | |
| 7450 | // Deferred, as returning() is — see #1110. | |
| 7451 | const retAllText = () => `${sqlText} RETURNING *` | |
| 7452 | const build = () => (delParams.length > 0 ? _sql.unsafe(retAllText(), delParams) : _sql.unsafe(retAllText())) | |
| 7411 | 7453 | return { |
| 7412 | toSQL: () => makeExecutableQuery(q, retAllText) as any, | |
| 7413 | execute: () => runWithHooks<any[]>(q, 'delete'), | |
| 7454 | toSQL: () => makeExecutableQuery(build(), retAllText()) as any, | |
| 7455 | execute: () => runWithHooks<any[]>(build(), 'delete'), | |
| 7414 | 7456 | async executeTakeFirst() { |
| 7415 | const result = await runWithHooks<any[]>(q, 'delete') | |
| 7457 | const result = await runWithHooks<any[]>(build(), 'delete') | |
| 7416 | 7458 | return Array.isArray(result) ? result[0] : result |
| 7417 | 7459 | }, |
| 7418 | 7460 | async executeTakeFirstOrThrow() { |
| 7419 | const result = await runWithHooks<any[]>(q, 'delete') | |
| 7461 | const result = await runWithHooks<any[]>(build(), 'delete') | |
| 7420 | 7462 | const first = Array.isArray(result) ? result[0] : result |
| 7421 | 7463 | if (!first) |
| 7422 | 7464 | throw new Error('Delete with RETURNING failed') |