Bun.SQL: statement and columns field for SQLResultArray

[alexandre-lavoie]

What does this PR do?

Improves: #18866

Exports a columns field on SQLResultArray providing additional context on columns returned by the query. This is mostly just binding PostgreSQL's RowDescription that was already previously implemented.

This is meant to behave similarly to postgres.js columns. The main difference is our type uses the internal OIDs while postgresql.js uses JDBC's OIDs. This would make ours more accurate but could impact DX that are used to the "less accurate" values.

Here is the result of a test query:

const sql = Bun.SQL(connectionString);

// Using safe
console.log((await sql`select 1 as x, 2 as longer_name, 3 as sp3c141_n4m3`));

// Using unsafe
console.log((await sql.unsafe("select 1 as x, 2 as longer_name, 3 as sp3c141_n4m3")));

[
  {
    x: 1,
    longer_name: 2,
    sp3c141_n4m3: 3,
  }, count: 1, command: "SELECT", columns: [
    {
      name: "x",
      table: 0,
      number: 0,
      type: 23,
    }, {
      name: "longer_name",
      table: 0,
      number: 0,
      type: 23,
    }, {
      name: "sp3c141_n4m3",
      table: 0,
      number: 0,
      type: 23,
    }
  ]
]

• [?] Documentation or TypeScript types (might need some extra work?)
• Code changes

How did you verify your code works?

• I wrote TypeScript tests and they pass locally (bun-debug test test/js/sql/sql.test.ts)
• I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
• JSValue used outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed

[Jarred-Sumner]

Thank you for this PR

Instead of creating the columns array dynamically before each time we run the callback, could we make it a lazy getter on the handle and avoid the work of creating the array when it usually never will be used?

To do that, you could add another item to this array in the class generator:

bun/src/bun.js/api/postgres.classes.ts

Line 80 in 727b085

values: ["pendingValue", "target", "columns", "binding"],

Then, a getter function declaration in the list where the other functions are in that file

Then, a getter implementation in postgres.zig, probably somewhat similar to getQueries

bun/src/sql/postgres.zig

Lines 1389 to 1398 in 727b085

	pub fn getQueries(_: *PostgresSQLConnection, thisValue: JSC.JSValue, globalObject: *JSC.JSGlobalObject) JSC.JSValue {
	if (PostgresSQLConnection.queriesGetCached(thisValue)) |value| {
	return value;
	}
	const array = JSC.JSValue.createEmptyArray(globalObject, 0);
	PostgresSQLConnection.queriesSetCached(thisValue, globalObject, array);
	return array;
	}

[alexandre-lavoie]

Great suggestion!

I refactored to the lazier approach. Given columns was already in use, decided I might as well implement the statement object to be closer to postgres.js's implementation.

I also updated the types so they should be abit cleaner on output. It could potentially be less strict with the optional fields but I'm worried something weird might happen in the future due to INSERT and UPDATE.

Also I noticed that there is a formatting issue. I'm not sure why but locally my VSCode's zig-format does not match the repo's. It should hopefully be fixed now.

Happy to make any other changes if you have any other suggestions!

Here is my latest test/result to cover all the changes:

const sql = new Bun.SQL(connectionString);

const res = await sql`select 1 as x, 2 as longer_name, 3 as sp3c141_n4m3`;

console.log("Query:", res.query);
console.log("Statement:", res.statement);
console.log("Columns:", res.columns);
console.log("Value[0]:", res[0]);

Query: PostgresQuery {   executed  }
Statement: {
  string: "select 1 as x, 2 as longer_name, 3 as sp3c141_n4m3",
  columns: [
    {
      name: "x",
      table: 0,
      number: 0,
      type: 23,
    }, {
      name: "longer_name",
      table: 0,
      number: 0,
      type: 23,
    }, {
      name: "sp3c141_n4m3",
      table: 0,
      number: 0,
      type: 23,
    }
  ],
}
Columns: [
  {
    name: "x",
    table: 0,
    number: 0,
    type: 23,
  }, {
    name: "longer_name",
    table: 0,
    number: 0,
    type: 23,
  }, {
    name: "sp3c141_n4m3",
    table: 0,
    number: 0,
    type: 23,
  }
]
Value[0]: {
  x: 1,
  longer_name: 2,
  sp3c141_n4m3: 3,
}

[Jarred-Sumner]

Suggested change

	obj.put(globalObject, JSC.ZigString.static("string"), bun.String.init(this.query).toJS(globalObject));
	obj.put(globalObject, JSC.ZigString.static("string"), bun.String.createUTF8ForJS(globalObject, this.query));

[alexandre-lavoie]

Perfect, was not aware of this one!

Maybe we should add to the String.init's documentation: "If you are creating a string for JS, you probably want to use createUTF8ForJS"?

I saw this pattern elsewhere in code. I assume this was added on at a later point.

[alexandre-lavoie]

Actually in this case, I misread and this.query is already a bun.String so I assume this.query.toJS(globalObject) would be more optimal.