diff --git a/src/js/internal/sql/postgres.ts b/src/js/internal/sql/postgres.ts index 897f6526c82b..11392cc372a5 100644 --- a/src/js/internal/sql/postgres.ts +++ b/src/js/internal/sql/postgres.ts @@ -146,6 +146,12 @@ function arrayValueSerializer(type: ArrayType, is_numeric: boolean, is_json: boo // we do minimal to none type validation, we just try to format nicely and let the server handle if is valid SQL // postgres will try to convert string -> array type // postgres will emit a nice error saying what value dont have the expected format outputing the value in the error + if (value === null) { + // Unquoted null is SQL NULL in array literal syntax. typeof null is "object", + // so without this check the default branch would JSON.stringify it and emit + // the quoted string "null". + return "null"; + } if ($isArray(value) || isTypedArray(value)) { if (!value.length) return "{}"; const delimiter = type === "BOX" ? ";" : ","; diff --git a/src/js/internal/sql/shared.ts b/src/js/internal/sql/shared.ts index 21c781812aa6..91695c835bc2 100644 --- a/src/js/internal/sql/shared.ts +++ b/src/js/internal/sql/shared.ts @@ -147,41 +147,75 @@ function normalizeSSLMode(value: string): SSLMode { throw $ERR_INVALID_ARG_VALUE("sslmode", value, "must be one of: disable, prefer, require, verify-ca, verify-full"); } +/** + * Reject helper keys that are neither identifier-like strings nor small + * non-negative integers. Numeric strings are coerced first so the same rule + * applies regardless of how the key was spelled. + */ +function validateHelperKey(key: unknown): void { + if (typeof key === "string") { + const asNumber = Number(key); + if (Number.isNaN(asNumber)) return; + key = asNumber; + } + if (typeof key !== "string") { + if (Number.isSafeInteger(key) && (key as number) >= 0 && (key as number) <= 64 * 1024) return; + throw new Error(`Keys must be strings or numbers: ${String(key)}`); + } +} + export type { SQLHelper }; class SQLHelper { public readonly value: T; public readonly columns: (keyof T)[]; + /** + * True when the column list was derived from the first row's own keys + * rather than supplied explicitly by the caller. The INSERT path uses this + * to widen the column list to the union of keys across every row so that + * keys first appearing on a later row are not silently dropped. + */ + public readonly autoColumns: boolean; constructor(value: T, keys?: (keyof T)[]) { + let autoColumns = false; if (keys !== undefined && keys.length === 0 && ($isObject(value[0]) || $isArray(value[0]))) { keys = Object.keys(value[0]) as (keyof T)[]; + autoColumns = true; } if (keys !== undefined) { - for (let key of keys) { - if (typeof key === "string") { - const asNumber = Number(key); - if (Number.isNaN(asNumber)) { - continue; - } - key = asNumber as keyof T; - } - - if (typeof key !== "string") { - if (Number.isSafeInteger(key)) { - if (key >= 0 && key <= 64 * 1024) { - continue; - } - } - - throw new Error(`Keys must be strings or numbers: ${String(key)}`); - } - } + for (const key of keys) validateHelperKey(key); } this.value = value; this.columns = keys ?? []; + this.autoColumns = autoColumns; + } +} + +/** + * Return the union of own keys across every object in a multi-row array, + * seeded from the first row's key list so column order is stable. Used by the + * INSERT helper when no explicit column list was given, so a key that first + * appears on a later row is still emitted instead of being silently dropped. + */ +function unionRowKeys(firstRowKeys: (keyof T)[], items: T[]): (keyof T)[] { + const seen = new Set(firstRowKeys); + const union = firstRowKeys.slice(); + for (let i = 1; i < items.length; i++) { + const item = items[i]; + if (item == null || !$isObject(item)) continue; + const itemKeys = Object.keys(item); + for (let k = 0; k < itemKeys.length; k++) { + const key = itemKeys[k] as keyof T; + if (!seen.has(key)) { + validateHelperKey(key); + seen.add(key); + union.push(key); + } + } } + return union; } /** @@ -421,9 +455,11 @@ function normalizeQuery( binding_idx += sub_values.length; } else if (value instanceof SQLHelper) { const command = adapter.getHelperCommand(query); - const { columns, value: items } = value as SQLHelper; + const { columns, value: items, autoColumns } = value as SQLHelper; const columnCount = columns.length; - if (columnCount === 0 && command !== SQLCommand.in) { + // INSERT widens the column list below and has its own empty-column + // guard after widening, so an empty first row is allowed here. + if (columnCount === 0 && command !== SQLCommand.in && command !== SQLCommand.insert) { throw new SyntaxError(`Cannot ${commandToString(command)} with no columns`); } const lastColumnIndex = columns.length - 1; @@ -433,9 +469,18 @@ function normalizeQuery( // insert into users ${sql(users)} or insert into users ${sql(user)} // + // When the column list came from Object.keys(rows[0]) and there are + // further rows, widen it to the union of keys across every row so a + // key that first appears on a later row is still emitted. Explicit + // column lists and single-row inserts are left as supplied. + const insertColumns = + autoColumns && $isArray(items) && items.length > 1 && !$isArray(items[0]) + ? unionRowKeys(columns, items) + : columns; + // Build column list while determining which columns have at least one defined value const { definedColumns, columnsSql } = buildDefinedColumnsAndQuery( - columns, + insertColumns, items, adapter.escapeIdentifier.bind(adapter), ); diff --git a/test/js/sql/sql-helpers-validation.test.ts b/test/js/sql/sql-helpers-validation.test.ts index a8210853641c..f2567752b2d4 100644 --- a/test/js/sql/sql-helpers-validation.test.ts +++ b/test/js/sql/sql-helpers-validation.test.ts @@ -152,4 +152,104 @@ describe("sqlite helper behavior preserved", () => { // a null item without a column binds NULL expect(await sql`SELECT 1 as num WHERE 1 IN ${sql([null, 1])}`).toEqual([{ num: 1 }]); }); + + // The insert helper derives its column list from the supplied rows. When a + // key is entirely absent from the first row (not merely set to undefined) it + // must still be emitted as a column if any later row supplies it; otherwise + // that later row's value is silently discarded and NULL is stored. + test("batch insert helper unions keys across all rows", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE h (id INTEGER, a TEXT, b TEXT)`; + + const rows = [ + { id: 1, a: "onlyA" }, // no `b` key at all + { id: 2, a: "hasB", b: "IMPORTANT-DATA" }, + { id: 3, b: "b-only" }, // no `a` key at all + ]; + const result = await sql`INSERT INTO h ${sql(rows)} RETURNING id, a, b`; + expect(result).toEqual([ + { id: 1, a: "onlyA", b: null }, + { id: 2, a: "hasB", b: "IMPORTANT-DATA" }, + { id: 3, a: null, b: "b-only" }, + ]); + + // Boundary case of the same widening: an empty first row contributes no + // keys of its own, so the column list comes entirely from later rows. + const withEmptyFirst = await sql`INSERT INTO h ${sql([{}, { id: 4, a: "later" }])} RETURNING id, a, b`; + expect(withEmptyFirst).toEqual([ + { id: null, a: null, b: null }, + { id: 4, a: "later", b: null }, + ]); + + // When no row supplies any key, the INSERT path's own guard still fires. + const err = await sql`INSERT INTO h ${sql([{}, {}])}`.catch(e => e); + expect(err).toBeInstanceOf(SyntaxError); + expect(err.message).toBe("Insert needs to have at least one column with a defined value"); + }); + + test("batch insert helper with explicit columns ignores extra keys", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE h (id INTEGER, a TEXT)`; + // When the caller names the columns explicitly, keys outside that set are + // ignored regardless of which row they appear on. + const rows = [ + { id: 1, a: "x" }, + { id: 2, a: "y", b: "ignored" }, + ]; + const result = await sql`INSERT INTO h ${sql(rows, "id", "a")} RETURNING id, a`; + expect(result).toEqual([ + { id: 1, a: "x" }, + { id: 2, a: "y" }, + ]); + }); + + // Column-list widening is scoped to INSERT. A WHERE IN helper over objects + // with no explicit column keeps deriving its single column from the first + // row's keys so existing queries keep working. + test("WHERE IN helper with heterogeneous objects still uses first-row keys", async () => { + await using sql = new SQL("sqlite://:memory:"); + await sql`CREATE TABLE t (id INTEGER)`; + await sql`INSERT INTO t VALUES (1), (2), (3)`; + const rows = await sql`SELECT id FROM t WHERE id IN ${sql([{ id: 1 }, { id: 2, other: "x" }] as any)} ORDER BY id`; + expect(rows).toEqual([{ id: 1 }, { id: 2 }]); + }); +}); + +// sql.array() builds the Postgres array literal up front, so its serialization +// can be asserted without a server. A JS null must become the unquoted token +// `null` (SQL NULL in array-literal syntax); previously `typeof null === +// "object"` sent it through JSON.stringify and produced the quoted string +// `"null"`, which a TEXT[] column stores as the four-character string and an +// INTEGER[] column rejects with 22P02. +describe("postgres sql.array serialization (no server)", () => { + const makeSql = () => new SQL("postgres://bun_sql_test@127.0.0.1:1/bun_sql_test", { max: 1 }); + + test("null in a TEXT array serializes as SQL NULL", async () => { + await using sql = makeSql(); + expect(sql.array(["a", null, "b"], "TEXT").serializedValues).toBe('{"a",null,"b"}'); + }); + + test("null in a numeric array serializes as SQL NULL", async () => { + await using sql = makeSql(); + expect(sql.array([1, null, 2], "INTEGER").serializedValues).toBe("{1,null,2}"); + expect(sql.array([1.5, null], "DOUBLE PRECISION").serializedValues).toBe("{1.5,null}"); + }); + + test("null in a nested array serializes as SQL NULL", async () => { + await using sql = makeSql(); + expect( + sql.array( + [ + [1, null], + [null, 4], + ], + "INTEGER", + ).serializedValues, + ).toBe("{{1,null},{null,4}}"); + }); + + test("undefined still serializes as SQL NULL", async () => { + await using sql = makeSql(); + expect(sql.array(["a", undefined, "b"], "TEXT").serializedValues).toBe('{"a",null,"b"}'); + }); }); diff --git a/test/js/sql/sql.test.ts b/test/js/sql/sql.test.ts index 7818fecb5caf..88e6a0e870be 100644 --- a/test/js/sql/sql.test.ts +++ b/test/js/sql/sql.test.ts @@ -242,6 +242,23 @@ if (isDockerEnabled()) { expect(x).toEqual(["hello", "world", "test"]); }); + test("sql.array should encode null elements as SQL NULL", async () => { + await using sql = postgres(options); + // TEXT[]: null must round-trip as SQL NULL, not the four-character string "null" + { + const [{ x }] = await sql`select ${sql.array(["a", null, "b"], "TEXT")} as x`; + expect(x).toEqual(["a", null, "b"]); + } + // INTEGER[]: previously the quoted "null" was rejected by the server with 22P02 + { + const [{ second_is_null, arr }] = + await sql`select (${sql.array([1, null, 2], "INTEGER")})[2] is null as second_is_null, + ${sql.array([1, null, 2], "INTEGER")}::text as arr`; + expect(arr).toBe("{1,NULL,2}"); + expect(second_is_null).toBe(true); + } + }); + test("sql.array should support BOOLEAN arrays", async () => { await using sql = postgres(options); @@ -1058,6 +1075,26 @@ if (isDockerEnabled()) { } }); + test("bulk insert unions keys across heterogeneous rows", async () => { + const random_name = "test_" + randomUUIDv7("hex").replaceAll("-", ""); + await sql`create table ${sql(random_name)} (id int, a text, b text)`; + try { + const rows = [ + { id: 1, a: "onlyA" }, // no `b` key + { id: 2, a: "hasB", b: "IMPORTANT-DATA" }, + { id: 3, b: "b-only" }, // no `a` key + ]; + const result = await sql`insert into ${sql(random_name)} ${sql(rows)} returning id, a, b`; + expect(result).toEqual([ + { id: 1, a: "onlyA", b: null }, + { id: 2, a: "hasB", b: "IMPORTANT-DATA" }, + { id: 3, a: null, b: "b-only" }, + ]); + } finally { + await sql`drop table ${sql(random_name)}`; + } + }); + // t("Empty array", async () => [true, Array.isArray((await sql`select ${sql.array([], 1009)} as x`)[0].x)]); test("string arg with ::int -> Array", async () =>