Skip to content

Panic on overflow in Decimal128/256 and BigInt append (+ silent truncation for Decimal32/64) #1849

Description

@fabricio-neri

Observed

  1. Inserting a value whose scaled big.Int exceeds 128 bits into a Decimal(P, S) column with P >= 19 (stored as Decimal128 or Decimal256) panics the caller's goroutine with panic: math/big: buffer too small to fit value. The panic originates in Go stdlib big.Int.FillBytes called from the unexported bigIntToRaw helper in lib/column/bigint.go. The panic cannot be recovered by the caller in typical database/sql batch flows, it unwinds the goroutine hosting the writer and crashes the process. Kafka/stream consumers that haven't committed an offset enter a crash loop replaying the poison record.
  2. The same panic occurs for Int128 / UInt128 / Int256 / UInt256 columns when the input *big.Int does not fit the column's byte width.
  3. For Decimal(P, S) with P <= 18 (stored as Decimal32 or Decimal64), the same class of overflow silently truncates instead of panicking. lib/column/decimal.go:253-260 does uint32(...IntPart()) / uint64(...IntPart()); IntPart() returns int64 which wraps on overflow, and the subsequent cast keeps only the low bits. The server accepts the wrapped value and the row is persisted with corrupted data, with no error returned to the caller.

Both failure modes violate the project's own contributor guidance in .claude/CLAUDE.md: "Do not panic in library code" and "Make the API easy to use correctly and hard to use incorrectly."

Expected behaviour

batch.Append / AppendRow should return a non-nil error that identifies the column type, the declared precision/scale (or bit width), and the offending value — e.g.

clickhouse [Decimal(38, 8)]: value 10000000000000000000000000000000 out of range

No panic. No silent truncation. Same behavior across all four Decimal widths and all four BigInt widths.

Code example

All three snippets follow the convention used in tests/issues/. Each asserts that batch.Append returns an error rather than panicking or silently succeeding.

1. Panic on Decimal128 overflow

Decimal(38, 8) is stored as Decimal128 (16 bytes). Max representable absolute value when scaled by 10^8 is 2^127 - 1 ≈ 1.7 × 10^38. 10^31 scaled becomes 10^39, which needs more than 128 bits.

package issues

import (
    "context"
    "fmt"
    "testing"

    "github.com/ClickHouse/clickhouse-go/v2"
    clickhouse_tests "github.com/ClickHouse/clickhouse-go/v2/tests"
    "github.com/shopspring/decimal"
    "github.com/stretchr/testify/require"
)

func TestIssueXXXX_Decimal128Overflow(t *testing.T) {
    conn, err := clickhouse_tests.GetConnection("issues", clickhouse.Settings{}, nil, nil)
    require.NoError(t, err)
    env, err := clickhouse_tests.GetTestEnvironment(testSet)
    require.NoError(t, err)
    ctx := context.Background()

    ddl := fmt.Sprintf(
        "CREATE TABLE `%s`.`issue_xxxx_dec128` (Col1 Decimal(38, 8)) Engine Memory",
        env.Database,
    )
    require.NoError(t, conn.Exec(ctx, ddl))
    defer conn.Exec(ctx, fmt.Sprintf("DROP TABLE `%s`.`issue_xxxx_dec128`", env.Database))

    batch, err := conn.PrepareBatch(ctx,
        fmt.Sprintf("INSERT INTO `%s`.`issue_xxxx_dec128`", env.Database))
    require.NoError(t, err)

    huge, _ := decimal.NewFromString("10000000000000000000000000000000") // 10^31
    require.NotPanics(t, func() {
        err := batch.Append(huge)
        require.Error(t, err, "expected an out-of-range error, not a panic")
    })
}

2. Silent truncation on Decimal32

Decimal(9, 0) stores as Decimal32 (int32). Max legal value per the column declaration is 10^9 - 1. Inserting 10^10 should error; today it silently stores 10^10 mod 2^32 = 1410065408.

