fix(client): place clauses by statement structure, not by text search

packages/bun-query-builder/test/client.clause-placement.test.tsadded+178-0

Changes to packages/bun-query-builder/test/client.clause-placement.test.ts

		@@ -0,0 +1,178 @@
	1	+/**
	2	+ * Clause placement reads statement structure, not text that merely looks like it.
	3	+ * stacksjs/bun-query-builder#1121.
	4	+ *
	5	+ * Three scanners decided where a clause goes, each with its own idea of what
	6	+ * SQL is:
	7	+ *
	8	+ * firstTailIndex paren depth only
	9	+ * insertJoin paren depth only
	10	+ * computeReorderedClauses paren depth + string literals
	11	+ *
	12	+ * A raw fragment is arbitrary caller text, so a keyword inside a string
	13	+ * literal or a comment was read as a clause and the splice landed inside it.
	14	+ * The statement stayed VALID, which is what made this the worst of the family:
	15	+ *
	16	+ * SELECT *, 'a WHERE id = $1 limit 3 b' as t FROM users
	17	+ * SELECT *, 'a INNER JOIN posts ON … where b' as tag FROM users
	18	+ *
	19	+ * The first has no WHERE and returns every row. The second never joins. Both
	20	+ * run without error, so the assertions here are on ROW COUNTS against the
	21	+ * control query — the emitted text alone would not prove the filter applied.
	22	+ *
	23	+ * All three now share one scanner, so they cannot disagree again.
	24	+ */
	25	+
	26	+import { mkdtempSync, rmSync } from 'node:fs'
	27	+import { tmpdir } from 'node:os'
	28	+import { join } from 'node:path'
	29	+import { Database } from 'bun:sqlite'
	30	+import { afterEach, beforeEach, describe, expect, it } from 'bun:test'
	31	+import { buildDatabaseSchema, buildSchemaMeta, createQueryBuilder, raw } from '../src'
	32	+import { config, setConfig } from '../src/config'
	33	+import { resetConnection } from '../src/db'
	34	+import { scanTopLevelKeywords } from '../src/sql-fragments'
	35	+
	36	+const MODELS = {
	37	+ users: { columns: { id: { type: 'integer', isPrimaryKey: true }, name: { type: 'string' } } },
	38	+ posts: { columns: { id: { type: 'integer', isPrimaryKey: true }, user_id: { type: 'integer' } } },
	39	+} as any
	40	+
	41	+let dir: string
	42	+let dbPath: string
	43	+let snapshot: { dialect: string, database: Record<string, unknown> }
	44	+
	45	+function qb(): any {
	46	+ return createQueryBuilder({
	47	+ schema: buildDatabaseSchema(MODELS),
	48	+ meta: buildSchemaMeta(MODELS),
	49	+ autoMigration: { enabled: false } as any,
	50	+ })
	51	+}
	52	+
	53	+beforeEach(() => {
	54	+ snapshot = { dialect: config.dialect, database: { ...config.database } }
	55	+ dir = mkdtempSync(join(tmpdir(), 'qb-1121-'))
	56	+ dbPath = join(dir, 'clause.sqlite')
	57	+ const seed = new Database(dbPath, { create: true })
	58	+ seed.run('CREATE TABLE users (id INTEGER PRIMARY KEY, name TEXT)')
	59	+ seed.run(`INSERT INTO users (id,name) VALUES (1,'a'),(2,'b'),(3,'c')`)
	60	+ seed.run('CREATE TABLE posts (id INTEGER PRIMARY KEY, user_id INTEGER)')
	61	+ seed.run('INSERT INTO posts (id,user_id) VALUES (10,1)')
	62	+ seed.close()
	63	+ setConfig({ dialect: 'sqlite', database: { database: dbPath } } as any)
	64	+ resetConnection()
	65	+})
	66	+
	67	+afterEach(() => {
	68	+ config.dialect = snapshot.dialect as any
	69	+ for (const k of Object.keys(config.database)) delete (config.database as any)[k]
	70	+ Object.assign(config.database, snapshot.database)
	71	+ resetConnection()
	72	+ rmSync(dir, { recursive: true, force: true })
	73	+})
	74	+
	75	+describe('a keyword inside a string literal is not a clause (#1121)', () => {
	76	+ it('the WHERE is not spliced into the literal, and the filter applies', async () => {
	77	+ const q = () => qb().selectFrom('users').selectRaw(raw("'a limit 3 b' as t")).where('id', '=', 1)
	78	+
	79	+ expect(String(q().toSQL())).toContain("'a limit 3 b' as t")
	80	+ expect(String(q().toSQL())).toContain('WHERE id = ?')
	81	+
	82	+ // The assertion that matters: previously 3 rows came back, not 1.
	83	+ expect((await q().execute()).map((r: any) => r.id)).toEqual([1])
	84	+ })
	85	+
	86	+ it('the JOIN is not spliced into the literal, and the join happens', async () => {
	87	+ const q = () => qb().selectFrom('users').selectRaw(raw("'a where b' as tag"))
	88	+ .innerJoin('posts', 'posts.user_id', '=', 'users.id')
	89	+
	90	+ expect(String(q().toSQL())).toContain("'a where b' as tag")
	91	+
	92	+ // Previously 3 unjoined rows; the control joins to exactly one.
	93	+ expect((await q().execute()).length).toBe(1)
	94	+ })
	95	+
	96	+ it('a genuine trailing clause is still found — the WHERE goes before LIMIT', () => {
	97	+ const sql = String(qb().selectFrom('users').where('id', '=', 1).limit(2).toSQL())
	98	+ expect(sql).toMatch(/WHERE id = \?.*LIMIT 2/)
	99	+ })
	100	+
	101	+ it('a subquery\'s own clauses are still ignored', () => {
	102	+ const sql = String(qb().selectFrom('users')
	103	+ .selectRaw(raw('(SELECT id FROM posts ORDER BY id LIMIT 1) as first_post'))
	104	+ .where('id', '=', 1)
	105	+ .toSQL())
	106	+
	107	+ expect(sql).toContain('WHERE id = ?')
	108	+ expect(sql).toContain('(SELECT id FROM posts ORDER BY id LIMIT 1)')
	109	+ })
	110	+})
	111	+
	112	+describe('a keyword inside a comment is not a clause (#1121)', () => {
	113	+ it('a block comment does not get cut in half', () => {
	114	+ const sql = String(qb().selectFrom('users').orderByRaw(raw('id desc /* where clause */')).limit(5).toSQL())
	115	+
	116	+ // Previously: `SELECT * FROM users where clause */ ORDER BY id desc /* LIMIT 5`
	117	+ expect(sql).toBe('SELECT * FROM users ORDER BY id desc /* where clause */ LIMIT 5')
	118	+ })
	119	+
	120	+ it('and the statement still runs', async () => {
	121	+ const rows = await qb().selectFrom('users').orderByRaw(raw('id desc /* where clause */')).limit(5).execute()
	122	+ expect(rows.map((r: any) => r.id)).toEqual([3, 2, 1])
	123	+ })
	124	+})
	125	+
	126	+describe('the shared scanner itself (#1121)', () => {
	127	+ const KW = [{ key: 'where', pattern: /WHERE\b/iy }, { key: 'limit', pattern: /LIMIT\b/iy }]
	128	+ const keys = (sql: string): string[] => scanTopLevelKeywords(sql, KW).map(h => h.key)
	129	+
	130	+ it('finds a real clause', () => {
	131	+ expect(keys('SELECT * FROM t WHERE id = 1 LIMIT 2')).toEqual(['where', 'limit'])
	132	+ })
	133	+
	134	+ it('skips single quotes, double quotes and backticks', () => {
	135	+ expect(keys(`SELECT 'where', "where", \`where\` FROM t`)).toEqual([])
	136	+ })
	137	+
	138	+ it('a doubled quote is an escaped one, not the end of the literal', () => {
	139	+ expect(keys(`SELECT 'it''s where' FROM t`)).toEqual([])
	140	+ })
	141	+
	142	+ it('skips line comments to end of line, but not past it', () => {
	143	+ expect(keys('SELECT 1 -- where\nFROM t WHERE id = 1')).toEqual(['where'])
	144	+ })
	145	+
	146	+ it('skips block comments, including nested ones', () => {
	147	+ expect(keys('SELECT 1 /* where /* limit */ where */ FROM t WHERE id = 1')).toEqual(['where'])
	148	+ })
	149	+
	150	+ it('skips dollar-quoted strings', () => {
	151	+ expect(keys('SELECT $$ where $$ FROM t WHERE id = 1')).toEqual(['where'])
	152	+ expect(keys('SELECT $tag$ where $tag$ FROM t WHERE id = 1')).toEqual(['where'])
	153	+ })
	154	+
	155	+ it('does not mistake a $1 placeholder for a dollar quote', () => {
	156	+ // If $1 opened a quoted run, everything after it would be skipped and the
	157	+ // real WHERE would be invisible.
	158	+ expect(keys('SELECT $1 FROM t WHERE id = $2 LIMIT 1')).toEqual(['where', 'limit'])
	159	+ })
	160	+
	161	+ it('ignores anything inside parentheses', () => {
	162	+ expect(keys('SELECT (SELECT 1 FROM x WHERE y LIMIT 1) FROM t')).toEqual([])
	163	+ })
	164	+
	165	+ it('requires a non-word character before the keyword', () => {
	166	+ expect(keys('SELECT * FROM somewhere')).toEqual([])
	167	+ })
	168	+
	169	+ it('never matches at index 0', () => {
	170	+ expect(keys('WHERE id = 1')).toEqual([])
	171	+ })
	172	+
	173	+ it('an unterminated literal swallows the rest rather than resyncing', () => {
	174	+ // The alternative — guessing where the caller meant it to end — is how a
	175	+ // splice lands inside caller text in the first place.
	176	+ expect(keys(`SELECT 'oops FROM t WHERE id = 1`)).toEqual([])
	177	+ })
	178	+})