Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 21 additions & 25 deletions src/sql_jsc/postgres/DataCell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1312,10 +1312,8 @@
let _ = decimal_pos; // matches Zig: computed but unused below
// Output all digits before the decimal point

let mut scale_start: i32 = 0;
if weight < 0 {
result.push(b'0');
scale_start = weight as i32 + 1;
} else {
let mut idx: usize = 0;
let mut first_non_zero = false;
Expand Down Expand Up @@ -1350,33 +1348,31 @@
}
}
// If requested, output a decimal point and all the digits that follow it.
// We initially put out a multiple of 4 digits, then truncate if needed.
// We initially put out a multiple of DEC_DIGITS (4) digits, then truncate.
//
// This mirrors Postgres' get_str_from_var: two independent counters —
// `d` walks base-10000 digits (advances by 1), `i` counts decimal places
// emitted (advances by DEC_DIGITS). Conflating them drops leading-zero
// groups when weight <= -3 and shifts significant digits left.
if dscale > 0 {
result.push(b'.');
// negative scale means we need to add zeros before the decimal point
// greater than ndigits means we need to add zeros after the decimal point
let mut idx: isize = scale_start as isize;
let end: usize = result.len() + usize::try_from(dscale).expect("int cast");
while idx < dscale as isize {
if idx >= 0 && idx < dscale as isize {
let digit: u16 = if cursor.len() >= 2 {
let v = u16::from_be_bytes(
cursor[..2].try_into().expect("infallible: size matches"),
);
cursor = &cursor[2..];
v
} else {
0
};
bun_core::scoped_log!(PostgresDataCell, "dscale digit: {}", digit);
let digit_str: [u8; 4] = bun_core::fmt::itoa_padded::<4>(u64::from(digit));
let digit_len = 4usize;
result.extend_from_slice(&digit_str[0..digit_len]);
let mut d: i32 = weight as i32 + 1;

Check notice on line 1360 in src/sql_jsc/postgres/DataCell.rs

View check run for this annotation

Claude / Claude Code Review

ndigits==0 early return drops dscale (pre-existing get_str_from_var divergence)

Pre-existing nit, not introduced here: the `if ndigits == 0 { return "0" }` early return a few lines above this hunk ignores `dscale`, so a binary-encoded zero with display scale (e.g. `0::numeric(10,2)`, sent as `{ndigits=0, weight=0, sign=0, dscale=2}`) decodes as `"0"` while text protocol and Postgres' `get_str_from_var` both yield `"0.00"`. It's the one remaining `get_str_from_var` divergence in this function and the new test's `"0"` case uses `dscale=0` so doesn't cover it; note that simply
Comment thread
robobun marked this conversation as resolved.
let mut i: i32 = 0;
while i < dscale as i32 {
let digit: u16 = if d >= 0 && d < ndigits as i32 && cursor.len() >= 2 {
let v =
u16::from_be_bytes(cursor[..2].try_into().expect("infallible: size matches"));
cursor = &cursor[2..];
v
} else {
bun_core::scoped_log!(PostgresDataCell, "dscale digit: 0000");
result.extend_from_slice(b"0000");
}
idx += 4;
0
};
bun_core::scoped_log!(PostgresDataCell, "dscale digit: {}", digit);
let digit_str: [u8; 4] = bun_core::fmt::itoa_padded::<4>(u64::from(digit));
result.extend_from_slice(&digit_str);
d += 1;
i += 4;
}
if result.len() > end {
result.truncate(end);
Expand Down
121 changes: 121 additions & 0 deletions test/js/sql/postgres-binary-numeric.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
// Postgres' binary NUMERIC wire format is {ndigits, weight, sign, dscale,
// digits[]} where each digit is a base-10000 group. get_str_from_var in
// numeric.c prints the fractional part with two independent counters: a
// base-10000 digit index `d` (++ per group) and a decimal-position counter `i`
// (+= DEC_DIGITS per group). A decoder that collapses both into one index
// walks the leading-zero region 4x too fast and, for weight <= -3, drops
// leading "0000" groups — returning e.g. "0.000010000" for 1e-9.
//
// Uses a minimal mock Postgres server so the test runs without Docker. The
// server replies to the simple 'Q' protocol but marks the result column as
// binary (format=1) so Bun's binary NUMERIC decoder is exercised.

import { SQL } from "bun";
import { expect, test } from "bun:test";
import net from "net";

function pkt(type: string, body: Buffer): Buffer {
const header = Buffer.alloc(5);
header.write(type, 0);
header.writeInt32BE(body.length + 4, 1);
return Buffer.concat([header, body]);
}
function i16(n: number): Buffer {
const b = Buffer.alloc(2);
b.writeInt16BE(n, 0);
return b;
}
function u16(n: number): Buffer {
const b = Buffer.alloc(2);
b.writeUInt16BE(n, 0);
return b;
}
function i32(n: number): Buffer {
const b = Buffer.alloc(4);
b.writeInt32BE(n, 0);
return b;
}
function cstr(s: string): Buffer {
return Buffer.concat([Buffer.from(s), Buffer.from([0])]);
}

const NUMERIC_OID = 1700;

function rowDescription(name: string): Buffer {
return pkt(
"T",
Buffer.concat([
i16(1), // 1 column
cstr(name),
i32(0), // table oid
i16(0), // column attr number
i32(NUMERIC_OID),
i16(-1), // type size
i32(-1), // type modifier
i16(1), // format: 1 = binary
]),
);
}

function dataRow(col: Buffer): Buffer {
return pkt("D", Buffer.concat([i16(1), i32(col.length), col]));
}

// Encode a Postgres binary NUMERIC field.
function numeric(ndigits: number, weight: number, sign: number, dscale: number, digits: number[]): Buffer {
return Buffer.concat([i16(ndigits), i16(weight), u16(sign), i16(dscale), ...digits.map(u16)]);
}

const authenticationOk = pkt("R", i32(0));
const readyForQuery = pkt("Z", Buffer.from("I"));
const commandComplete = pkt("C", cstr("SELECT 1"));

async function decodeNumeric(bytes: Buffer): Promise<unknown> {
const server = net.createServer(socket => {
let startup = true;
socket.on("data", data => {
if (startup) {
startup = false;
socket.write(Buffer.concat([authenticationOk, readyForQuery]));
return;
}
if (data[0] !== 0x51 /* 'Q' */) return;
socket.write(Buffer.concat([rowDescription("n"), dataRow(bytes), commandComplete, readyForQuery]));
});
socket.on("error", () => {});
});
await new Promise<void>(r => server.listen(0, "127.0.0.1", () => r()));
const port = (server.address() as net.AddressInfo).port;
const sql = new SQL({ url: `postgres://u@127.0.0.1:${port}/db`, max: 1, idleTimeout: 5, connectionTimeout: 5 });
try {
const [row]: any = await sql`select n`.simple();
return row.n;
} finally {
await sql.close().catch(() => {});
await new Promise<void>(r => server.close(() => r()));
}
}

// Wire-format encodings for each test value. weight/dscale/digits match what a
// real Postgres server sends (verified against psql).
const cases: { literal: string; bytes: Buffer }[] = [
// --- weight <= -3: previously corrupted --------------------------------
{ literal: "0.000000001", bytes: numeric(1, -3, 0x0000, 9, [1000]) },
{ literal: "0.000000000001", bytes: numeric(1, -3, 0x0000, 12, [1]) },
{ literal: "0.00000000123", bytes: numeric(1, -3, 0x0000, 11, [1230]) },
{ literal: "0.0000000000001", bytes: numeric(1, -4, 0x0000, 13, [1000]) },
{ literal: "-0.000000001", bytes: numeric(1, -3, 0x4000, 9, [1000]) },
{ literal: "0.00000000000000012345", bytes: numeric(2, -4, 0x0000, 20, [1, 2345]) },
// --- boundary & previously-correct paths: must remain unchanged --------
{ literal: "0.00000001", bytes: numeric(1, -2, 0x0000, 8, [1]) },
{ literal: "0.0001", bytes: numeric(1, -1, 0x0000, 4, [1]) },
{ literal: "123.456", bytes: numeric(2, 0, 0x0000, 3, [123, 4560]) },
{ literal: "1000000", bytes: numeric(1, 1, 0x0000, 0, [100]) },
{ literal: "0", bytes: numeric(0, 0, 0x0000, 0, []) },
{ literal: "0.123456789012345", bytes: numeric(4, -1, 0x0000, 15, [1234, 5678, 9012, 3450]) },
{ literal: "12345678.000000009", bytes: numeric(5, 1, 0x0000, 9, [1234, 5678, 0, 0, 9000]) },
];

test.each(cases)("binary NUMERIC decodes $literal", async ({ literal, bytes }) => {
expect(await decodeNumeric(bytes)).toBe(literal);
});
Loading