func TestIssueXXXX_Decimal32SilentTruncation(t *testing.T) {
    conn, err := clickhouse_tests.GetConnection("issues", clickhouse.Settings{}, nil, nil)
    require.NoError(t, err)
    env, err := clickhouse_tests.GetTestEnvironment(testSet)
    require.NoError(t, err)
    ctx := context.Background()

    ddl := fmt.Sprintf(
        "CREATE TABLE `%s`.`issue_xxxx_dec32` (Col1 Decimal(9, 0)) Engine Memory",
        env.Database,
    )
    require.NoError(t, conn.Exec(ctx, ddl))
    defer conn.Exec(ctx, fmt.Sprintf("DROP TABLE `%s`.`issue_xxxx_dec32`", env.Database))

    batch, err := conn.PrepareBatch(ctx,
        fmt.Sprintf("INSERT INTO `%s`.`issue_xxxx_dec32`", env.Database))
    require.NoError(t, err)

    over := decimal.NewFromInt(10_000_000_000) // 10^10, exceeds Decimal(9, 0)
    err = batch.Append(over)
    require.Error(t, err, "expected an out-of-range error; got silent truncation")
}

3. Panic on Int128 overflow (BigInt side, same class)

func TestIssueXXXX_Int128Overflow(t *testing.T) {
    conn, err := clickhouse_tests.GetConnection("issues", clickhouse.Settings{}, nil, nil)
    require.NoError(t, err)
    env, err := clickhouse_tests.GetTestEnvironment(testSet)
    require.NoError(t, err)
    ctx := context.Background()

    ddl := fmt.Sprintf(
        "CREATE TABLE `%s`.`issue_xxxx_int128` (Col1 Int128) Engine Memory",
        env.Database,
    )
    require.NoError(t, conn.Exec(ctx, ddl))
    defer conn.Exec(ctx, fmt.Sprintf("DROP TABLE `%s`.`issue_xxxx_int128`", env.Database))

    batch, err := conn.PrepareBatch(ctx,
        fmt.Sprintf("INSERT INTO `%s`.`issue_xxxx_int128`", env.Database))
    require.NoError(t, err)

    over := new(big.Int).Lsh(big.NewInt(1), 128) // 2^128
    require.NotPanics(t, func() {
        err := batch.Append(over)
        require.Error(t, err, "expected an out-of-range error, not a panic")
    })
}

Error log

Stack trace from a production incident that inserted an out-of-range value into a Decimal(38, 8) column:

panic: math/big: buffer too small to fit value

goroutine 2474 [running]:
math/big.nat.bytes(...)
    /opt/hostedtoolcache/go/1.26.2/x64/src/math/big/nat.go:996
math/big.(*Int).FillBytes(...)
    /opt/hostedtoolcache/go/1.26.2/x64/src/math/big/int.go:542
github.com/ClickHouse/clickhouse-go/v2/lib/column.bigIntToRaw({0x37fff7cad2b8, 0x10, 0x400000000?}, 0x38005193d180?)
    /tmp/go-mod-cache/github.com/!click!house/clickhouse-go/v2@v2.35.0/lib/column/bigint.go:241
github.com/ClickHouse/clickhouse-go/v2/lib/column.(*Decimal).append(0x37ffff759280, 0x37fff7cad608)
    /tmp/go-mod-cache/github.com/!click!house/clickhouse-go/v2@v2.35.0/lib/column/decimal.go:282
github.com/ClickHouse/clickhouse-go/v2/lib/column.(*Decimal).AppendRow(...)
    .../v2@v2.35.0/lib/column/decimal.go:264
github.com/ClickHouse/clickhouse-go/v2/lib/proto.(*Block).Append(...)
    .../v2@v2.35.0/lib/proto/block.go:62
github.com/ClickHouse/clickhouse-go/v2.(*batch).Append(...)
    .../v2@v2.35.0/conn_batch.go:131

