diff --git a/scripts/client-compat/clients/pgx/main.go b/scripts/client-compat/clients/pgx/main.go index 64c30f7b..fd935487 100644 --- a/scripts/client-compat/clients/pgx/main.go +++ b/scripts/client-compat/clients/pgx/main.go @@ -386,6 +386,41 @@ func testBatch(ctx context.Context, r *reporter) { } } +// testBinaryNumeric forces binary result format for a UBIGINT column, which +// duckgres advertises as NUMERIC. psql and other text-format clients never hit +// this path, so a UBIGINT that encoded as text bytes inside a binary numeric +// field broke every binary-format client (pgAdmin, DuckDB's Postgres +// extension, JDBC) with "Postgres numeric NA/Inf". This selects the maximum +// UBIGINT (2^64 - 1) in binary and checks the round-trip. +func testBinaryNumeric(ctx context.Context, r *reporter) { + fmt.Println("\n=== Binary numeric encoding ===") + suite := "binary_numeric" + + conn, err := connect(ctx) + if err != nil { + r.report(suite, "connect", "fail", err.Error()) + return + } + defer conn.Close(ctx) + + const want = "18446744073709551615" // 2^64 - 1 + var got string + err = conn.QueryRow( + ctx, + "SELECT 18446744073709551615::UBIGINT AS v", + pgx.QueryResultFormats{pgx.BinaryFormatCode}, + ).Scan(&got) + if err != nil { + r.report(suite, "ubigint_binary_format", "fail", err.Error()) + return + } + if got == want { + r.report(suite, "ubigint_binary_format", "pass", got) + } else { + r.report(suite, "ubigint_binary_format", "fail", fmt.Sprintf("expected %s, got %s", want, got)) + } +} + func main() { ctx := context.Background() @@ -403,6 +438,7 @@ func main() { testSharedQueries(ctx, r) testDDLDML(ctx, r) testBatch(ctx, r) + testBinaryNumeric(ctx, r) fmt.Printf("\n%s\n", "==================================================") fmt.Printf("Results: %d passed, %d failed\n", r.passed, r.failed) diff --git a/server/conn_results.go b/server/conn_results.go index 0e8e7403..5c277dab 100644 --- a/server/conn_results.go +++ b/server/conn_results.go @@ -281,14 +281,14 @@ func (c *clientConn) sendDataRowWithFormats(values []interface{}, formatCodes [] // Binary encoding encoded := encodeBinary(v, typeOIDs[i]) if encoded == nil { - // Fallback to text if binary encoding fails - str := formatValue(v) - _ = binary.Write(&buf, binary.BigEndian, int32(len(str))) - buf.WriteString(str) - } else { - _ = binary.Write(&buf, binary.BigEndian, int32(len(encoded))) - buf.Write(encoded) + // The client requested binary results for this column but we + // have no binary encoding for the value. Writing text bytes + // under a binary format code would corrupt the field, so fail + // the row instead of shipping garbage. + return fmt.Errorf("cannot binary-encode column %d (OID %d, Go type %T)", i, typeOIDs[i], v) } + _ = binary.Write(&buf, binary.BigEndian, int32(len(encoded))) + buf.Write(encoded) } else { // Text encoding must use the column OID for types whose PostgreSQL // representation cannot be inferred from the scanned Go value alone. diff --git a/server/conn_results_test.go b/server/conn_results_test.go index ac6dc2bd..9c5af5ab 100644 --- a/server/conn_results_test.go +++ b/server/conn_results_test.go @@ -38,6 +38,26 @@ func TestSendErrorDoesNotLogErrorContents(t *testing.T) { } } +func TestSendDataRowWithFormatsBinaryEncodeFailureReturnsError(t *testing.T) { + // When the client requests binary results but a column value cannot be + // binary-encoded, the row must fail with an error rather than write text + // bytes under a binary format code, which would corrupt the field. + var out bytes.Buffer + conn := &clientConn{writer: bufio.NewWriter(&out)} + + err := conn.sendDataRowWithFormats( + []any{true}, // bool has no binary NUMERIC encoding + []int16{1}, // binary format + []int32{OidNumeric}, + ) + if err == nil { + t.Fatal("sendDataRowWithFormats: expected error for unencodable binary column, got nil") + } + if out.Len() != 0 { + t.Errorf("sendDataRowWithFormats wrote %d bytes on failure, want 0", out.Len()) + } +} + func TestSendDataRowWithFormatsUsesTypeOIDForTextDates(t *testing.T) { date := time.Date(2022, 4, 1, 0, 0, 0, 0, time.UTC) timestampTZ := time.Date(2022, 3, 31, 20, 0, 0, 123456000, time.FixedZone("UTC-4", -4*60*60)) diff --git a/server/types.go b/server/types.go index b54cdfc9..1318fbfe 100644 --- a/server/types.go +++ b/server/types.go @@ -6,6 +6,7 @@ import ( "encoding/hex" "encoding/json" "fmt" + "log/slog" "math" "math/big" "strings" @@ -691,6 +692,38 @@ func encodeNumeric(v interface{}) []byte { // HUGEINT comes from the Go driver as *big.Int (scale 0) val = new(big.Int).Set(x) dscale = 0 + case uint64: + // UBIGINT can arrive as a native Go uint64 (scale 0). uint64 exceeds + // int64, so build the big.Int from the unsigned value directly. + val = new(big.Int).SetUint64(x) + dscale = 0 + case uint: + val = new(big.Int).SetUint64(uint64(x)) + dscale = 0 + case uint8: + val = big.NewInt(int64(x)) + dscale = 0 + case uint16: + val = big.NewInt(int64(x)) + dscale = 0 + case uint32: + val = big.NewInt(int64(x)) + dscale = 0 + case int: + val = big.NewInt(int64(x)) + dscale = 0 + case int8: + val = big.NewInt(int64(x)) + dscale = 0 + case int16: + val = big.NewInt(int64(x)) + dscale = 0 + case int32: + val = big.NewInt(int64(x)) + dscale = 0 + case int64: + val = big.NewInt(x) + dscale = 0 case string: // Arrow Flight returns decimals with non-zero scale as strings like "123.45". // Parse the string back into unscaled big.Int + scale. @@ -712,8 +745,12 @@ func encodeNumeric(v interface{}) []byte { val.Neg(val) } default: - // Fallback: try to format as text and let the caller handle it - return encodeText(v) + // A NUMERIC column reached the wire with a Go type we cannot encode as + // binary numeric. Emitting text bytes here would corrupt the field for + // any client that requested binary results, so refuse loudly and let + // the caller surface a clean error instead. + slog.Error("encodeNumeric: unsupported Go type for binary numeric encoding", "go_type", fmt.Sprintf("%T", v)) + return nil } // Handle sign diff --git a/server/types_test.go b/server/types_test.go index 693821d5..f14e74d2 100644 --- a/server/types_test.go +++ b/server/types_test.go @@ -1103,6 +1103,55 @@ func TestEncodeDecodeUBIGINTMax(t *testing.T) { } } +func TestEncodeNumericFromGoIntegers(t *testing.T) { + // A worker can return a UBIGINT column as a native Go uint64 rather than a + // duckdb.Decimal. The catalog advertises UBIGINT and HUGEINT columns as + // NUMERIC, so encodeNumeric must turn every Go integer kind into a valid + // binary numeric. Before the fix these values fell through to text bytes + // inside a binary field, which clients read as "Postgres numeric NA/Inf". + tests := []struct { + name string + input any + want string + }{ + {"uint64 max", uint64(18446744073709551615), "18446744073709551615"}, + {"uint64 mid", uint64(1234567890123), "1234567890123"}, + {"uint64 zero", uint64(0), "0"}, + {"uint32", uint32(4294967295), "4294967295"}, + {"uint16", uint16(65535), "65535"}, + {"uint8", uint8(255), "255"}, + {"uint", uint(42), "42"}, + {"int64 positive", int64(9223372036854775807), "9223372036854775807"}, + {"int64 negative", int64(-9223372036854775808), "-9223372036854775808"}, + {"int", int(-7), "-7"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + encoded := encodeNumeric(tt.input) + if encoded == nil { + t.Fatalf("encodeNumeric(%v) returned nil", tt.input) + } + decoded, err := decodeNumeric(encoded) + if err != nil { + t.Fatalf("decodeNumeric failed: %v", err) + } + if decoded != tt.want { + t.Errorf("roundtrip %s = %q, want %q", tt.name, decoded, tt.want) + } + }) + } +} + +func TestEncodeNumericUnsupportedTypeReturnsNil(t *testing.T) { + // An unencodable value on a NUMERIC column must not emit text bytes into a + // binary field. encodeNumeric returns nil so the caller can fail the row + // with a clean error instead of shipping a corrupt numeric. + if got := encodeNumeric(true); got != nil { + t.Errorf("encodeNumeric(bool) = %v, want nil", got) + } +} + func TestDecodeNumericRaw(t *testing.T) { // Test decoding a manually constructed binary numeric: 99.99 // ndigits=2, weight=0, sign=0x0000, dscale=2, digits=[99, 9900]