From cca4095833e7730bbe832b2dbe1a078c8bafd7dd Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:55:49 +0000 Subject: [PATCH 1/7] sql(sqlite): decide row-returning via column count, not a tokenizer Bun.SQL's SQLite adapter guessed whether a query returns rows by tokenizing the SQL in JavaScript. The guess missed row-returning statements it could not tokenize (quotes inside literals, leading comments, select*from, top-level VALUES) and flagged INSERT ... SELECT / WITH ... INSERT as row-returning, which discarded their affected-row counts (#30811). Prepare the statement and let sqlite3_column_count decide: statements with result columns run through the prepared statement, everything else through db.run(). Named-parameter objects keep taking the $call path added for unsafe() in both branches. The tokenizer now only produces the helper command and the statement's leading keyword, which becomes the result's command label, and it skips comments, string literals and quoted identifiers. --- src/js/internal/sql/sqlite.ts | 287 +++++++--------- test/js/sql/sqlite-sql-row-detection.test.ts | 332 +++++++++++++++++++ 2 files changed, 449 insertions(+), 170 deletions(-) create mode 100644 test/js/sql/sqlite-sql-row-detection.test.ts diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 239f982cebc1..26b9c50500f3 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -33,182 +33,126 @@ const enum SQLCommand { } interface SQLParsedInfo { + /** Helper command keyword closest to the end of the text. */ command: SQLCommand; - lastToken?: string; - canReturnRows: boolean; + /** Leading keyword of the statement, upper-cased; used for the result's `command` label. */ + firstToken: string; } -function commandToString(command: SQLCommand, lastToken?: string): string { - switch (command) { - case SQLCommand.insert: - return "INSERT"; - case SQLCommand.updateSet: - case SQLCommand.update: - return "UPDATE"; - case SQLCommand.in: - case SQLCommand.where: - if (lastToken) return lastToken; - return "WHERE"; +function isSQLWordChar(code: number): boolean { + // A-Z (text is upper-cased), 0-9, "_", "$", or non-ASCII (SQLite's IdChar accepts bytes >= 0x80). + return (code >= 65 && code <= 90) || (code >= 48 && code <= 57) || code === 95 || code === 36 || code >= 0x80; +} + +function keywordToCommand(word: string): SQLCommand { + switch (word) { + case "INSERT": + return SQLCommand.insert; + case "UPDATE": + return SQLCommand.update; + case "SET": + return SQLCommand.updateSet; + case "WHERE": + return SQLCommand.where; + case "IN": + return SQLCommand.in; default: - if (lastToken) return lastToken; - return ""; + return SQLCommand.none; } } /** - * Parse the SQL query and return the command and the last token - * @param query - The SQL query to parse - * @param partial - Whether to stop on the first command we find - * @returns The command, the last token, and whether it can return rows + * Lexes the query for the `command` label and helper detection only; whether + * it returns rows is decided by the prepared statement's column count. + * Skips comments, string literals and quoted identifiers. */ -function parseSQLQuery(query: string, partial: boolean = false): SQLParsedInfo { - const text = query.toUpperCase().trim(); - const text_len = text.length; +function parseSQLQuery(query: string): SQLParsedInfo { + const text = query.toUpperCase(); + const len = text.length; - let token = ""; let command = SQLCommand.none; - let lastToken = ""; - let canReturnRows = false; - let quoted: false | "'" | '"' = false; - // we need to reverse search so we find the closest command to the parameter - for (let i = text_len - 1; i >= 0; i--) { + let firstToken = ""; + + let i = 0; + while (i < len) { const char = text[i]; - switch (char) { - case " ": - case "\n": - case "\t": - case "\r": - case "\f": - case "\v": { - switch (token) { - case "INSERT": { - if (command === SQLCommand.none) { - command = SQLCommand.insert; - } - lastToken = token; - token = ""; - if (partial) { - return { command: SQLCommand.insert, lastToken, canReturnRows }; - } - continue; - } - case "UPDATE": { - if (command === SQLCommand.none) { - command = SQLCommand.update; - } - lastToken = token; - token = ""; - if (partial) { - return { command: SQLCommand.update, lastToken, canReturnRows }; - } - continue; - } - case "WHERE": { - if (command === SQLCommand.none) { - command = SQLCommand.where; - } - lastToken = token; - token = ""; - if (partial) { - return { command: SQLCommand.where, lastToken, canReturnRows }; - } - continue; - } - case "SET": { - if (command === SQLCommand.none) { - command = SQLCommand.updateSet; - } - lastToken = token; - token = ""; - if (partial) { - return { command: SQLCommand.updateSet, lastToken, canReturnRows }; - } - continue; - } - case "IN": { - if (command === SQLCommand.none) { - command = SQLCommand.in; - } - lastToken = token; - token = ""; - if (partial) { - return { command: SQLCommand.in, lastToken, canReturnRows }; - } - continue; - } - case "SELECT": - case "PRAGMA": - case "WITH": - case "EXPLAIN": - case "RETURNING": { - lastToken = token; - canReturnRows = true; - token = ""; - continue; - } - default: { - lastToken = token; - token = ""; + + // line comment: -- ... end of line + if (char === "-" && text[i + 1] === "-") { + i += 2; + while (i < len && text[i] !== "\n") i++; + continue; + } + // block comment: /* ... */ + if (char === "/" && text[i + 1] === "*") { + i += 2; + while (i < len && !(text[i] === "*" && text[i + 1] === "/")) i++; + i += 2; + continue; + } + // string literal: '...' with '' as an escaped quote + if (char === "'") { + i++; + while (i < len) { + if (text[i] === "'") { + if (text[i + 1] === "'") { + i += 2; continue; } + i++; + break; } + i++; } - default: { - // skip quoted commands - if (char === '"' || char === "'") { - if (quoted === char) { - quoted = false; - } else { - quoted = char; + continue; + } + // quoted identifier: "..." with "" as an escaped quote + if (char === '"') { + i++; + while (i < len) { + if (text[i] === '"') { + if (text[i + 1] === '"') { + i += 2; + continue; } - continue; - } - if (!quoted) { - token = char + token; + i++; + break; } + i++; } + continue; } - } - if (token) { - lastToken = token; - switch (token) { - case "INSERT": - if (command === SQLCommand.none) { - command = SQLCommand.insert; - } - break; - case "UPDATE": - if (command === SQLCommand.none) command = SQLCommand.update; - break; - case "WHERE": - if (command === SQLCommand.none) { - command = SQLCommand.where; - } - break; - case "SET": - if (command === SQLCommand.none) { - command = SQLCommand.updateSet; - } - break; - case "IN": - if (command === SQLCommand.none) { - command = SQLCommand.in; - } - break; - case "SELECT": - case "PRAGMA": - case "WITH": - case "EXPLAIN": - case "RETURNING": { - canReturnRows = true; - break; - } - default: - command = SQLCommand.none; - break; + // backtick and bracket identifiers + if (char === "`") { + i++; + while (i < len && text[i] !== "`") i++; + i++; + continue; + } + if (char === "[") { + i++; + while (i < len && text[i] !== "]") i++; + i++; + continue; + } + // keyword or identifier word + if (isSQLWordChar(text.charCodeAt(i))) { + const start = i; + i++; + while (i < len && isSQLWordChar(text.charCodeAt(i))) i++; + const word = text.slice(start, i); + if (firstToken === "") firstToken = word; + const cmd = keywordToCommand(word); + // keep the command keyword closest to the end of the query + if (cmd !== SQLCommand.none) command = cmd; + continue; } + + // whitespace, punctuation and operators + i++; } - return { command, lastToken, canReturnRows }; + + return { command, firstToken }; } class SQLiteQueryHandle implements BaseQueryHandle { @@ -216,13 +160,13 @@ class SQLiteQueryHandle implements BaseQueryHandle { private readonly sql: string; private readonly values: unknown[] | Record; - private readonly parsedInfo: SQLParsedInfo; + /** The result's `command` label: the statement's leading keyword. */ + private readonly command: string; public constructor(sql: string, values: unknown[] | Record) { this.sql = sql; this.values = values; - // Parse the SQL query once when creating the handle - this.parsedInfo = parseSQLQuery(sql); + this.command = parseSQLQuery(sql).firstToken; } setMode(mode: SQLQueryResultMode) { @@ -237,16 +181,18 @@ class SQLiteQueryHandle implements BaseQueryHandle { }); } - const { sql, values, mode, parsedInfo } = this; + const { sql, values, mode, command } = this; try { - const command = parsedInfo.command; - // For SELECT queries, we need to use a prepared statement - // For other queries, we can check if there are multiple statements and use db.run() if so - if (parsedInfo.canReturnRows) { - // SELECT queries must use prepared statements for results - const stmt = db.prepare(sql); - let result: unknown[] | undefined; + // A statement with result columns returns rows; preparing does not execute it. + let stmt: BunSQLiteModule.Statement | undefined; + try { + stmt = db.prepare(sql); + } catch { + // Empty or comment-only SQL does not prepare; db.run() below reports it. + } + if (stmt && stmt.native.columnsCount > 0) { + let result: unknown[] | undefined; try { // bun:sqlite binds from its first argument; spreading would let values[0] pose as the whole binding set. if (mode === SQLQueryResultMode.values) { @@ -262,16 +208,17 @@ class SQLiteQueryHandle implements BaseQueryHandle { const sqlResult = $isArray(result) ? new SQLResultArray(result) : new SQLResultArray([result]); - sqlResult.command = commandToString(command, parsedInfo.lastToken); + sqlResult.command = command; sqlResult.count = $isArray(result) ? result.length : 1; query.resolve(sqlResult); } else { - // For INSERT/UPDATE/DELETE/CREATE etc., use db.run() which handles multiple statements natively + // db.run() executes every statement in a multi-statement string. + stmt?.finalize(); const changes = db.run.$call(db, sql, values); const sqlResult = new SQLResultArray(); - sqlResult.command = commandToString(command, parsedInfo.lastToken); + sqlResult.command = command; sqlResult.count = changes.changes; sqlResult.lastInsertRowid = changes.lastInsertRowid; @@ -406,8 +353,8 @@ class SQLiteAdapter implements DatabaseAdapter { + test("LIKE predicate containing a double quote returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v TEXT)`; + await sql`INSERT INTO t VALUES (${"a"}), (${'b"c'}), (${"d"})`; + + const rows = await sql.unsafe(`select v from t where v like '%"%'`); + expect(rows).toEqual([{ v: 'b"c' }]); + }); + + test("leading block comment before SELECT returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2), (3)`; + + const rows = await sql.unsafe(`/*hdr*/select v from t`); + expect(rows).toEqual([{ v: 1 }, { v: 2 }, { v: 3 }]); + }); + + test("leading line comment before SELECT returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2)`; + + const rows = await sql.unsafe(`-- header\nselect v from t`); + expect(rows).toEqual([{ v: 1 }, { v: 2 }]); + }); + + test("SELECT with no whitespace before punctuation returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2), (3)`; + + expect(await sql.unsafe(`select*from t`)).toEqual([{ v: 1 }, { v: 2 }, { v: 3 }]); + + const paren = await sql.unsafe(`select(v)from t`); + expect(paren.map(r => Object.values(r)[0])).toEqual([1, 2, 3]); + }); + + test("top-level VALUES returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + const rows = await sql.unsafe(`values (1,'a'),(2,'b')`); + expect(rows.map(r => Object.values(r))).toEqual([ + [1, "a"], + [2, "b"], + ]); + }); + + test("INSERT ... RETURNING writes and returns the rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`; + + const returned = await sql.unsafe(`insert into t(v)values('x')returning id,v`); + expect(returned).toEqual([{ id: 1, v: "x" }]); + + const rows = await sql`SELECT v FROM t`; + expect(rows).toEqual([{ v: "x" }]); + }); + + test("row-producing PRAGMA returns rows, non-row PRAGMA does not", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + + const info = await sql.unsafe(`pragma table_info(t)`); + expect(info.length).toBe(1); + expect(info[0].name).toBe("v"); + + // A PRAGMA that sets a value yields no rows and must not throw. + const res = await sql.unsafe(`pragma user_version = 5`); + expect(res.length).toBe(0); + expect((await sql.unsafe(`pragma user_version`))[0].user_version).toBe(5); + }); + + test("command label survives comments and punctuation", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1)`; + + expect((await sql.unsafe(`/*hdr*/select v from t`)).command).toBe("SELECT"); + expect((await sql.unsafe(`select*from t`)).command).toBe("SELECT"); + }); + + test("command label is the statement's leading keyword, whatever DML keywords its body contains", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`CREATE TABLE log (id INTEGER PRIMARY KEY, n INTEGER)`; + await sql`INSERT INTO log VALUES (1, 0)`; + + const labels = { + createTrigger: (await sql.unsafe(`CREATE TRIGGER trg AFTER INSERT ON t BEGIN UPDATE log SET n = n + 1; END`)) + .command, + transactionString: (await sql.unsafe(`BEGIN; UPDATE log SET n = 5; COMMIT`)).command, + upsert: (await sql`INSERT INTO log VALUES (1, 9) ON CONFLICT(id) DO UPDATE SET n = excluded.n`).command, + update: (await sql`UPDATE log SET n = 6`).command, + deleteWhereIn: (await sql`DELETE FROM log WHERE n IN (${7})`).command, + withInsert: (await sql`WITH x AS (SELECT 1 AS v) INSERT INTO t SELECT v FROM x`).command, + explainUpdate: (await sql.unsafe(`EXPLAIN UPDATE log SET n = 1`)).command, + }; + + expect(labels).toEqual({ + createTrigger: "CREATE", + transactionString: "BEGIN", + upsert: "INSERT", + update: "UPDATE", + deleteWhereIn: "DELETE", + withInsert: "WITH", + explainUpdate: "EXPLAIN", + }); + }); + + test("helper after an unquoted non-ASCII identifier", async () => { + await using sql = new SQL("sqlite://:memory:"); + // SQLite accepts any byte >= 0x80 in an unquoted identifier. The lexer must not + // split "CAFÉIN" at the É and mistake the trailing "IN" for a WHERE IN helper. + await sql`CREATE TABLE caféin (name TEXT)`; + await sql`CREATE TABLE 数据 (name TEXT)`; + + await sql`INSERT INTO caféin ${sql({ name: "a" })}`; + await sql`INSERT INTO 数据 ${sql({ name: "b" })}`; + await sql`UPDATE caféin SET ${sql({ name: "c" })}`; + + expect(await sql`SELECT name FROM caféin`).toEqual([{ name: "c" }]); + expect(await sql`SELECT name FROM 数据`).toEqual([{ name: "b" }]); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("INSERT ... SELECT without RETURNING reports affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE company (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL)`; + await sql`INSERT INTO company (name) VALUES (${"ACME"}), (${"FOO"})`; + + const result = await sql`INSERT INTO company (name) SELECT name || ${" 2"} FROM company`; + expect(result.command).toBe("INSERT"); + expect(result.count).toBe(2); + expect(result.lastInsertRowid).toBe(4); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("WITH ... INSERT without RETURNING reports affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b'), (3, 'c')`; + + const ins = await sql`WITH cte AS (SELECT id + 10 AS id, name FROM src) INSERT INTO dst SELECT id, name FROM cte`; + expect(ins.count).toBe(3); + expect(ins.lastInsertRowid).toBe(13); + + // WITH ... SELECT must still return rows (not over-corrected). + const sel = await sql`WITH cte AS (SELECT id, name FROM src WHERE id > 1) SELECT * FROM cte ORDER BY id`; + expect(Array.from(sel)).toEqual([ + { id: 2, name: "b" }, + { id: 3, name: "c" }, + ]); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("UPDATE / DELETE with a subquery and no RETURNING report affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (2), (3)`; + await sql`INSERT INTO dst VALUES (1, 'x'), (2, 'y'), (3, 'z')`; + + const upd = await sql` + UPDATE dst SET name = ${"updated"} WHERE id IN ( + SELECT id FROM src + )`; + expect({ command: upd.command, count: upd.count, length: upd.length }).toEqual({ + command: "UPDATE", + count: 2, + length: 0, + }); + + const del = await sql` + DELETE FROM dst WHERE id IN ( + SELECT id FROM src + )`; + expect({ command: del.command, count: del.count, length: del.length }).toEqual({ + command: "DELETE", + count: 2, + length: 0, + }); + + expect(await sql`SELECT id, name FROM dst`).toEqual([{ id: 1, name: "x" }]); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("WITH ... UPDATE / DELETE / REPLACE INTO without RETURNING report affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b'), (3, 'c')`; + await sql`INSERT INTO dst VALUES (1, 'x'), (2, 'y'), (3, 'z')`; + + const upd = + await sql`WITH cte AS (SELECT id FROM src WHERE id > 1) UPDATE dst SET name = ${"updated"} WHERE id IN (SELECT id FROM cte)`; + expect(upd.count).toBe(2); + + const del = + await sql`WITH cte AS (SELECT id FROM src WHERE id > 1) DELETE FROM dst WHERE id IN (SELECT id FROM cte)`; + expect(del.count).toBe(2); + + // dst now holds only (1, 'x'); REPLACE INTO replaces it and inserts two more. + const rep = await sql`WITH cte AS (SELECT id, name FROM src) REPLACE INTO dst SELECT id, name FROM cte`; + expect(rep.count).toBe(3); + expect(await sql`SELECT id, name FROM dst ORDER BY id`).toEqual([ + { id: 1, name: "a" }, + { id: 2, name: "b" }, + { id: 3, name: "c" }, + ]); + }); + + test("INSERT ... SELECT and WITH ... INSERT with RETURNING return the inserted rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b')`; + + const plain = await sql`INSERT INTO dst SELECT id, name FROM src RETURNING id, name`; + expect(plain.count).toBe(2); + expect(Array.from(plain)).toEqual([ + { id: 1, name: "a" }, + { id: 2, name: "b" }, + ]); + + const cte = + await sql`WITH cte AS (SELECT id + 100 AS id, name FROM src) INSERT INTO dst SELECT id, name FROM cte RETURNING id, name`; + expect(cte.count).toBe(2); + expect(Array.from(cte)).toEqual([ + { id: 101, name: "a" }, + { id: 102, name: "b" }, + ]); + }); + + test("leading comment on INSERT ... SELECT still reports affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO t VALUES (1, 'Alice'), (2, 'Bob')`; + + const block = await sql`/* tag=me */ INSERT INTO t (id, name) SELECT id + 10, name || ${"!"} FROM t`; + expect({ command: block.command, count: block.count }).toEqual({ command: "INSERT", count: 2 }); + + const line = await sql`-- tag=me +INSERT INTO t (id, name) SELECT id + 20, name FROM t WHERE id <= 2`; + expect({ command: line.command, count: line.count }).toEqual({ command: "INSERT", count: 2 }); + + expect((await sql`SELECT count(*) AS n FROM t`)[0].n).toBe(6); + }); + + test("comment markers inside string literals are data, not comments", async () => { + await using sql = new SQL("sqlite://:memory:"); + + expect(await sql.unsafe(`SELECT 'hello -- world' AS quoted`)).toEqual([{ quoted: "hello -- world" }]); + expect(await sql.unsafe(`SELECT 'x /* not a comment */ y' AS quoted`)).toEqual([ + { quoted: "x /* not a comment */ y" }, + ]); + + await sql`CREATE TABLE t (name TEXT)`; + const ins = await sql.unsafe(`INSERT INTO t VALUES ('a -- b'), ('c /* d */ e')`); + expect({ command: ins.command, count: ins.count }).toEqual({ command: "INSERT", count: 2 }); + }); + + test("REPLACE as a scalar function inside WITH ... SELECT still returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (name TEXT)`; + await sql`INSERT INTO t VALUES ('apple'), ('banana')`; + + // A keyword-based classifier has to guess whether REPLACE is the statement + // or the function; the column count does not care. + const spaced = await sql.unsafe( + `WITH cte AS (SELECT REPLACE (name, 'a', 'x') AS n FROM t) SELECT * FROM cte ORDER BY n`, + ); + expect(Array.from(spaced)).toEqual([{ n: "bxnxnx" }, { n: "xpple" }]); + + const aliased = await sql.unsafe(`WITH cte AS (SELECT name AS replace FROM t ORDER BY name) SELECT * FROM cte`); + expect(Array.from(aliased)).toEqual([{ replace: "apple" }, { replace: "banana" }]); + }); + + test("EXPLAIN on a write returns the plan rows and does not execute the write", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + + const plan = await sql.unsafe(`EXPLAIN INSERT INTO t VALUES (1)`); + expect(plan.length).toBeGreaterThan(0); + expect(plan[0]).toHaveProperty("opcode"); + + expect(await sql`SELECT count(*) AS n FROM t`).toEqual([{ n: 0 }]); + }); + + test("whitespace- and comment-only queries still report 'no valid SQL statement'", async () => { + await using sql = new SQL("sqlite://:memory:"); + + // These never succeeded pre-PR either; db.run() rejects them with a clear + // message. The probe prepare must not leak a confusing "finalized" error. + for (const q of [" ", "-- noop", "/* placeholder */", "-- a\n-- b\n"]) { + await expect(Promise.resolve(sql.unsafe(q))).rejects.toThrow("Query contained no valid SQL statement"); + } + }); + + test("multi-statement writes still execute every statement", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql.unsafe(` + CREATE TABLE m1 (id INTEGER); + CREATE TABLE m2 (id INTEGER); + INSERT INTO m1 VALUES (1); + INSERT INTO m2 VALUES (2); + `); + + expect(await sql`SELECT id FROM m1`).toEqual([{ id: 1 }]); + expect(await sql`SELECT id FROM m2`).toEqual([{ id: 2 }]); + }); + + test("INSERT helper detected past a comment containing a quote", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE items (name TEXT)`; + + // The comment holds a lone apostrophe; the old reverse-scan tokenizer let + // it hijack the quote state and miss INSERT, throwing a bogus SyntaxError. + await sql`INSERT INTO items /* don't */ ${sql({ name: "a" })}`; + + const rows = await sql`SELECT name FROM items`; + expect(rows).toEqual([{ name: "a" }]); + }); +}); From 1303021bf8e3289604acb2994130f3100592daa0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:26:45 +0000 Subject: [PATCH 2/7] sql(sqlite): report prepare errors from the row-returning probe The probe swallowed every prepare failure so that empty or comment-only SQL could fall through to db.run() for its message. That also swallowed real SQLite errors such as SQLITE_BUSY; db.run() then retried the statement and, when the lock had been released in between, executed a SELECT through the row-discarding path, resolving it with an empty result. Rethrow SQLiteError from the probe and only let the no-statement plain Error fall through. --- src/js/internal/sql/sqlite.ts | 7 +++-- test/js/sql/sqlite-sql-row-detection.test.ts | 28 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 26b9c50500f3..5ac609a101db 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -187,8 +187,11 @@ class SQLiteQueryHandle implements BaseQueryHandle { let stmt: BunSQLiteModule.Statement | undefined; try { stmt = db.prepare(sql); - } catch { - // Empty or comment-only SQL does not prepare; db.run() below reports it. + } catch (err) { + // SQLite itself refused the statement (syntax error, missing table, SQLITE_BUSY, ...): + // report that. Empty or comment-only SQL yields no statement and is a plain Error; + // db.run() below reports it with a clearer message. + if (err && typeof err === "object" && "name" in err && err.name === "SQLiteError") throw err; } if (stmt && stmt.native.columnsCount > 0) { diff --git a/test/js/sql/sqlite-sql-row-detection.test.ts b/test/js/sql/sqlite-sql-row-detection.test.ts index 5586c2a8e742..864302340ebc 100644 --- a/test/js/sql/sqlite-sql-row-detection.test.ts +++ b/test/js/sql/sqlite-sql-row-detection.test.ts @@ -3,7 +3,10 @@ // heuristic silently dropped rows for valid SQL it could not tokenize and // misreported affected-row counts for INSERT ... SELECT (#30811). import { SQL } from "bun"; +import { Database } from "bun:sqlite"; import { describe, expect, test } from "bun:test"; +import { tempDir } from "harness"; +import { join } from "node:path"; describe("row-returning detection (column count, not tokenizer)", () => { test("LIKE predicate containing a double quote returns rows", async () => { @@ -305,6 +308,31 @@ INSERT INTO t (id, name) SELECT id + 20, name FROM t WHERE id <= 2`; } }); + test("a SELECT that SQLite refuses to compile rejects instead of resolving empty", async () => { + using dir = tempDir("sqlite-sql-busy", {}); + const filename = join(String(dir), "db.sqlite"); + using holder = new Database(filename); + holder.run("CREATE TABLE t (v TEXT)"); + holder.run("INSERT INTO t VALUES ('a')"); + // An exclusive lock makes the probe prepare (which needs the schema) fail + // with SQLITE_BUSY; that error must reach the caller, not send the SELECT + // through the row-discarding db.run() path. + holder.run("BEGIN EXCLUSIVE"); + + await using sql = new SQL({ adapter: "sqlite", filename }); + await expect(Promise.resolve(sql`SELECT v FROM t`)).rejects.toMatchObject({ + name: "SQLiteError", + code: "SQLITE_BUSY", + }); + + holder.run("ROLLBACK"); + expect(await sql`SELECT v FROM t`).toEqual([{ v: "a" }]); + await expect(Promise.resolve(sql`SELECT v FROM nope`)).rejects.toMatchObject({ + name: "SQLiteError", + message: "no such table: nope", + }); + }); + test("multi-statement writes still execute every statement", async () => { await using sql = new SQL("sqlite://:memory:"); await sql.unsafe(` From 67cb79f4a2c9467feab38b68b6ff3d17c8db3858 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:13:17 +0000 Subject: [PATCH 3/7] sql(sqlite): share one isSQLiteError predicate across the three catch sites --- src/js/internal/sql/sqlite.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 5ac609a101db..b90353d581a5 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -15,6 +15,11 @@ const { SQLResultArray, normalizeQuery, pushBindParam } = require("internal/sql/ const { SQLQueryResultMode } = require("internal/sql/query"); const { SQLiteError } = require("internal/sql/errors"); +/** Errors thrown by bun:sqlite itself, as opposed to plain Errors from its JS wrapper. */ +function isSQLiteError(err: unknown): err is object & { name: "SQLiteError" } { + return err !== null && typeof err === "object" && "name" in err && err.name === "SQLiteError"; +} + let lazySQLiteModule: typeof BunSQLiteModule; function getSQLiteModule() { if (!lazySQLiteModule) { @@ -188,10 +193,8 @@ class SQLiteQueryHandle implements BaseQueryHandle { try { stmt = db.prepare(sql); } catch (err) { - // SQLite itself refused the statement (syntax error, missing table, SQLITE_BUSY, ...): - // report that. Empty or comment-only SQL yields no statement and is a plain Error; - // db.run() below reports it with a clearer message. - if (err && typeof err === "object" && "name" in err && err.name === "SQLiteError") throw err; + if (isSQLiteError(err)) throw err; + // Otherwise the SQL compiled to no statement (empty or comment-only); db.run() reports that. } if (stmt && stmt.native.columnsCount > 0) { @@ -229,7 +232,7 @@ class SQLiteQueryHandle implements BaseQueryHandle { } } catch (err) { // Convert bun:sqlite errors to SQLiteError - if (err && typeof err === "object" && "name" in err && err.name === "SQLiteError") { + if (isSQLiteError(err)) { // Extract SQLite error properties const code = "code" in err ? String(err.code) : "SQLITE_ERROR"; const errno = "errno" in err ? Number(err.errno) : 1; @@ -287,7 +290,7 @@ class SQLiteAdapter implements DatabaseAdapter Date: Tue, 6 Oct 2026 18:22:03 +0000 Subject: [PATCH 4/7] sql(sqlite): run a single statement through the probe instead of preparing it twice The lexer now counts the semicolon-separated chunks that hold content. A string with one chunk runs through the prepared statement. A string with more than one chunk still goes through db.run(). A string with none skips the probe, so the prepare no longer needs a catch around it. --- src/js/internal/sql/sqlite.ts | 60 +++++++++++++++----- test/js/sql/sqlite-sql-row-detection.test.ts | 38 ++++++++++++- 2 files changed, 82 insertions(+), 16 deletions(-) diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index b90353d581a5..ae793d7cd481 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -42,6 +42,15 @@ interface SQLParsedInfo { command: SQLCommand; /** Leading keyword of the statement, upper-cased; used for the result's `command` label. */ firstToken: string; + /** + * Number of `;`-separated chunks that hold more than whitespace and comments. + * An upper bound: a `CREATE TRIGGER` body counts each of its statements. + */ + statements: number; +} + +function isSQLWhitespace(char: string): boolean { + return char === " " || char === "\n" || char === "\t" || char === "\r" || char === "\f" || char === "\v"; } function isSQLWordChar(code: number): boolean { @@ -77,6 +86,8 @@ function parseSQLQuery(query: string): SQLParsedInfo { let command = SQLCommand.none; let firstToken = ""; + let statements = 0; + let inStatement = false; let i = 0; while (i < len) { @@ -95,6 +106,20 @@ function parseSQLQuery(query: string): SQLParsedInfo { i += 2; continue; } + if (isSQLWhitespace(char)) { + i++; + continue; + } + if (char === ";") { + inStatement = false; + i++; + continue; + } + // everything below is statement content + if (!inStatement) { + inStatement = true; + statements++; + } // string literal: '...' with '' as an escaped quote if (char === "'") { i++; @@ -153,11 +178,11 @@ function parseSQLQuery(query: string): SQLParsedInfo { continue; } - // whitespace, punctuation and operators + // punctuation and operators i++; } - return { command, firstToken }; + return { command, firstToken, statements }; } class SQLiteQueryHandle implements BaseQueryHandle { @@ -167,11 +192,14 @@ class SQLiteQueryHandle implements BaseQueryHandle { private readonly values: unknown[] | Record; /** The result's `command` label: the statement's leading keyword. */ private readonly command: string; + private readonly statements: number; public constructor(sql: string, values: unknown[] | Record) { this.sql = sql; this.values = values; - this.command = parseSQLQuery(sql).firstToken; + const parsed = parseSQLQuery(sql); + this.command = parsed.firstToken; + this.statements = parsed.statements; } setMode(mode: SQLQueryResultMode) { @@ -186,16 +214,11 @@ class SQLiteQueryHandle implements BaseQueryHandle { }); } - const { sql, values, mode, command } = this; + const { sql, values, mode, command, statements } = this; try { // A statement with result columns returns rows; preparing does not execute it. - let stmt: BunSQLiteModule.Statement | undefined; - try { - stmt = db.prepare(sql); - } catch (err) { - if (isSQLiteError(err)) throw err; - // Otherwise the SQL compiled to no statement (empty or comment-only); db.run() reports that. - } + // Whitespace or comment-only input prepares to nothing; db.run() reports that as before. + const stmt = statements > 0 ? db.prepare(sql) : undefined; if (stmt && stmt.native.columnsCount > 0) { let result: unknown[] | undefined; @@ -219,9 +242,18 @@ class SQLiteQueryHandle implements BaseQueryHandle { query.resolve(sqlResult); } else { - // db.run() executes every statement in a multi-statement string. - stmt?.finalize(); - const changes = db.run.$call(db, sql, values); + let changes: BunSQLiteModule.Changes; + if (stmt && statements === 1) { + try { + changes = stmt.run.$call(stmt, values); + } finally { + stmt.finalize(); + } + } else { + // db.run() executes every statement in a multi-statement string. + stmt?.finalize(); + changes = db.run.$call(db, sql, values); + } const sqlResult = new SQLResultArray(); sqlResult.command = command; diff --git a/test/js/sql/sqlite-sql-row-detection.test.ts b/test/js/sql/sqlite-sql-row-detection.test.ts index 864302340ebc..755d19a8efc5 100644 --- a/test/js/sql/sqlite-sql-row-detection.test.ts +++ b/test/js/sql/sqlite-sql-row-detection.test.ts @@ -302,12 +302,46 @@ INSERT INTO t (id, name) SELECT id + 20, name FROM t WHERE id <= 2`; await using sql = new SQL("sqlite://:memory:"); // These never succeeded pre-PR either; db.run() rejects them with a clear - // message. The probe prepare must not leak a confusing "finalized" error. - for (const q of [" ", "-- noop", "/* placeholder */", "-- a\n-- b\n"]) { + // message, and the lexer must not hand them to the probe prepare. + for (const q of [" ", "-- noop", "/* placeholder */", "-- a\n-- b\n", ";", " ; /* x */ ;"]) { await expect(Promise.resolve(sql.unsafe(q))).rejects.toThrow("Query contained no valid SQL statement"); } }); + test("single-statement writes report changes whether or not a semicolon or comment follows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`; + + const results = { + bare: await sql.unsafe(`INSERT INTO t (v) VALUES ('a')`), + semicolon: await sql.unsafe(`INSERT INTO t (v) VALUES ('b');`), + trailingComment: await sql.unsafe(`INSERT INTO t (v) VALUES ('c'); -- done\n/* end */`), + bound: await sql`INSERT INTO t (v) VALUES (${"d"});`, + trigger: await sql.unsafe( + `CREATE TRIGGER trg AFTER INSERT ON t BEGIN UPDATE t SET v = v || '!' WHERE id = new.id; END;`, + ), + }; + expect( + Object.fromEntries( + Object.entries(results).map(([k, r]) => [k, { count: r.count, lastInsertRowid: r.lastInsertRowid }]), + ), + ).toEqual({ + bare: { count: 1, lastInsertRowid: 1 }, + semicolon: { count: 1, lastInsertRowid: 2 }, + trailingComment: { count: 1, lastInsertRowid: 3 }, + bound: { count: 1, lastInsertRowid: 4 }, + trigger: { count: 0, lastInsertRowid: 4 }, + }); + + // The trigger fires on the next insert; its UPDATE counts toward the changes. + const withTrigger = await sql`INSERT INTO t (v) VALUES (${"e"})`; + expect({ count: withTrigger.count, lastInsertRowid: withTrigger.lastInsertRowid }).toEqual({ + count: 2, + lastInsertRowid: 5, + }); + expect((await sql`SELECT v FROM t ORDER BY id`).map(r => r.v)).toEqual(["a", "b", "c", "d", "e!"]); + }); + test("a SELECT that SQLite refuses to compile rejects instead of resolving empty", async () => { using dir = tempDir("sqlite-sql-busy", {}); const filename = join(String(dir), "db.sqlite"); From 0ceb132664d14f5271da0f3384439ec674db6784 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:22:55 +0000 Subject: [PATCH 5/7] sql(sqlite): shorten two comments --- src/js/internal/sql/sqlite.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index ae793d7cd481..5e01d1a26f7d 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -42,10 +42,7 @@ interface SQLParsedInfo { command: SQLCommand; /** Leading keyword of the statement, upper-cased; used for the result's `command` label. */ firstToken: string; - /** - * Number of `;`-separated chunks that hold more than whitespace and comments. - * An upper bound: a `CREATE TRIGGER` body counts each of its statements. - */ + /** Chunks between top-level `;` that hold content. A trigger body counts each inner statement. */ statements: number; } @@ -216,8 +213,7 @@ class SQLiteQueryHandle implements BaseQueryHandle { const { sql, values, mode, command, statements } = this; try { - // A statement with result columns returns rows; preparing does not execute it. - // Whitespace or comment-only input prepares to nothing; db.run() reports that as before. + // Preparing does not execute. Input with no statement skips it and lets db.run() report that. const stmt = statements > 0 ? db.prepare(sql) : undefined; if (stmt && stmt.native.columnsCount > 0) { From 2cac064fcdb849670f99f5d91896c7e07734a363 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:31:45 +0000 Subject: [PATCH 6/7] sql(sqlite): move the row-detection tests into sqlite-sql.test.ts --- ? | Bin 0 -> 40960 bytes test/js/sql/sqlite-sql-row-detection.test.ts | 394 ------------------- test/js/sql/sqlite-sql.test.ts | 389 ++++++++++++++++++ 3 files changed, 389 insertions(+), 394 deletions(-) create mode 100644 ? delete mode 100644 test/js/sql/sqlite-sql-row-detection.test.ts diff --git a/? b/? new file mode 100644 index 0000000000000000000000000000000000000000..7356f523b55765168ecf7c98b6acc465a77c2a3c GIT binary patch literal 40960 zcmeI)%TE(Q9KiA2J}I$XE~eo?7?u#)q>7+kO%WF)7L?LP2{vWhuCd`!9$PVbk(-Gz z@j~>dp1gVTzwqKeVB!PiWa7!Iv%4*{aFBzSlJ6wDJ2N}`+nLYIW!k3mbD64biF>up zRnrz1l^#V^m9!9wqNwudmPa!=`9QNEug?8{B~>}U`!O1Or9{qrSG2dYcEAPhal6cNVTiI#$uu9@o3?J<)i?c|*Cj zqU-h5wZ=PklwdeMIH>MSx$W!gTh^vsbi%h}Lv4{;Ziqy+(j2z9m7Sg$&n}1?#zIOw zFjwl9>m%~B({p(vCCat>ntjmvnlU+^&*a2~j%!Ro$$dJDT;k z#n)RJ$%&J}N1N+S=Fg2CuD82tI6g3-?)cq+&fFD`lPE7ZL&pAbB_1q}A60c9xSWJP_wV%k>4`BQ(N_EXXR$P)T5kLR|1Q0*~0R#|0009ILIGF(Z z|C2e<3IPNVKmY**5I_I{1Q0*~fld}+|KG{8N8b@Z009ILKmY**5I_I{1Q0lxz+c6m BDPsTt literal 0 HcmV?d00001 diff --git a/test/js/sql/sqlite-sql-row-detection.test.ts b/test/js/sql/sqlite-sql-row-detection.test.ts deleted file mode 100644 index 755d19a8efc5..000000000000 --- a/test/js/sql/sqlite-sql-row-detection.test.ts +++ /dev/null @@ -1,394 +0,0 @@ -// Bun.SQL (sqlite adapter): whether a query returns rows is now decided by the -// prepared statement's column count, not by a JavaScript tokenizer. The old -// heuristic silently dropped rows for valid SQL it could not tokenize and -// misreported affected-row counts for INSERT ... SELECT (#30811). -import { SQL } from "bun"; -import { Database } from "bun:sqlite"; -import { describe, expect, test } from "bun:test"; -import { tempDir } from "harness"; -import { join } from "node:path"; - -describe("row-returning detection (column count, not tokenizer)", () => { - test("LIKE predicate containing a double quote returns rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v TEXT)`; - await sql`INSERT INTO t VALUES (${"a"}), (${'b"c'}), (${"d"})`; - - const rows = await sql.unsafe(`select v from t where v like '%"%'`); - expect(rows).toEqual([{ v: 'b"c' }]); - }); - - test("leading block comment before SELECT returns rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - await sql`INSERT INTO t VALUES (1), (2), (3)`; - - const rows = await sql.unsafe(`/*hdr*/select v from t`); - expect(rows).toEqual([{ v: 1 }, { v: 2 }, { v: 3 }]); - }); - - test("leading line comment before SELECT returns rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - await sql`INSERT INTO t VALUES (1), (2)`; - - const rows = await sql.unsafe(`-- header\nselect v from t`); - expect(rows).toEqual([{ v: 1 }, { v: 2 }]); - }); - - test("SELECT with no whitespace before punctuation returns rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - await sql`INSERT INTO t VALUES (1), (2), (3)`; - - expect(await sql.unsafe(`select*from t`)).toEqual([{ v: 1 }, { v: 2 }, { v: 3 }]); - - const paren = await sql.unsafe(`select(v)from t`); - expect(paren.map(r => Object.values(r)[0])).toEqual([1, 2, 3]); - }); - - test("top-level VALUES returns rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - const rows = await sql.unsafe(`values (1,'a'),(2,'b')`); - expect(rows.map(r => Object.values(r))).toEqual([ - [1, "a"], - [2, "b"], - ]); - }); - - test("INSERT ... RETURNING writes and returns the rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`; - - const returned = await sql.unsafe(`insert into t(v)values('x')returning id,v`); - expect(returned).toEqual([{ id: 1, v: "x" }]); - - const rows = await sql`SELECT v FROM t`; - expect(rows).toEqual([{ v: "x" }]); - }); - - test("row-producing PRAGMA returns rows, non-row PRAGMA does not", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - - const info = await sql.unsafe(`pragma table_info(t)`); - expect(info.length).toBe(1); - expect(info[0].name).toBe("v"); - - // A PRAGMA that sets a value yields no rows and must not throw. - const res = await sql.unsafe(`pragma user_version = 5`); - expect(res.length).toBe(0); - expect((await sql.unsafe(`pragma user_version`))[0].user_version).toBe(5); - }); - - test("command label survives comments and punctuation", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - await sql`INSERT INTO t VALUES (1)`; - - expect((await sql.unsafe(`/*hdr*/select v from t`)).command).toBe("SELECT"); - expect((await sql.unsafe(`select*from t`)).command).toBe("SELECT"); - }); - - test("command label is the statement's leading keyword, whatever DML keywords its body contains", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - await sql`CREATE TABLE log (id INTEGER PRIMARY KEY, n INTEGER)`; - await sql`INSERT INTO log VALUES (1, 0)`; - - const labels = { - createTrigger: (await sql.unsafe(`CREATE TRIGGER trg AFTER INSERT ON t BEGIN UPDATE log SET n = n + 1; END`)) - .command, - transactionString: (await sql.unsafe(`BEGIN; UPDATE log SET n = 5; COMMIT`)).command, - upsert: (await sql`INSERT INTO log VALUES (1, 9) ON CONFLICT(id) DO UPDATE SET n = excluded.n`).command, - update: (await sql`UPDATE log SET n = 6`).command, - deleteWhereIn: (await sql`DELETE FROM log WHERE n IN (${7})`).command, - withInsert: (await sql`WITH x AS (SELECT 1 AS v) INSERT INTO t SELECT v FROM x`).command, - explainUpdate: (await sql.unsafe(`EXPLAIN UPDATE log SET n = 1`)).command, - }; - - expect(labels).toEqual({ - createTrigger: "CREATE", - transactionString: "BEGIN", - upsert: "INSERT", - update: "UPDATE", - deleteWhereIn: "DELETE", - withInsert: "WITH", - explainUpdate: "EXPLAIN", - }); - }); - - test("helper after an unquoted non-ASCII identifier", async () => { - await using sql = new SQL("sqlite://:memory:"); - // SQLite accepts any byte >= 0x80 in an unquoted identifier. The lexer must not - // split "CAFÉIN" at the É and mistake the trailing "IN" for a WHERE IN helper. - await sql`CREATE TABLE caféin (name TEXT)`; - await sql`CREATE TABLE 数据 (name TEXT)`; - - await sql`INSERT INTO caféin ${sql({ name: "a" })}`; - await sql`INSERT INTO 数据 ${sql({ name: "b" })}`; - await sql`UPDATE caféin SET ${sql({ name: "c" })}`; - - expect(await sql`SELECT name FROM caféin`).toEqual([{ name: "c" }]); - expect(await sql`SELECT name FROM 数据`).toEqual([{ name: "b" }]); - }); - - // https://github.com/oven-sh/bun/issues/30811 - test("INSERT ... SELECT without RETURNING reports affected row count", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE company (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL)`; - await sql`INSERT INTO company (name) VALUES (${"ACME"}), (${"FOO"})`; - - const result = await sql`INSERT INTO company (name) SELECT name || ${" 2"} FROM company`; - expect(result.command).toBe("INSERT"); - expect(result.count).toBe(2); - expect(result.lastInsertRowid).toBe(4); - }); - - // https://github.com/oven-sh/bun/issues/30811 - test("WITH ... INSERT without RETURNING reports affected row count", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b'), (3, 'c')`; - - const ins = await sql`WITH cte AS (SELECT id + 10 AS id, name FROM src) INSERT INTO dst SELECT id, name FROM cte`; - expect(ins.count).toBe(3); - expect(ins.lastInsertRowid).toBe(13); - - // WITH ... SELECT must still return rows (not over-corrected). - const sel = await sql`WITH cte AS (SELECT id, name FROM src WHERE id > 1) SELECT * FROM cte ORDER BY id`; - expect(Array.from(sel)).toEqual([ - { id: 2, name: "b" }, - { id: 3, name: "c" }, - ]); - }); - - // https://github.com/oven-sh/bun/issues/30811 - test("UPDATE / DELETE with a subquery and no RETURNING report affected row count", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE src (id INTEGER PRIMARY KEY)`; - await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`INSERT INTO src VALUES (2), (3)`; - await sql`INSERT INTO dst VALUES (1, 'x'), (2, 'y'), (3, 'z')`; - - const upd = await sql` - UPDATE dst SET name = ${"updated"} WHERE id IN ( - SELECT id FROM src - )`; - expect({ command: upd.command, count: upd.count, length: upd.length }).toEqual({ - command: "UPDATE", - count: 2, - length: 0, - }); - - const del = await sql` - DELETE FROM dst WHERE id IN ( - SELECT id FROM src - )`; - expect({ command: del.command, count: del.count, length: del.length }).toEqual({ - command: "DELETE", - count: 2, - length: 0, - }); - - expect(await sql`SELECT id, name FROM dst`).toEqual([{ id: 1, name: "x" }]); - }); - - // https://github.com/oven-sh/bun/issues/30811 - test("WITH ... UPDATE / DELETE / REPLACE INTO without RETURNING report affected row count", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b'), (3, 'c')`; - await sql`INSERT INTO dst VALUES (1, 'x'), (2, 'y'), (3, 'z')`; - - const upd = - await sql`WITH cte AS (SELECT id FROM src WHERE id > 1) UPDATE dst SET name = ${"updated"} WHERE id IN (SELECT id FROM cte)`; - expect(upd.count).toBe(2); - - const del = - await sql`WITH cte AS (SELECT id FROM src WHERE id > 1) DELETE FROM dst WHERE id IN (SELECT id FROM cte)`; - expect(del.count).toBe(2); - - // dst now holds only (1, 'x'); REPLACE INTO replaces it and inserts two more. - const rep = await sql`WITH cte AS (SELECT id, name FROM src) REPLACE INTO dst SELECT id, name FROM cte`; - expect(rep.count).toBe(3); - expect(await sql`SELECT id, name FROM dst ORDER BY id`).toEqual([ - { id: 1, name: "a" }, - { id: 2, name: "b" }, - { id: 3, name: "c" }, - ]); - }); - - test("INSERT ... SELECT and WITH ... INSERT with RETURNING return the inserted rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b')`; - - const plain = await sql`INSERT INTO dst SELECT id, name FROM src RETURNING id, name`; - expect(plain.count).toBe(2); - expect(Array.from(plain)).toEqual([ - { id: 1, name: "a" }, - { id: 2, name: "b" }, - ]); - - const cte = - await sql`WITH cte AS (SELECT id + 100 AS id, name FROM src) INSERT INTO dst SELECT id, name FROM cte RETURNING id, name`; - expect(cte.count).toBe(2); - expect(Array.from(cte)).toEqual([ - { id: 101, name: "a" }, - { id: 102, name: "b" }, - ]); - }); - - test("leading comment on INSERT ... SELECT still reports affected row count", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT)`; - await sql`INSERT INTO t VALUES (1, 'Alice'), (2, 'Bob')`; - - const block = await sql`/* tag=me */ INSERT INTO t (id, name) SELECT id + 10, name || ${"!"} FROM t`; - expect({ command: block.command, count: block.count }).toEqual({ command: "INSERT", count: 2 }); - - const line = await sql`-- tag=me -INSERT INTO t (id, name) SELECT id + 20, name FROM t WHERE id <= 2`; - expect({ command: line.command, count: line.count }).toEqual({ command: "INSERT", count: 2 }); - - expect((await sql`SELECT count(*) AS n FROM t`)[0].n).toBe(6); - }); - - test("comment markers inside string literals are data, not comments", async () => { - await using sql = new SQL("sqlite://:memory:"); - - expect(await sql.unsafe(`SELECT 'hello -- world' AS quoted`)).toEqual([{ quoted: "hello -- world" }]); - expect(await sql.unsafe(`SELECT 'x /* not a comment */ y' AS quoted`)).toEqual([ - { quoted: "x /* not a comment */ y" }, - ]); - - await sql`CREATE TABLE t (name TEXT)`; - const ins = await sql.unsafe(`INSERT INTO t VALUES ('a -- b'), ('c /* d */ e')`); - expect({ command: ins.command, count: ins.count }).toEqual({ command: "INSERT", count: 2 }); - }); - - test("REPLACE as a scalar function inside WITH ... SELECT still returns rows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (name TEXT)`; - await sql`INSERT INTO t VALUES ('apple'), ('banana')`; - - // A keyword-based classifier has to guess whether REPLACE is the statement - // or the function; the column count does not care. - const spaced = await sql.unsafe( - `WITH cte AS (SELECT REPLACE (name, 'a', 'x') AS n FROM t) SELECT * FROM cte ORDER BY n`, - ); - expect(Array.from(spaced)).toEqual([{ n: "bxnxnx" }, { n: "xpple" }]); - - const aliased = await sql.unsafe(`WITH cte AS (SELECT name AS replace FROM t ORDER BY name) SELECT * FROM cte`); - expect(Array.from(aliased)).toEqual([{ replace: "apple" }, { replace: "banana" }]); - }); - - test("EXPLAIN on a write returns the plan rows and does not execute the write", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (v INTEGER)`; - - const plan = await sql.unsafe(`EXPLAIN INSERT INTO t VALUES (1)`); - expect(plan.length).toBeGreaterThan(0); - expect(plan[0]).toHaveProperty("opcode"); - - expect(await sql`SELECT count(*) AS n FROM t`).toEqual([{ n: 0 }]); - }); - - test("whitespace- and comment-only queries still report 'no valid SQL statement'", async () => { - await using sql = new SQL("sqlite://:memory:"); - - // These never succeeded pre-PR either; db.run() rejects them with a clear - // message, and the lexer must not hand them to the probe prepare. - for (const q of [" ", "-- noop", "/* placeholder */", "-- a\n-- b\n", ";", " ; /* x */ ;"]) { - await expect(Promise.resolve(sql.unsafe(q))).rejects.toThrow("Query contained no valid SQL statement"); - } - }); - - test("single-statement writes report changes whether or not a semicolon or comment follows", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`; - - const results = { - bare: await sql.unsafe(`INSERT INTO t (v) VALUES ('a')`), - semicolon: await sql.unsafe(`INSERT INTO t (v) VALUES ('b');`), - trailingComment: await sql.unsafe(`INSERT INTO t (v) VALUES ('c'); -- done\n/* end */`), - bound: await sql`INSERT INTO t (v) VALUES (${"d"});`, - trigger: await sql.unsafe( - `CREATE TRIGGER trg AFTER INSERT ON t BEGIN UPDATE t SET v = v || '!' WHERE id = new.id; END;`, - ), - }; - expect( - Object.fromEntries( - Object.entries(results).map(([k, r]) => [k, { count: r.count, lastInsertRowid: r.lastInsertRowid }]), - ), - ).toEqual({ - bare: { count: 1, lastInsertRowid: 1 }, - semicolon: { count: 1, lastInsertRowid: 2 }, - trailingComment: { count: 1, lastInsertRowid: 3 }, - bound: { count: 1, lastInsertRowid: 4 }, - trigger: { count: 0, lastInsertRowid: 4 }, - }); - - // The trigger fires on the next insert; its UPDATE counts toward the changes. - const withTrigger = await sql`INSERT INTO t (v) VALUES (${"e"})`; - expect({ count: withTrigger.count, lastInsertRowid: withTrigger.lastInsertRowid }).toEqual({ - count: 2, - lastInsertRowid: 5, - }); - expect((await sql`SELECT v FROM t ORDER BY id`).map(r => r.v)).toEqual(["a", "b", "c", "d", "e!"]); - }); - - test("a SELECT that SQLite refuses to compile rejects instead of resolving empty", async () => { - using dir = tempDir("sqlite-sql-busy", {}); - const filename = join(String(dir), "db.sqlite"); - using holder = new Database(filename); - holder.run("CREATE TABLE t (v TEXT)"); - holder.run("INSERT INTO t VALUES ('a')"); - // An exclusive lock makes the probe prepare (which needs the schema) fail - // with SQLITE_BUSY; that error must reach the caller, not send the SELECT - // through the row-discarding db.run() path. - holder.run("BEGIN EXCLUSIVE"); - - await using sql = new SQL({ adapter: "sqlite", filename }); - await expect(Promise.resolve(sql`SELECT v FROM t`)).rejects.toMatchObject({ - name: "SQLiteError", - code: "SQLITE_BUSY", - }); - - holder.run("ROLLBACK"); - expect(await sql`SELECT v FROM t`).toEqual([{ v: "a" }]); - await expect(Promise.resolve(sql`SELECT v FROM nope`)).rejects.toMatchObject({ - name: "SQLiteError", - message: "no such table: nope", - }); - }); - - test("multi-statement writes still execute every statement", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql.unsafe(` - CREATE TABLE m1 (id INTEGER); - CREATE TABLE m2 (id INTEGER); - INSERT INTO m1 VALUES (1); - INSERT INTO m2 VALUES (2); - `); - - expect(await sql`SELECT id FROM m1`).toEqual([{ id: 1 }]); - expect(await sql`SELECT id FROM m2`).toEqual([{ id: 2 }]); - }); - - test("INSERT helper detected past a comment containing a quote", async () => { - await using sql = new SQL("sqlite://:memory:"); - await sql`CREATE TABLE items (name TEXT)`; - - // The comment holds a lone apostrophe; the old reverse-scan tokenizer let - // it hijack the quote state and miss INSERT, throwing a bogus SyntaxError. - await sql`INSERT INTO items /* don't */ ${sql({ name: "a" })}`; - - const rows = await sql`SELECT name FROM items`; - expect(rows).toEqual([{ name: "a" }]); - }); -}); diff --git a/test/js/sql/sqlite-sql.test.ts b/test/js/sql/sqlite-sql.test.ts index ed0567c186fb..0f0fbb7f5ab4 100644 --- a/test/js/sql/sqlite-sql.test.ts +++ b/test/js/sql/sqlite-sql.test.ts @@ -5666,3 +5666,392 @@ describe("Unicode & Encoding Fuzzing Tests", () => { } }); }); + +// Whether a query returns rows is decided by the prepared statement's column +// count, not by a JavaScript tokenizer. The old heuristic silently dropped rows +// for valid SQL it could not tokenize and misreported affected-row counts for +// INSERT ... SELECT (#30811). +describe("row-returning detection (column count, not tokenizer)", () => { + test("LIKE predicate containing a double quote returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v TEXT)`; + await sql`INSERT INTO t VALUES (${"a"}), (${'b"c'}), (${"d"})`; + + const rows = await sql.unsafe(`select v from t where v like '%"%'`); + expect(rows).toEqual([{ v: 'b"c' }]); + }); + + test("leading block comment before SELECT returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2), (3)`; + + const rows = await sql.unsafe(`/*hdr*/select v from t`); + expect(rows).toEqual([{ v: 1 }, { v: 2 }, { v: 3 }]); + }); + + test("leading line comment before SELECT returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2)`; + + const rows = await sql.unsafe(`-- header\nselect v from t`); + expect(rows).toEqual([{ v: 1 }, { v: 2 }]); + }); + + test("SELECT with no whitespace before punctuation returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2), (3)`; + + expect(await sql.unsafe(`select*from t`)).toEqual([{ v: 1 }, { v: 2 }, { v: 3 }]); + + const paren = await sql.unsafe(`select(v)from t`); + expect(paren.map(r => Object.values(r)[0])).toEqual([1, 2, 3]); + }); + + test("top-level VALUES returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + const rows = await sql.unsafe(`values (1,'a'),(2,'b')`); + expect(rows.map(r => Object.values(r))).toEqual([ + [1, "a"], + [2, "b"], + ]); + }); + + test("INSERT ... RETURNING writes and returns the rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`; + + const returned = await sql.unsafe(`insert into t(v)values('x')returning id,v`); + expect(returned).toEqual([{ id: 1, v: "x" }]); + + const rows = await sql`SELECT v FROM t`; + expect(rows).toEqual([{ v: "x" }]); + }); + + test("row-producing PRAGMA returns rows, non-row PRAGMA does not", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + + const info = await sql.unsafe(`pragma table_info(t)`); + expect(info.length).toBe(1); + expect(info[0].name).toBe("v"); + + // A PRAGMA that sets a value yields no rows and must not throw. + const res = await sql.unsafe(`pragma user_version = 5`); + expect(res.length).toBe(0); + expect((await sql.unsafe(`pragma user_version`))[0].user_version).toBe(5); + }); + + test("command label survives comments and punctuation", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`INSERT INTO t VALUES (1)`; + + expect((await sql.unsafe(`/*hdr*/select v from t`)).command).toBe("SELECT"); + expect((await sql.unsafe(`select*from t`)).command).toBe("SELECT"); + }); + + test("command label is the statement's leading keyword, whatever DML keywords its body contains", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + await sql`CREATE TABLE log (id INTEGER PRIMARY KEY, n INTEGER)`; + await sql`INSERT INTO log VALUES (1, 0)`; + + const labels = { + createTrigger: (await sql.unsafe(`CREATE TRIGGER trg AFTER INSERT ON t BEGIN UPDATE log SET n = n + 1; END`)) + .command, + transactionString: (await sql.unsafe(`BEGIN; UPDATE log SET n = 5; COMMIT`)).command, + upsert: (await sql`INSERT INTO log VALUES (1, 9) ON CONFLICT(id) DO UPDATE SET n = excluded.n`).command, + update: (await sql`UPDATE log SET n = 6`).command, + deleteWhereIn: (await sql`DELETE FROM log WHERE n IN (${7})`).command, + withInsert: (await sql`WITH x AS (SELECT 1 AS v) INSERT INTO t SELECT v FROM x`).command, + explainUpdate: (await sql.unsafe(`EXPLAIN UPDATE log SET n = 1`)).command, + }; + + expect(labels).toEqual({ + createTrigger: "CREATE", + transactionString: "BEGIN", + upsert: "INSERT", + update: "UPDATE", + deleteWhereIn: "DELETE", + withInsert: "WITH", + explainUpdate: "EXPLAIN", + }); + }); + + test("helper after an unquoted non-ASCII identifier", async () => { + await using sql = new SQL("sqlite://:memory:"); + // SQLite accepts any byte >= 0x80 in an unquoted identifier. The lexer must not + // split "CAFÉIN" at the É and mistake the trailing "IN" for a WHERE IN helper. + await sql`CREATE TABLE caféin (name TEXT)`; + await sql`CREATE TABLE 数据 (name TEXT)`; + + await sql`INSERT INTO caféin ${sql({ name: "a" })}`; + await sql`INSERT INTO 数据 ${sql({ name: "b" })}`; + await sql`UPDATE caféin SET ${sql({ name: "c" })}`; + + expect(await sql`SELECT name FROM caféin`).toEqual([{ name: "c" }]); + expect(await sql`SELECT name FROM 数据`).toEqual([{ name: "b" }]); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("INSERT ... SELECT without RETURNING reports affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE company (id INTEGER PRIMARY KEY AUTOINCREMENT, name TEXT NOT NULL)`; + await sql`INSERT INTO company (name) VALUES (${"ACME"}), (${"FOO"})`; + + const result = await sql`INSERT INTO company (name) SELECT name || ${" 2"} FROM company`; + expect(result.command).toBe("INSERT"); + expect(result.count).toBe(2); + expect(result.lastInsertRowid).toBe(4); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("WITH ... INSERT without RETURNING reports affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b'), (3, 'c')`; + + const ins = await sql`WITH cte AS (SELECT id + 10 AS id, name FROM src) INSERT INTO dst SELECT id, name FROM cte`; + expect(ins.count).toBe(3); + expect(ins.lastInsertRowid).toBe(13); + + // WITH ... SELECT must still return rows (not over-corrected). + const sel = await sql`WITH cte AS (SELECT id, name FROM src WHERE id > 1) SELECT * FROM cte ORDER BY id`; + expect(Array.from(sel)).toEqual([ + { id: 2, name: "b" }, + { id: 3, name: "c" }, + ]); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("UPDATE / DELETE with a subquery and no RETURNING report affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (2), (3)`; + await sql`INSERT INTO dst VALUES (1, 'x'), (2, 'y'), (3, 'z')`; + + const upd = await sql` + UPDATE dst SET name = ${"updated"} WHERE id IN ( + SELECT id FROM src + )`; + expect({ command: upd.command, count: upd.count, length: upd.length }).toEqual({ + command: "UPDATE", + count: 2, + length: 0, + }); + + const del = await sql` + DELETE FROM dst WHERE id IN ( + SELECT id FROM src + )`; + expect({ command: del.command, count: del.count, length: del.length }).toEqual({ + command: "DELETE", + count: 2, + length: 0, + }); + + expect(await sql`SELECT id, name FROM dst`).toEqual([{ id: 1, name: "x" }]); + }); + + // https://github.com/oven-sh/bun/issues/30811 + test("WITH ... UPDATE / DELETE / REPLACE INTO without RETURNING report affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b'), (3, 'c')`; + await sql`INSERT INTO dst VALUES (1, 'x'), (2, 'y'), (3, 'z')`; + + const upd = + await sql`WITH cte AS (SELECT id FROM src WHERE id > 1) UPDATE dst SET name = ${"updated"} WHERE id IN (SELECT id FROM cte)`; + expect(upd.count).toBe(2); + + const del = + await sql`WITH cte AS (SELECT id FROM src WHERE id > 1) DELETE FROM dst WHERE id IN (SELECT id FROM cte)`; + expect(del.count).toBe(2); + + // dst now holds only (1, 'x'); REPLACE INTO replaces it and inserts two more. + const rep = await sql`WITH cte AS (SELECT id, name FROM src) REPLACE INTO dst SELECT id, name FROM cte`; + expect(rep.count).toBe(3); + expect(await sql`SELECT id, name FROM dst ORDER BY id`).toEqual([ + { id: 1, name: "a" }, + { id: 2, name: "b" }, + { id: 3, name: "c" }, + ]); + }); + + test("INSERT ... SELECT and WITH ... INSERT with RETURNING return the inserted rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE src (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`CREATE TABLE dst (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO src VALUES (1, 'a'), (2, 'b')`; + + const plain = await sql`INSERT INTO dst SELECT id, name FROM src RETURNING id, name`; + expect(plain.count).toBe(2); + expect(Array.from(plain)).toEqual([ + { id: 1, name: "a" }, + { id: 2, name: "b" }, + ]); + + const cte = + await sql`WITH cte AS (SELECT id + 100 AS id, name FROM src) INSERT INTO dst SELECT id, name FROM cte RETURNING id, name`; + expect(cte.count).toBe(2); + expect(Array.from(cte)).toEqual([ + { id: 101, name: "a" }, + { id: 102, name: "b" }, + ]); + }); + + test("leading comment on INSERT ... SELECT still reports affected row count", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, name TEXT)`; + await sql`INSERT INTO t VALUES (1, 'Alice'), (2, 'Bob')`; + + const block = await sql`/* tag=me */ INSERT INTO t (id, name) SELECT id + 10, name || ${"!"} FROM t`; + expect({ command: block.command, count: block.count }).toEqual({ command: "INSERT", count: 2 }); + + const line = await sql`-- tag=me +INSERT INTO t (id, name) SELECT id + 20, name FROM t WHERE id <= 2`; + expect({ command: line.command, count: line.count }).toEqual({ command: "INSERT", count: 2 }); + + expect((await sql`SELECT count(*) AS n FROM t`)[0].n).toBe(6); + }); + + test("comment markers inside string literals are data, not comments", async () => { + await using sql = new SQL("sqlite://:memory:"); + + expect(await sql.unsafe(`SELECT 'hello -- world' AS quoted`)).toEqual([{ quoted: "hello -- world" }]); + expect(await sql.unsafe(`SELECT 'x /* not a comment */ y' AS quoted`)).toEqual([ + { quoted: "x /* not a comment */ y" }, + ]); + + await sql`CREATE TABLE t (name TEXT)`; + const ins = await sql.unsafe(`INSERT INTO t VALUES ('a -- b'), ('c /* d */ e')`); + expect({ command: ins.command, count: ins.count }).toEqual({ command: "INSERT", count: 2 }); + }); + + test("REPLACE as a scalar function inside WITH ... SELECT still returns rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (name TEXT)`; + await sql`INSERT INTO t VALUES ('apple'), ('banana')`; + + // A keyword-based classifier has to guess whether REPLACE is the statement + // or the function; the column count does not care. + const spaced = await sql.unsafe( + `WITH cte AS (SELECT REPLACE (name, 'a', 'x') AS n FROM t) SELECT * FROM cte ORDER BY n`, + ); + expect(Array.from(spaced)).toEqual([{ n: "bxnxnx" }, { n: "xpple" }]); + + const aliased = await sql.unsafe(`WITH cte AS (SELECT name AS replace FROM t ORDER BY name) SELECT * FROM cte`); + expect(Array.from(aliased)).toEqual([{ replace: "apple" }, { replace: "banana" }]); + }); + + test("EXPLAIN on a write returns the plan rows and does not execute the write", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + + const plan = await sql.unsafe(`EXPLAIN INSERT INTO t VALUES (1)`); + expect(plan.length).toBeGreaterThan(0); + expect(plan[0]).toHaveProperty("opcode"); + + expect(await sql`SELECT count(*) AS n FROM t`).toEqual([{ n: 0 }]); + }); + + test("whitespace- and comment-only queries still report 'no valid SQL statement'", async () => { + await using sql = new SQL("sqlite://:memory:"); + + // These never succeeded pre-PR either; db.run() rejects them with a clear + // message, and the lexer must not hand them to the probe prepare. + for (const q of [" ", "-- noop", "/* placeholder */", "-- a\n-- b\n", ";", " ; /* x */ ;"]) { + await expect(Promise.resolve(sql.unsafe(q))).rejects.toThrow("Query contained no valid SQL statement"); + } + }); + + test("single-statement writes report changes whether or not a semicolon or comment follows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`; + + const results = { + bare: await sql.unsafe(`INSERT INTO t (v) VALUES ('a')`), + semicolon: await sql.unsafe(`INSERT INTO t (v) VALUES ('b');`), + trailingComment: await sql.unsafe(`INSERT INTO t (v) VALUES ('c'); -- done\n/* end */`), + bound: await sql`INSERT INTO t (v) VALUES (${"d"});`, + trigger: await sql.unsafe( + `CREATE TRIGGER trg AFTER INSERT ON t BEGIN UPDATE t SET v = v || '!' WHERE id = new.id; END;`, + ), + }; + expect( + Object.fromEntries( + Object.entries(results).map(([k, r]) => [k, { count: r.count, lastInsertRowid: r.lastInsertRowid }]), + ), + ).toEqual({ + bare: { count: 1, lastInsertRowid: 1 }, + semicolon: { count: 1, lastInsertRowid: 2 }, + trailingComment: { count: 1, lastInsertRowid: 3 }, + bound: { count: 1, lastInsertRowid: 4 }, + trigger: { count: 0, lastInsertRowid: 4 }, + }); + + // The trigger fires on the next insert; its UPDATE counts toward the changes. + const withTrigger = await sql`INSERT INTO t (v) VALUES (${"e"})`; + expect({ count: withTrigger.count, lastInsertRowid: withTrigger.lastInsertRowid }).toEqual({ + count: 2, + lastInsertRowid: 5, + }); + expect((await sql`SELECT v FROM t ORDER BY id`).map(r => r.v)).toEqual(["a", "b", "c", "d", "e!"]); + }); + + test("a SELECT that SQLite refuses to compile rejects instead of resolving empty", async () => { + using dir = tempDir("sqlite-sql-busy", {}); + const filename = join(String(dir), "db.sqlite"); + using holder = new Database(filename); + holder.run("CREATE TABLE t (v TEXT)"); + holder.run("INSERT INTO t VALUES ('a')"); + // An exclusive lock makes the probe prepare (which needs the schema) fail + // with SQLITE_BUSY; that error must reach the caller, not send the SELECT + // through the row-discarding db.run() path. + holder.run("BEGIN EXCLUSIVE"); + + await using sql = new SQL({ adapter: "sqlite", filename }); + await expect(Promise.resolve(sql`SELECT v FROM t`)).rejects.toMatchObject({ + name: "SQLiteError", + code: "SQLITE_BUSY", + }); + + holder.run("ROLLBACK"); + expect(await sql`SELECT v FROM t`).toEqual([{ v: "a" }]); + await expect(Promise.resolve(sql`SELECT v FROM nope`)).rejects.toMatchObject({ + name: "SQLiteError", + message: "no such table: nope", + }); + }); + + test("multi-statement writes still execute every statement", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql.unsafe(` + CREATE TABLE m1 (id INTEGER); + CREATE TABLE m2 (id INTEGER); + INSERT INTO m1 VALUES (1); + INSERT INTO m2 VALUES (2); + `); + + expect(await sql`SELECT id FROM m1`).toEqual([{ id: 1 }]); + expect(await sql`SELECT id FROM m2`).toEqual([{ id: 2 }]); + }); + + test("INSERT helper detected past a comment containing a quote", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE items (name TEXT)`; + + // The comment holds a lone apostrophe; the old reverse-scan tokenizer let + // it hijack the quote state and miss INSERT, throwing a bogus SyntaxError. + await sql`INSERT INTO items /* don't */ ${sql({ name: "a" })}`; + + const rows = await sql`SELECT name FROM items`; + expect(rows).toEqual([{ name: "a" }]); + }); +}); From 02cbd019040eb7df6826a3b1b3dec981d9fe2e3d Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Tue, 6 Oct 2026 18:35:20 +0000 Subject: [PATCH 7/7] sql(sqlite): reject surplus bindings on a single-statement write as before A bun:sqlite Statement with no parameters ignores the bindings it is given. db.run() rejects them. Route that case to db.run() so the error does not depend on how many statements the string holds. Also drop a stray SQLite file that a test wrote into the checkout. --- ? | Bin 40960 -> 0 bytes src/js/internal/sql/sqlite.ts | 4 +++- test/js/sql/sqlite-sql.test.ts | 10 ++++++++++ 3 files changed, 13 insertions(+), 1 deletion(-) delete mode 100644 ? diff --git a/? b/? deleted file mode 100644 index 7356f523b55765168ecf7c98b6acc465a77c2a3c..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 40960 zcmeI)%TE(Q9KiA2J}I$XE~eo?7?u#)q>7+kO%WF)7L?LP2{vWhuCd`!9$PVbk(-Gz z@j~>dp1gVTzwqKeVB!PiWa7!Iv%4*{aFBzSlJ6wDJ2N}`+nLYIW!k3mbD64biF>up zRnrz1l^#V^m9!9wqNwudmPa!=`9QNEug?8{B~>}U`!O1Or9{qrSG2dYcEAPhal6cNVTiI#$uu9@o3?J<)i?c|*Cj zqU-h5wZ=PklwdeMIH>MSx$W!gTh^vsbi%h}Lv4{;Ziqy+(j2z9m7Sg$&n}1?#zIOw zFjwl9>m%~B({p(vCCat>ntjmvnlU+^&*a2~j%!Ro$$dJDT;k z#n)RJ$%&J}N1N+S=Fg2CuD82tI6g3-?)cq+&fFD`lPE7ZL&pAbB_1q}A60c9xSWJP_wV%k>4`BQ(N_EXXR$P)T5kLR|1Q0*~0R#|0009ILIGF(Z z|C2e<3IPNVKmY**5I_I{1Q0*~fld}+|KG{8N8b@Z009ILKmY**5I_I{1Q0lxz+c6m BDPsTt diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 5e01d1a26f7d..83ce0e480a29 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -239,7 +239,9 @@ class SQLiteQueryHandle implements BaseQueryHandle { query.resolve(sqlResult); } else { let changes: BunSQLiteModule.Changes; - if (stmt && statements === 1) { + // A Statement with no parameters ignores bindings; db.run() rejects surplus ones. + const surplusBindings = stmt?.paramsCount === 0 && $isArray(values) && values.length > 0; + if (stmt && statements === 1 && !surplusBindings) { try { changes = stmt.run.$call(stmt, values); } finally { diff --git a/test/js/sql/sqlite-sql.test.ts b/test/js/sql/sqlite-sql.test.ts index 0f0fbb7f5ab4..c9abb7fd7ec1 100644 --- a/test/js/sql/sqlite-sql.test.ts +++ b/test/js/sql/sqlite-sql.test.ts @@ -5971,6 +5971,16 @@ INSERT INTO t (id, name) SELECT id + 20, name FROM t WHERE id <= 2`; } }); + test("surplus bindings on a write with no placeholders still reject", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (v INTEGER)`; + + for (const q of ["INSERT INTO t VALUES (1)", "INSERT INTO t VALUES (1); INSERT INTO t VALUES (2)"]) { + await expect(Promise.resolve(sql.unsafe(q, [42]))).rejects.toThrow("SQLite query expected 0 values, received 1"); + } + expect(await sql`SELECT count(*) AS n FROM t`).toEqual([{ n: 0 }]); + }); + test("single-statement writes report changes whether or not a semicolon or comment follows", async () => { await using sql = new SQL("sqlite://:memory:"); await sql`CREATE TABLE t (id INTEGER PRIMARY KEY, v TEXT)`;