From b6abbd50a0b1292bc41041f6e8217be644b8d920 Mon Sep 17 00:00:00 2001 From: Markus Schmidt Date: Mon, 12 Jan 2026 19:56:02 +0000 Subject: [PATCH] fix(Bun.SQL): handle binary columns in MySQL correctly (#26011) ## What does this PR do? Currently binary columns are returned as strings which means they get corrupted when encoded in UTF8. This PR returns binary columns as Buffers which is what user's actually expect and is also consistent with PostgreSQL and SQLite. ### How did you verify your code works? I added tests to verify the correct behavior. Before there were no tests for binary columns at all. This fixes #23991 --- src/sql/mysql/protocol/DecodeBinaryValue.zig | 8 +++++--- src/sql/mysql/protocol/ResultSet.zig | 10 +++++++--- test/js/sql/sql-mysql.test.ts | 19 +++++++++++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/src/sql/mysql/protocol/DecodeBinaryValue.zig b/src/sql/mysql/protocol/DecodeBinaryValue.zig index 5406d48060..b488efedfb 100644 --- a/src/sql/mysql/protocol/DecodeBinaryValue.zig +++ b/src/sql/mysql/protocol/DecodeBinaryValue.zig @@ -1,4 +1,4 @@ -pub fn decodeBinaryValue(globalObject: *jsc.JSGlobalObject, field_type: types.FieldType, column_length: u32, raw: bool, bigint: bool, unsigned: bool, comptime Context: type, reader: NewReader(Context)) !SQLDataCell { +pub fn decodeBinaryValue(globalObject: *jsc.JSGlobalObject, field_type: types.FieldType, column_length: u32, raw: bool, bigint: bool, unsigned: bool, binary: bool, comptime Context: type, reader: NewReader(Context)) !SQLDataCell { debug("decodeBinaryValue: {s}", .{@tagName(field_type)}); return switch (field_type) { .MYSQL_TYPE_TINY => { @@ -131,6 +131,7 @@ pub fn decodeBinaryValue(globalObject: *jsc.JSGlobalObject, field_type: types.Fi else => error.InvalidBinaryValue, }, + // When the column contains a binary string we return a Buffer otherwise a string .MYSQL_TYPE_ENUM, .MYSQL_TYPE_SET, .MYSQL_TYPE_GEOMETRY, @@ -138,7 +139,6 @@ pub fn decodeBinaryValue(globalObject: *jsc.JSGlobalObject, field_type: types.Fi .MYSQL_TYPE_STRING, .MYSQL_TYPE_VARCHAR, .MYSQL_TYPE_VAR_STRING, - // We could return Buffer here BUT TEXT, LONGTEXT, MEDIUMTEXT, TINYTEXT, etc. are BLOB and the user expects a string .MYSQL_TYPE_TINY_BLOB, .MYSQL_TYPE_MEDIUM_BLOB, .MYSQL_TYPE_LONG_BLOB, @@ -151,7 +151,9 @@ pub fn decodeBinaryValue(globalObject: *jsc.JSGlobalObject, field_type: types.Fi } var string_data = try reader.encodeLenString(); defer string_data.deinit(); - + if (binary) { + return SQLDataCell.raw(&string_data); + } const slice = string_data.slice(); return SQLDataCell{ .tag = .string, .value = .{ .string = if (slice.len > 0) bun.String.cloneUTF8(slice).value.WTFStringImpl else null }, .free_value = 1 }; }, diff --git a/src/sql/mysql/protocol/ResultSet.zig b/src/sql/mysql/protocol/ResultSet.zig index d13a71ac8f..9e72d9d734 100644 --- a/src/sql/mysql/protocol/ResultSet.zig +++ b/src/sql/mysql/protocol/ResultSet.zig @@ -140,8 +140,12 @@ pub const Row = struct { } }, else => { - const slice = value.slice(); - cell.* = SQLDataCell{ .tag = .string, .value = .{ .string = if (slice.len > 0) bun.String.cloneUTF8(slice).value.WTFStringImpl else null }, .free_value = 1 }; + if (column.flags.BINARY) { + cell.* = SQLDataCell.raw(value); + } else { + const slice = value.slice(); + cell.* = SQLDataCell{ .tag = .string, .value = .{ .string = if (slice.len > 0) bun.String.cloneUTF8(slice).value.WTFStringImpl else null }, .free_value = 1 }; + } }, }; } @@ -226,7 +230,7 @@ pub const Row = struct { } const column = this.columns[i]; - value.* = try decodeBinaryValue(this.globalObject, column.column_type, column.column_length, this.raw, this.bigint, column.flags.UNSIGNED, Context, reader); + value.* = try decodeBinaryValue(this.globalObject, column.column_type, column.column_length, this.raw, this.bigint, column.flags.UNSIGNED, column.flags.BINARY, Context, reader); value.index = switch (column.name_or_index) { // The indexed columns can be out of order. .index => |idx| idx, diff --git a/test/js/sql/sql-mysql.test.ts b/test/js/sql/sql-mysql.test.ts index 6179339ae2..0228dcf82d 100644 --- a/test/js/sql/sql-mysql.test.ts +++ b/test/js/sql/sql-mysql.test.ts @@ -480,6 +480,25 @@ if (isDockerEnabled()) { expect(b).toEqual({ b: 2 }); }); + test("Binary", async () => { + const random_name = ("t_" + Bun.randomUUIDv7("hex").replaceAll("-", "")).toLowerCase(); + await sql`CREATE TEMPORARY TABLE ${sql(random_name)} (a binary(1), b varbinary(1), c blob)`; + const values = [ + { a: Buffer.from([1]), b: Buffer.from([2]), c: Buffer.from([3]) }, + ]; + await sql`INSERT INTO ${sql(random_name)} ${sql(values)}`; + const results = await sql`select * from ${sql(random_name)}`; + // return buffers + expect(results[0].a).toEqual(Buffer.from([1])); + expect(results[0].b).toEqual(Buffer.from([2])); + expect(results[0].c).toEqual(Buffer.from([3])); + // text protocol should behave the same + const results2 = await sql`select * from ${sql(random_name)}`.simple(); + expect(results2[0].a).toEqual(Buffer.from([1])); + expect(results2[0].b).toEqual(Buffer.from([2])); + expect(results2[0].c).toEqual(Buffer.from([3])); + }) + test("bulk insert nested sql()", async () => { await using sql = new SQL({ ...getOptions(), max: 1 }); await sql`create temporary table test_users (name text, age int)`;