fix(client): an empty insert is a no-op, not a stand-in query

packages/bun-query-builder/test/client.insert-empty-values.test.tsadded+168-0

Changes to packages/bun-query-builder/test/client.insert-empty-values.test.ts

		@@ -0,0 +1,168 @@
	1	+/**
	2	+ * `insertInto(t).values([])` must be a no-op, not a stand-in query.
	3	+ * stacksjs/bun-query-builder#1097.
	4	+ *
	5	+ * It ran `SELECT 1` when handed an empty batch, which is invisible in the
	6	+ * database — no row is ever written — but wrong at every other layer:
	7	+ *
	8	+ * - `.execute()` resolved to a FABRICATED row: `[{ '?column?': 1 }]` on
	9	+ * Postgres, `[{ '1': 1 }]` on SQLite. A caller checking `result.length` saw
	10	+ * 1 where the truth was 0.
	11	+ * - a query hook fired, labelled `insert`, for an insert that never happened.
	12	+ * - `.returning(...)` appended RETURNING to `SELECT 1` and died with
	13	+ * `near "RETURNING": syntax error`.
	14	+ *
	15	+ * It answers empty rather than throwing because `values(rows)` where `rows`
	16	+ * came back empty is ordinary calling code — the seeder scaffold this package
	17	+ * generates (`actions/seed.ts`) is written exactly that way, so a throw would
	18	+ * turn an empty dataset into a failed seed.
	19	+ */
	20	+
	21	+import { mkdtempSync, rmSync } from 'node:fs'
	22	+import { tmpdir } from 'node:os'
	23	+import { join } from 'node:path'
	24	+import { Database } from 'bun:sqlite'
	25	+import { afterEach, beforeEach, describe, expect, it } from 'bun:test'
	26	+import { buildDatabaseSchema, buildSchemaMeta, createQueryBuilder } from '../src'
	27	+import { config, setConfig } from '../src/config'
	28	+import { resetConnection } from '../src/db'
	29	+
	30	+const MODELS = {
	31	+ t: { columns: { id: { type: 'integer', isPrimaryKey: true }, name: { type: 'string' } } },
	32	+} as any
	33	+
	34	+// A file rather than `:memory:`: the client opens its own connection from the
	35	+// configured path, and a `:memory:` database is per-connection, so the table
	36	+// created here would be invisible to it.
	37	+let dir: string
	38	+let dbPath: string
	39	+let snapshot: { dialect: string, database: Record<string, unknown>, hooks: unknown }
	40	+
	41	+function qb(): any {
	42	+ return createQueryBuilder({
	43	+ schema: buildDatabaseSchema(MODELS),
	44	+ meta: buildSchemaMeta(MODELS),
	45	+ autoMigration: { enabled: false } as any,
	46	+ })
	47	+}
	48	+
	49	+/** Row count read through a fresh connection, not the builder's. */
	50	+function rowCount(): number {
	51	+ const probe = new Database(dbPath)
	52	+ const [{ c }] = probe.query('SELECT count(*) c FROM t').all() as any[]
	53	+ probe.close()
	54	+ return c
	55	+}
	56	+
	57	+beforeEach(() => {
	58	+ snapshot = { dialect: config.dialect, database: { ...config.database }, hooks: (config as any).hooks }
	59	+ dir = mkdtempSync(join(tmpdir(), 'qb-1097-'))
	60	+ dbPath = join(dir, 'empty-values.sqlite')
	61	+ const seed = new Database(dbPath, { create: true })
	62	+ seed.run('CREATE TABLE t (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT)')
	63	+ seed.close()
	64	+ setConfig({ dialect: 'sqlite', database: { database: dbPath } } as any)
	65	+ resetConnection()
	66	+})
	67	+
	68	+afterEach(() => {
	69	+ config.dialect = snapshot.dialect as any
	70	+ for (const k of Object.keys(config.database)) delete (config.database as any)[k]
	71	+ Object.assign(config.database, snapshot.database)
	72	+ ;(config as any).hooks = snapshot.hooks
	73	+ resetConnection()
	74	+ rmSync(dir, { recursive: true, force: true })
	75	+})
	76	+
	77	+describe('insertInto().values([]) is a no-op (#1097)', () => {
	78	+ it('reports nothing inserted instead of a fabricated row', async () => {
	79	+ const result: any = await qb().insertInto('t').values([]).execute()
	80	+
	81	+ // The bun:sqlite wrapper reports { changes, lastInsertRowid }; zeros are
	82	+ // the honest answer. What must never come back is a row.
	83	+ expect(Array.isArray(result)).toBe(false)
	84	+ expect(result.changes).toBe(0)
	85	+
	86	+ expect(rowCount()).toBe(0)
	87	+ })
	88	+
	89	+ it('does not run a query, so no hook fires', async () => {
	90	+ const seen: string[] = []
	91	+ setConfig({ hooks: { onQueryStart: () => { seen.push('start') } } } as any)
	92	+
	93	+ await qb().insertInto('t').values([]).execute()
	94	+
	95	+ expect(seen).toEqual([])
	96	+ })
	97	+
	98	+ it('emits no SQL', () => {
	99	+ const sql = qb().insertInto('t').values([]).toSQL()
	100	+ expect(String(sql?.sql ?? '')).toBe('')
	101	+ })
	102	+
	103	+ describe('returning()', () => {
	104	+ it('resolves empty instead of failing to parse', async () => {
	105	+ // Previously: `near "RETURNING": syntax error`, because RETURNING was
	106	+ // appended to the `SELECT 1` stand-in.
	107	+ const rows = await qb().insertInto('t').values([]).returning('id').execute()
	108	+ expect(rows).toEqual([])
	109	+ })
	110	+
	111	+ it('keeps the row accessors present and empty', async () => {
	112	+ const b = qb().insertInto('t').values([]).returning('id')
	113	+
	114	+ // `.returning(...).first()` exists on the real builder, so it has to
	115	+ // exist here too rather than throwing "first is not a function".
	116	+ expect(await b.get()).toEqual([])
	117	+ expect(await b.first()).toBeUndefined()
	118	+ expect(await b.executeTakeFirst()).toBeUndefined()
	119	+ await expect(b.firstOrFail()).rejects.toThrow(/returned no rows/)
	120	+ })
	121	+
	122	+ it('behaves the same through returningAll()', async () => {
	123	+ const rows = await qb().insertInto('t').values([]).returningAll().execute()
	124	+ expect(rows).toEqual([])
	125	+ })
	126	+ })
	127	+
	128	+ it('leaves a non-empty insert alone', async () => {
	129	+ const seen: string[] = []
	130	+ setConfig({ hooks: { onQueryStart: () => { seen.push('start') } } } as any)
	131	+
	132	+ const result: any = await qb().insertInto('t').values([{ name: 'a' }]).execute()
	133	+ expect(result.changes).toBe(1)
	134	+ expect(seen.length).toBe(1)
	135	+
	136	+ const rows = await qb().insertInto('t').values([{ name: 'b' }]).returning('id').execute()
	137	+ expect(rows).toHaveLength(1)
	138	+
	139	+ expect(rowCount()).toBe(2)
	140	+ })
	141	+
	142	+ it('reuses the builder correctly when a later call has rows', async () => {
	143	+ // The empty batch sets a flag; a subsequent values() with rows has to clear
	144	+ // it, or the second insert would silently do nothing.
	145	+ const b = qb().insertInto('t')
	146	+ b.values([])
	147	+ b.values([{ name: 'later' }])
	148	+ const result: any = await b.execute()
	149	+
	150	+ expect(result.changes).toBe(1)
	151	+ expect(rowCount()).toBe(1)
	152	+ })
	153	+})
	154	+
	155	+describe('insertInto().values([]) on Postgres (#1097)', () => {
	156	+ it('resolves to [] — what a real insert affecting no rows returns', async () => {
	157	+ // No connection is opened: the empty batch short-circuits before touching
	158	+ // the driver, which is what makes this assertable without a live server.
	159	+ setConfig({ dialect: 'postgres', database: { url: 'postgres://unused:5432/none' } } as any)
	160	+ resetConnection()
	161	+
	162	+ const result = await qb().insertInto('t').values([]).execute()
	163	+
	164	+ // Postgres returns [] for an insert without RETURNING, so an empty batch is
	165	+ // deliberately indistinguishable from a real one that inserted nothing.
	166	+ expect(result).toEqual([])
	167	+ })
	168	+})