From dcdf2537e810213dd6e2ee352a7a7ad4bf1dd20d Mon Sep 17 00:00:00 2001 From: Claude Bot Date: Sat, 3 Jan 2026 20:11:47 +0000 Subject: [PATCH] fix(sql): filter out undefined values in INSERT helper instead of treating as NULL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, the sql() helper would convert undefined values to NULL for INSERT statements, which could cause issues when inserting into NOT NULL columns. This change makes INSERT behave consistently with UPDATE, which already skips undefined values. Now when using `sql({ foo: undefined, id: 1 })` in an INSERT: - The `foo` column is omitted entirely from the query - Only columns with defined values are included - If all columns have undefined values, a descriptive error is thrown This aligns with the expected JavaScript behavior where undefined typically means "not present" rather than "explicitly null". Fixes #22156 (comment from EvHaus about INSERT statements) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/js/internal/sql/mysql.ts | 54 ++++++++++++++++-------------- src/js/internal/sql/postgres.ts | 58 +++++++++++++++++++-------------- src/js/internal/sql/sqlite.ts | 45 ++++++++++++++----------- test/js/sql/sqlite-sql.test.ts | 34 +++++++++++++++++++ 4 files changed, 124 insertions(+), 67 deletions(-) diff --git a/src/js/internal/sql/mysql.ts b/src/js/internal/sql/mysql.ts index 9dce2eb17c..946571516c 100644 --- a/src/js/internal/sql/mysql.ts +++ b/src/js/internal/sql/mysql.ts @@ -455,11 +455,10 @@ class PooledMySQLConnection { } } -class MySQLAdapter implements DatabaseAdapter< - PooledMySQLConnection, - $ZigGeneratedClasses.MySQLConnection, - $ZigGeneratedClasses.MySQLQuery -> { +class MySQLAdapter + implements + DatabaseAdapter +{ public readonly connectionInfo: Bun.SQL.__internal.DefinedPostgresOrMySQLOptions; public readonly connections: PooledMySQLConnection[]; @@ -1020,10 +1019,25 @@ class MySQLAdapter implements DatabaseAdapter< // insert into users ${sql(users)} or insert into users ${sql(user)} // - query += "("; + // Filter out columns with undefined values (use first item to determine columns) + const firstItem = $isArray(items) ? items[0] : items; + const definedColumns: string[] = []; for (let j = 0; j < columnCount; j++) { - query += this.escapeIdentifier(columns[j]); - if (j < lastColumnIndex) { + const column = columns[j]; + if (typeof firstItem[column] !== "undefined") { + definedColumns.push(column); + } + } + const definedColumnCount = definedColumns.length; + if (definedColumnCount === 0) { + throw new SyntaxError("Insert needs to have at least one column with a defined value"); + } + const lastDefinedColumnIndex = definedColumnCount - 1; + + query += "("; + for (let j = 0; j < definedColumnCount; j++) { + query += this.escapeIdentifier(definedColumns[j]); + if (j < lastDefinedColumnIndex) { query += ", "; } } @@ -1034,15 +1048,11 @@ class MySQLAdapter implements DatabaseAdapter< for (let j = 0; j < itemsCount; j++) { query += "("; const item = items[j]; - for (let k = 0; k < columnCount; k++) { - const column = columns[k]; + for (let k = 0; k < definedColumnCount; k++) { + const column = definedColumns[k]; const columnValue = item[column]; - query += `?${k < lastColumnIndex ? ", " : ""}`; - if (typeof columnValue === "undefined") { - binding_values.push(null); - } else { - binding_values.push(columnValue); - } + query += `?${k < lastDefinedColumnIndex ? ", " : ""}`; + binding_values.push(columnValue); } if (j < lastItemIndex) { query += "),"; @@ -1053,15 +1063,11 @@ class MySQLAdapter implements DatabaseAdapter< } else { query += "("; const item = items; - for (let j = 0; j < columnCount; j++) { - const column = columns[j]; + for (let j = 0; j < definedColumnCount; j++) { + const column = definedColumns[j]; const columnValue = item[column]; - query += `?${j < lastColumnIndex ? ", " : ""}`; - if (typeof columnValue === "undefined") { - binding_values.push(null); - } else { - binding_values.push(columnValue); - } + query += `?${j < lastDefinedColumnIndex ? ", " : ""}`; + binding_values.push(columnValue); } query += ") "; // the user can add RETURNING * or RETURNING id } diff --git a/src/js/internal/sql/postgres.ts b/src/js/internal/sql/postgres.ts index 99123d93a3..08d0afbce0 100644 --- a/src/js/internal/sql/postgres.ts +++ b/src/js/internal/sql/postgres.ts @@ -672,11 +672,14 @@ class PooledPostgresConnection { } } -class PostgresAdapter implements DatabaseAdapter< - PooledPostgresConnection, - $ZigGeneratedClasses.PostgresSQLConnection, - $ZigGeneratedClasses.PostgresSQLQuery -> { +class PostgresAdapter + implements + DatabaseAdapter< + PooledPostgresConnection, + $ZigGeneratedClasses.PostgresSQLConnection, + $ZigGeneratedClasses.PostgresSQLQuery + > +{ public readonly connectionInfo: Bun.SQL.__internal.DefinedPostgresOrMySQLOptions; public readonly connections: PooledPostgresConnection[]; @@ -1249,10 +1252,25 @@ class PostgresAdapter implements DatabaseAdapter< // insert into users ${sql(users)} or insert into users ${sql(user)} // - query += "("; + // Filter out columns with undefined values (use first item to determine columns) + const firstItem = $isArray(items) ? items[0] : items; + const definedColumns: string[] = []; for (let j = 0; j < columnCount; j++) { - query += this.escapeIdentifier(columns[j]); - if (j < lastColumnIndex) { + const column = columns[j]; + if (typeof firstItem[column] !== "undefined") { + definedColumns.push(column); + } + } + const definedColumnCount = definedColumns.length; + if (definedColumnCount === 0) { + throw new SyntaxError("Insert needs to have at least one column with a defined value"); + } + const lastDefinedColumnIndex = definedColumnCount - 1; + + query += "("; + for (let j = 0; j < definedColumnCount; j++) { + query += this.escapeIdentifier(definedColumns[j]); + if (j < lastDefinedColumnIndex) { query += ", "; } } @@ -1263,15 +1281,11 @@ class PostgresAdapter implements DatabaseAdapter< for (let j = 0; j < itemsCount; j++) { query += "("; const item = items[j]; - for (let k = 0; k < columnCount; k++) { - const column = columns[k]; + for (let k = 0; k < definedColumnCount; k++) { + const column = definedColumns[k]; const columnValue = item[column]; - query += `$${binding_idx++}${k < lastColumnIndex ? ", " : ""}`; - if (typeof columnValue === "undefined") { - binding_values.push(null); - } else { - binding_values.push(columnValue); - } + query += `$${binding_idx++}${k < lastDefinedColumnIndex ? ", " : ""}`; + binding_values.push(columnValue); } if (j < lastItemIndex) { query += "),"; @@ -1282,15 +1296,11 @@ class PostgresAdapter implements DatabaseAdapter< } else { query += "("; const item = items; - for (let j = 0; j < columnCount; j++) { - const column = columns[j]; + for (let j = 0; j < definedColumnCount; j++) { + const column = definedColumns[j]; const columnValue = item[column]; - query += `$${binding_idx++}${j < lastColumnIndex ? ", " : ""}`; - if (typeof columnValue === "undefined") { - binding_values.push(null); - } else { - binding_values.push(columnValue); - } + query += `$${binding_idx++}${j < lastDefinedColumnIndex ? ", " : ""}`; + binding_values.push(columnValue); } query += ") "; // the user can add RETURNING * or RETURNING id } diff --git a/src/js/internal/sql/sqlite.ts b/src/js/internal/sql/sqlite.ts index 9899948908..648f2c29f7 100644 --- a/src/js/internal/sql/sqlite.ts +++ b/src/js/internal/sql/sqlite.ts @@ -433,10 +433,25 @@ class SQLiteAdapter implements DatabaseAdapter { await sqlSafe.close(); }); + test("insert helper filters out undefined values", async () => { + await sql`CREATE TABLE insert_undefined_test (id INTEGER PRIMARY KEY, name TEXT NOT NULL, optional TEXT)`; + + // Insert with undefined value - should only include defined columns + await sql`INSERT INTO insert_undefined_test ${sql({ id: 1, name: "test", optional: undefined })}`; + + const result = await sql`SELECT * FROM insert_undefined_test WHERE id = 1`; + expect(result).toHaveLength(1); + expect(result[0].id).toBe(1); + expect(result[0].name).toBe("test"); + expect(result[0].optional).toBe(null); // SQLite default + + // Insert with all defined values - should work normally + await sql`INSERT INTO insert_undefined_test ${sql({ id: 2, name: "test2", optional: "value" })}`; + const result2 = await sql`SELECT * FROM insert_undefined_test WHERE id = 2`; + expect(result2[0].optional).toBe("value"); + + // Bulk insert with undefined values + await sql`INSERT INTO insert_undefined_test ${sql([ + { id: 3, name: "bulk1", optional: undefined }, + { id: 4, name: "bulk2", optional: undefined }, + ])}`; + const result3 = await sql`SELECT * FROM insert_undefined_test WHERE id IN (3, 4) ORDER BY id`; + expect(result3).toHaveLength(2); + expect(result3[0].name).toBe("bulk1"); + expect(result3[1].name).toBe("bulk2"); + + // Insert with all undefined except one column should throw + expect( + async () => + await sql`INSERT INTO insert_undefined_test ${sql({ id: undefined, name: undefined, optional: undefined })}`.execute(), + ).toThrow("Insert needs to have at least one column with a defined value"); + }); + test("invalid keys for helper throw immediately", () => { const obj = { id: 1, text_val: "x" }; expect(() => sql`INSERT INTO helper_invalid ${sql(obj, Symbol("k") as any)}`).toThrowErrorMatchingInlineSnapshot(