also looking at this
fix(client): an empty insert is a no-op, not a stand-in query
#1107
2 files
+244
-1
| @@ -6657,6 +6657,50 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 6657 | 6657 | let built: any |
| 6658 | 6658 | let sqlText = '' |
| 6659 | 6659 | const params: any[] = [] |
| 6660 | // Set when `values()` is handed an empty batch. There is no statement to | |
| 6661 | // run in that case, so every terminal below answers from here instead of | |
| 6662 | // executing a stand-in query. See #1097. | |
| 6663 | let noRows = false | |
| 6664 | ||
| 6665 | /** | |
| 6666 | * What the dialect returns for an insert that affected no rows. | |
| 6667 | * | |
| 6668 | * Postgres gives `[]` for an insert without RETURNING, so an empty batch | |
| 6669 | * is indistinguishable from a real one that inserted nothing — which is | |
| 6670 | * the point. The bun:sqlite wrapper reports `{ changes, lastInsertRowid }` | |
| 6671 | * instead, so it gets zeros. | |
| 6672 | */ | |
| 6673 | const emptyInsertResult = (): any => (isPostgres ? [] : { changes: 0, lastInsertRowid: 0 }) | |
| 6674 | ||
| 6675 | /** | |
| 6676 | * The `.returning(...)` / `.returningAll()` surface for an empty batch. | |
| 6677 | * | |
| 6678 | * Mirrors the real one's method set so `.returning('id').first()` still | |
| 6679 | * resolves rather than throwing "first is not a function". No rows were | |
| 6680 | * inserted, so the row accessors answer empty and the *OrFail pair throws | |
| 6681 | * the same message they throw when a real RETURNING comes back empty. | |
| 6682 | */ | |
| 6683 | const emptyReturningBuilder = (): any => { | |
| 6684 | const noRow = async (): Promise<any> => undefined | |
| 6685 | const noRowOrFail = async (): Promise<any> => { | |
| 6686 | throw new Error('Insert with RETURNING returned no rows') | |
| 6687 | } | |
| 6688 | return { | |
| 6689 | where: () => emptyReturningBuilder(), | |
| 6690 | andWhere: () => emptyReturningBuilder(), | |
| 6691 | orWhere: () => emptyReturningBuilder(), | |
| 6692 | orderBy: () => emptyReturningBuilder(), | |
| 6693 | limit: () => emptyReturningBuilder(), | |
| 6694 | offset: () => emptyReturningBuilder(), | |
| 6695 | toSQL: () => makeExecutableQuery(null as any, '') as any, | |
| 6696 | execute: async () => [], | |
| 6697 | get: async () => [], | |
| 6698 | first: noRow, | |
| 6699 | executeTakeFirst: noRow, | |
| 6700 | firstOrFail: noRowOrFail, | |
| 6701 | executeTakeFirstOrThrow: noRowOrFail, | |
| 6702 | } | |
| 6703 | } | |
| 6660 | 6704 | const isPostgres = config.dialect === 'postgres' |
| 6661 | 6705 | |
| 6662 | 6706 | // Quote identifier based on dialect. SQLite supports double-quoted |
| @@ -6683,9 +6727,24 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 6683 | 6727 | const rows = Array.isArray(data) ? data : [data] |
| 6684 | 6728 | const rowCount = rows.length |
| 6685 | 6729 | if (rowCount === 0) { |
| 6686 | built = _sql.unsafe('SELECT 1') | |
| 6730 | // Inserting no rows is a no-op, not a query. This used to run | |
| 6731 | // `SELECT 1` as a stand-in, which meant `.execute()` resolved to a | |
| 6732 | // fabricated row — `[{ '?column?': 1 }]` on Postgres, `[{ '1': 1 }]` | |
| 6733 | // on SQLite — so a caller checking `result.length` saw 1 where the | |
| 6734 | // truth was 0, a query hook fired for an insert that never | |
| 6735 | // happened, and `.returning(...)` appended RETURNING to `SELECT 1` | |
| 6736 | // and died on a syntax error. See #1097. | |
| 6737 | // | |
| 6738 | // `values(rows)` where `rows` came back empty is ordinary calling | |
| 6739 | // code — the seeder scaffold this package generates is written that | |
| 6740 | // way — so this answers empty rather than throwing. | |
| 6741 | noRows = true | |
| 6742 | built = null | |
| 6743 | sqlText = '' | |
| 6744 | params.length = 0 | |
| 6687 | 6745 | return this |
| 6688 | 6746 | } |
| 6747 | noRows = false | |
| 6689 | 6748 | |
| 6690 | 6749 | const firstRow = rows[0] |
| 6691 | 6750 | const keys = Object.keys(firstRow) |
| @@ -6760,6 +6819,11 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 6760 | 6819 | return this |
| 6761 | 6820 | }, |
| 6762 | 6821 | returning(...cols: (keyof any & string)[]) { |
| 6822 | // Nothing was inserted, so there is nothing to return. Without this | |
| 6823 | // the RETURNING clause was appended to the `SELECT 1` stand-in and | |
| 6824 | // the statement failed to parse. | |
| 6825 | if (noRows) | |
| 6826 | return emptyReturningBuilder() | |
| 6763 | 6827 | // Append RETURNING clause to the existing SQL |
| 6764 | 6828 | const returningSql = `${sqlText} RETURNING ${cols.join(', ')}` |
| 6765 | 6829 | const q = _sql.unsafe(returningSql, params) |
| @@ -6798,10 +6862,14 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 6798 | 6862 | } |
| 6799 | 6863 | }, |
| 6800 | 6864 | toSQL() { |
| 6865 | if (noRows) | |
| 6866 | return makeExecutableQuery(null as any, '') as any | |
| 6801 | 6867 | if (!built) built = _sql.unsafe(sqlText, params) |
| 6802 | 6868 | return makeExecutableQuery(built, sqlText) as any |
| 6803 | 6869 | }, |
| 6804 | 6870 | execute() { |
| 6871 | if (noRows) | |
| 6872 | return Promise.resolve(emptyInsertResult()) | |
| 6805 | 6873 | // Ultra-fast path: use _prepareStatement to skip unsafe() and runWithHooks overhead |
| 6806 | 6874 | const hooks = activeHooks() |
| 6807 | 6875 | const hasHooks = hooks && (hooks.onQueryStart || hooks.onQueryEnd || hooks.onQueryError || hooks.startSpan || hooks.beforeCreate || hooks.afterCreate || hasSlowQueryHook(hooks)) |
| @@ -6816,11 +6884,15 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 6816 | 6884 | return runWithHooks(built, 'insert') |
| 6817 | 6885 | }, |
| 6818 | 6886 | async executeTakeFirst() { |
| 6887 | if (noRows) | |
| 6888 | return emptyInsertResult() | |
| 6819 | 6889 | if (!built) built = _sql.unsafe(sqlText, params) |
| 6820 | 6890 | const result = await runWithHooks(built, 'insert') |
| 6821 | 6891 | return result |
| 6822 | 6892 | }, |
| 6823 | 6893 | async executeTakeFirstOrThrow() { |
| 6894 | if (noRows) | |
| 6895 | throw new Error('Insert failed') | |
| 6824 | 6896 | if (!built) built = _sql.unsafe(sqlText, params) |
| 6825 | 6897 | const result = await runWithHooks(built, 'insert') |
| 6826 | 6898 | if (!result) |
| @@ -6828,6 +6900,9 @@ export function createQueryBuilder<DB extends DatabaseSchema<any>>(state?: Parti | ||
| 6828 | 6900 | return result |
| 6829 | 6901 | }, |
| 6830 | 6902 | returningAll() { |
| 6903 | // As in returning(): no rows inserted, nothing to return. | |
| 6904 | if (noRows) | |
| 6905 | return emptyReturningBuilder() | |
| 6831 | 6906 | const returningSql = `${sqlText} RETURNING *` |
| 6832 | 6907 | const q = _sql.unsafe(returningSql, params) |
| 6833 | 6908 | const runFirst = async () => { |