Details

Environment

  • clickhouse-go version: v2.35.0 (also reproduces on main at the time of writing)
  • Interface: database/sql compatible driver, via Batch.Append
  • Go version: 1.26.2 (bug reproduces on any Go 1.x since big.Int.FillBytes panics on oversize buffers by contract)
  • Operating system: Linux amd64 (platform-independent)
  • ClickHouse version: any (the server never sees the bad write on the panic path)
  • Is it a ClickHouse Cloud? Yes (reproduces locally against OSS ClickHouse as well)
  • ClickHouse Server non-default settings, if any: none relevant
  • CREATE TABLE statements for tables involved: see code snippets above (Decimal(38, 8), Decimal(9, 0), Int128)
  • Sample data for all these tables: literal values in the snippets above (10^31, 10^10, 2^128) — no obfuscation needed

Root cause

Panic pathlib/column/bigint.go:219-228:

func bigIntToRaw(dest []byte, v *big.Int) {
    var sign int
    if v.Sign() < 0 {
        v.Not(v).FillBytes(dest)  // panics if abs(v) doesn't fit in len(dest) bytes
        sign = -1
    } else {
        v.FillBytes(dest)          // same
    }
    endianSwap(dest, sign < 0)
}

No bounds check before FillBytes. Callers BigInt.append and Decimal.append (both unexported) have no error return either, so the error cannot be surfaced from any layer of the stack.

Silent-truncation pathlib/column/decimal.go:253-260:

case *proto.ColDecimal32:
    var part uint32
    part = uint32(decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).IntPart())
    vCol.Append(proto.Decimal32(part))
case *proto.ColDecimal64:
    var part uint64
    part = uint64(decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).IntPart())
    vCol.Append(proto.Decimal64(part))

IntPart() wraps for values beyond int64; the subsequent cast to uint32/uint64 keeps only the low bits.

Relation to prior work

Proposed fix direction

One approach that feels like a natural fit for the code as it stands — happy to follow a different path if maintainers see a cleaner one:

  1. Per-column pre-computed bound. At column construction (Decimal.parse / BigInt init), compute and cache max *big.Int (10^precision for Decimal; 2^(bits-1) / 2^bits for signed/unsigned BigInt). O(1) per row after that.
  2. Validation at the public entry point. In AppendRow / Append, compute the scaled big.Int once and Cmp against the cached bound. Handle the signed-min edge case for BigInt explicitly (Int128 min is -2^127, which has BitLen == 128 — naive checks reject it).
  3. Structured error. ColumnConverterError extension, a new ErrValueOutOfRange sentinel, or both — whichever matches maintainer preference.
  4. Internal helpers. Keep unexported bigIntToRaw / *.append signatures unchanged; they can rely on the caller having validated. Puts validation where the column-type context lives.
  5. Fix Decimal32/Decimal64 branches in Decimal.append so the silent-truncation hazard doesn't survive the fix.

Backwards compatibility

This is a behavior change: callers that today implicitly rely on the panic (or the silent truncation) will see a non-nil error from Append/AppendRow. No API shape change. Suggest shipping as a patch release with release notes highlighting the new error path. Optionally gate behind a clickhouse.Options flag for one release if a soft rollout is preferred, though for a fix to a documented-contract violation I'd lean default-on.

Offer to contribute

Happy to open a PR implementing this (or a different approach the maintainers prefer), with regression tests under tests/issues/issue_<THIS>_test.go per project convention. Before writing code I'd value a quick check on:

  1. Preferred error type, extend ColumnConverterError, introduce a sentinel, or both?
  2. Should the Decimal32/64 silent-truncation fix land in the same PR or a follow-up?
  3. Any objection to adding a pre-computed max *big.Int field to Decimal / BigInt structs?

Open to a narrower or wider scope, or a structurally different approach, if that fits the codebase better. Thanks for maintaining this driver.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions