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(