fix(client): unsafe() refuses ?-style bindings on Postgres

Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.

packages/bun-query-builder/test/client.unsafe-placeholders-refused.test.tsmodified+28-21

Changes to packages/bun-query-builder/test/client.unsafe-placeholders-refused.test.ts

		⋯94 linesShow the 94 lines above this hunk@@ -95,21 +95,22 @@ describe('unsafe() placeholder style (#1171)', () => {
95	95	beforeEach(() => { config.dialect = 'postgres' as any })
96	96
97	97	it('accepts $n, in any order', () => {
98		- // The shape db.d.ts documents. Throwing here would be the false positive
99		- // that matters, so assert the guard specifically, not the connection.
100		- expect(() => assertOnly(() => qb().unsafe('SELECT $2 AS a, $1 AS b', ['ONE', 'TWO']))).not.toThrow(REFUSED)
	98	+ // The shape db.d.ts documents. A throw here is the false positive that
	99	+ // would matter, so this asserts nothing is thrown at all.
	100	+ expect(refusal(() => qb().unsafe('SELECT $2 AS a, $1 AS b', ['ONE', 'TWO']))).toBeNull()
101	101	})
102	102
103	103	it('accepts a jsonb ? operator when $n is also present', () => {
104		- // `data ? 'key'` is a real Postgres operator, not a placeholder.
105		- expect(() => assertOnly(() => qb().unsafe('SELECT * FROM t WHERE data ? \'key\' AND id = $1', [7]))).not.toThrow(REFUSED)
	104	+ // `data ? 'key'` is a real Postgres operator, not a placeholder. Keying the
	105	+ // check on `?` instead of on the absence of $n would reject this.
	106	+ expect(refusal(() => qb().unsafe('SELECT * FROM t WHERE data ? \'key\' AND id = $1', [7]))).toBeNull()
106	107	})
107	108
108	109	it('never inspects a query with no bindings', () => {
109	110	// Raw DDL, and the `?` in a literal that is data rather than a placeholder.
110		- expect(() => assertOnly(() => qb().unsafe('CREATE TABLE t (id int)'))).not.toThrow(REFUSED)
111		- expect(() => assertOnly(() => qb().unsafe('SELECT * FROM users WHERE name = \'what?\''))).not.toThrow(REFUSED)
112		- expect(() => assertOnly(() => qb().unsafe('SELECT * FROM users WHERE email = ?', []))).not.toThrow(REFUSED)
	111	+ expect(refusal(() => qb().unsafe('CREATE TABLE t (id int)'))).toBeNull()
	112	+ expect(refusal(() => qb().unsafe('SELECT * FROM users WHERE name = \'what?\''))).toBeNull()
	113	+ expect(refusal(() => qb().unsafe('SELECT * FROM users WHERE email = ?', []))).toBeNull()
113	114	})
114	115	})
115	116
		⋯1 lineShow the 1 line above this hunk@@ -117,29 +118,35 @@ describe('unsafe() placeholder style (#1171)', () => {
117	118	it('SQLite and MySQL still take ?', () => {
118	119	for (const d of ['sqlite', 'mysql'] as const) {
119	120	config.dialect = d as any
120		- expect(() => assertOnly(() => qb().unsafe('SELECT * FROM users WHERE email = ? LIMIT 1', ['a@b.c'])))
121		- .not.toThrow(REFUSED)
	121	+ expect(refusal(() => qb().unsafe('SELECT * FROM users WHERE email = ? LIMIT 1', ['a@b.c']))).toBeNull()
122	122	}
123	123	})
124	124	})
125	125	})
126	126
127	127	/**
128		- * Run `fn` and swallow anything that is not this guard's refusal.
	128	+ * The message `fn` threw, or null if it returned.
129	129	*
130		- * These cases assert that the GUARD does not fire. Past it the call reaches a
131		- * connection that no test here configures, so a driver error is expected and
132		- * says nothing about the thing under test; only a `[query-builder] unsafe()`
133		- * message would.
	130	+ * `unsafe` returns a LAZY query: it reaches a database when something awaits it,
	131	+ * and not before. So these cases call it and discard the result, which exercises
	132	+ * the guard while executing nothing.
	133	+ *
	134	+ * That is load-bearing, not tidiness. An earlier version of this helper did
	135	+ * `void Promise.resolve(out).catch(…)` to ignore driver errors — but
	136	+ * `Promise.resolve` calls `.then` on the thenable, so every "leaves it alone"
	137	+ * case below really ran, `CREATE TABLE t (id int)` included, against whatever
	138	+ * database the environment pointed at. In CI that is the shared `test_db`, and
	139	+ * the unawaited queries outlived the file and timed out the next one's
	140	+ * `beforeAll` (`closeConnection` → `resetDatabase` → `generateMigration`, on a
	141	+ * 5s hook budget). Returning the message instead of swallowing it also means an
	142	+ * unexpected throw fails loudly rather than passing as "not refused".
134	143	*/
135		-function assertOnly(fn: () => unknown): void {
	144	+function refusal(fn: () => unknown): string | null {
136	145	try {
137		- const out = fn()
138		- // A promise that rejects in the driver is not this guard's business either.
139		- void Promise.resolve(out).catch(() => {})
	146	+ fn()
	147	+ return null
140	148	}
141	149	catch (error) {
142		- if (REFUSED.test((error as Error).message))
143		- throw error
	150	+ return (error as Error).message
144	151	}
145	152	}