From 2d8666bc951491bd16f1d63af72f1d24c07a87b7 Mon Sep 17 00:00:00 2001 From: wucm667 Date: Thu, 7 May 2026 15:25:08 +0800 Subject: [PATCH 1/3] fix(column): return error instead of panic on Decimal/BigInt overflow When inserting values that exceed the range of Decimal128/256 or Int128/UInt128/Int256/UInt256 columns, the driver previously panicked with 'math/big: buffer too small' in bigIntToRaw(), killing the caller goroutine with no way to recover. Changes: - bigIntToRaw now uses deferred recover to catch FillBytes panics and returns a proper error instead - Added explicit signed range check: positive values exceeding 2^(n*8-1)-1 are caught before FillBytes (which treats values as unsigned and won't panic for these) - Decimal32/64 now check for overflow before silent IntPart() truncation - append() in both BigInt and Decimal now propagate errors through Append() and AppendRow() - Added unit tests for all four Decimal sizes and BigInt overflow/valid boundary cases Fixes #1849 Signed-off-by: wucm667 --- lib/column/bigint.go | 57 +++++++++++++++----- lib/column/decimal.go | 66 +++++++++++++++--------- lib/column/decimal_overflow_test.go | 80 +++++++++++++++++++++++++++++ 3 files changed, 168 insertions(+), 35 deletions(-) create mode 100644 lib/column/decimal_overflow_test.go diff --git a/lib/column/bigint.go b/lib/column/bigint.go index 6c9e4c26d9..906c18a16a 100644 --- a/lib/column/bigint.go +++ b/lib/column/bigint.go @@ -68,17 +68,23 @@ func (col *BigInt) Append(v any) (nulls []uint8, err error) { case []big.Int: nulls = make([]uint8, len(v)) for i := range v { - col.append(&v[i]) + if err := col.append(&v[i]); err != nil { + return nil, err + } } case []*big.Int: nulls = make([]uint8, len(v)) for i := range v { switch { case v[i] != nil: - col.append(v[i]) + if err := col.append(v[i]); err != nil { + return nil, err + } default: nulls[i] = 1 - col.append(big.NewInt(0)) + if err := col.append(big.NewInt(0)); err != nil { + return nil, err + } } } default: @@ -106,16 +112,16 @@ func (col *BigInt) Append(v any) (nulls []uint8, err error) { func (col *BigInt) AppendRow(v any) error { switch v := v.(type) { case big.Int: - col.append(&v) + return col.append(&v) case *big.Int: switch { case v != nil: - col.append(v) + return col.append(v) default: - col.append(big.NewInt(0)) + return col.append(big.NewInt(0)) } case nil: - col.append(big.NewInt(0)) + return col.append(big.NewInt(0)) default: if valuer, ok := v.(driver.Valuer); ok { val, err := valuer.Value() @@ -135,7 +141,6 @@ func (col *BigInt) AppendRow(v any) error { From: fmt.Sprintf("%T", v), } } - return nil } func (col *BigInt) Decode(reader *proto.Reader, rows int) error { @@ -177,9 +182,16 @@ func (col *BigInt) row(i int) *big.Int { return big.NewInt(0) } -func (col *BigInt) append(v *big.Int) { +func (col *BigInt) append(v *big.Int) error { dest := make([]byte, col.size) - bigIntToRaw(dest, new(big.Int).Set(v)) + if err := bigIntToRaw(dest, new(big.Int).Set(v), col.signed); err != nil { + return &ColumnConverterError{ + Op: "Append", + To: string(col.chType), + From: "big.Int", + Hint: err.Error(), + } + } switch v := col.col.(type) { case *proto.ColInt128: v.Append(proto.Int128{ @@ -214,17 +226,38 @@ func (col *BigInt) append(v *big.Int) { }, }) } + return nil } -func bigIntToRaw(dest []byte, v *big.Int) { +func bigIntToRaw(dest []byte, v *big.Int, signed bool) (err error) { + defer func() { + if r := recover(); r != nil { + err = fmt.Errorf("value overflows %d-byte buffer", len(dest)) + } + }() + + // For signed types, FillBytes treats the value as unsigned so it won't + // catch values that fit unsigned but exceed the signed range (e.g., 2^127 + // fits in 16 bytes but is out of Int128 range). Check explicitly. + if signed && v.Sign() >= 0 { + maxBits := len(dest)*8 - 1 + if v.BitLen() > maxBits { + return fmt.Errorf("value overflows %d-byte signed buffer", len(dest)) + } + } + var sign int if v.Sign() < 0 { - v.Not(v).FillBytes(dest) + if !signed { + return fmt.Errorf("negative value not allowed for unsigned type") + } + new(big.Int).Not(v).FillBytes(dest) sign = -1 } else { v.FillBytes(dest) } endianSwap(dest, sign < 0) + return nil } func rawToBigInt(v []byte, signed bool) *big.Int { diff --git a/lib/column/decimal.go b/lib/column/decimal.go index 66d103d873..7ce478967a 100644 --- a/lib/column/decimal.go +++ b/lib/column/decimal.go @@ -6,7 +6,7 @@ import ( "encoding/binary" "errors" "fmt" - "math/big" + "math" "reflect" "strconv" "strings" @@ -139,18 +139,24 @@ func (col *Decimal) Append(v any) (nulls []uint8, err error) { case []decimal.Decimal: nulls = make([]uint8, len(v)) for i := range v { - col.append(&v[i]) + if err := col.append(&v[i]); err != nil { + return nil, err + } } case []*decimal.Decimal: nulls = make([]uint8, len(v)) for i := range v { switch { case v[i] != nil: - col.append(v[i]) + if err := col.append(v[i]); err != nil { + return nil, err + } default: nulls[i] = 1 value := decimal.New(0, 0) - col.append(&value) + if err := col.append(&value); err != nil { + return nil, err + } } } case []string: @@ -160,7 +166,9 @@ func (col *Decimal) Append(v any) (nulls []uint8, err error) { if err != nil { return nil, fmt.Errorf("could not convert \"%v\" to decimal: %w", v[i], err) } - col.append(&d) + if err := col.append(&d); err != nil { + return nil, err + } } case []*string: nulls = make([]uint8, len(v)) @@ -168,8 +176,9 @@ func (col *Decimal) Append(v any) (nulls []uint8, err error) { if v[i] == nil { nulls[i] = 1 value := decimal.New(0, 0) - col.append(&value) - + if err := col.append(&value); err != nil { + return nil, err + } continue } @@ -177,7 +186,9 @@ func (col *Decimal) Append(v any) (nulls []uint8, err error) { if err != nil { return nil, fmt.Errorf("could not convert \"%v\" to decimal: %w", *v[i], err) } - col.append(&d) + if err := col.append(&d); err != nil { + return nil, err + } } default: if valuer, ok := v.(driver.Valuer); ok { @@ -244,34 +255,42 @@ func (col *Decimal) AppendRow(v any) error { From: fmt.Sprintf("%T", v), } } - col.append(&value) - return nil + return col.append(&value) } -func (col *Decimal) append(v *decimal.Decimal) { +func (col *Decimal) append(v *decimal.Decimal) error { switch vCol := col.col.(type) { case *proto.ColDecimal32: - var part uint32 - part = uint32(decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).IntPart()) - vCol.Append(proto.Decimal32(part)) + scaled := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)) + bi := scaled.BigInt() + i64 := bi.Int64() + if !bi.IsInt64() || i64 > math.MaxInt32 || i64 < math.MinInt32 { + return fmt.Errorf("value overflow: %s exceeds Decimal32 range", v.String()) + } + vCol.Append(proto.Decimal32(uint32(i64))) case *proto.ColDecimal64: - var part uint64 - part = uint64(decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).IntPart()) - vCol.Append(proto.Decimal64(part)) + scaled := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)) + bi := scaled.BigInt() + if !bi.IsInt64() { + return fmt.Errorf("value overflow: %s exceeds Decimal64 range", v.String()) + } + vCol.Append(proto.Decimal64(uint64(bi.Int64()))) case *proto.ColDecimal128: - var bi *big.Int - bi = decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).BigInt() + bi := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).BigInt() dest := make([]byte, 16) - bigIntToRaw(dest, bi) + if err := bigIntToRaw(dest, bi, true); err != nil { + return fmt.Errorf("value overflow: %s exceeds Decimal128 range", v.String()) + } vCol.Append(proto.Decimal128{ Low: binary.LittleEndian.Uint64(dest[0 : 64/8]), High: binary.LittleEndian.Uint64(dest[64/8 : 128/8]), }) case *proto.ColDecimal256: - var bi *big.Int - bi = decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).BigInt() + bi := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).BigInt() dest := make([]byte, 32) - bigIntToRaw(dest, bi) + if err := bigIntToRaw(dest, bi, true); err != nil { + return fmt.Errorf("value overflow: %s exceeds Decimal256 range", v.String()) + } vCol.Append(proto.Decimal256{ Low: proto.UInt128{ Low: binary.LittleEndian.Uint64(dest[0 : 64/8]), @@ -283,6 +302,7 @@ func (col *Decimal) append(v *decimal.Decimal) { }, }) } + return nil } func (col *Decimal) Decode(reader *proto.Reader, rows int) error { diff --git a/lib/column/decimal_overflow_test.go b/lib/column/decimal_overflow_test.go new file mode 100644 index 0000000000..339e494994 --- /dev/null +++ b/lib/column/decimal_overflow_test.go @@ -0,0 +1,80 @@ +package column + +import ( + "math/big" + "testing" + + "github.com/ClickHouse/ch-go/proto" + "github.com/shopspring/decimal" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDecimal32OverflowReturnsError(t *testing.T) { + col := &Decimal{} + _, err := col.parse("Decimal(9, 2)") + require.NoError(t, err) + + // max int32 is 2147483647; scaled by 10^2 the max representable value is ~21474836.47 + overflow := decimal.NewFromFloat(21474836.48) + err = col.AppendRow(overflow) + assert.ErrorContains(t, err, "overflow") +} + +func TestDecimal64OverflowReturnsError(t *testing.T) { + col := &Decimal{} + _, err := col.parse("Decimal(18, 2)") + require.NoError(t, err) + + // max int64 is 9223372036854775807; scaled by 10^2 the max representable value is ~92233720368547758.07 + overflow := decimal.NewFromFloat(92233720368547758.08) + err = col.AppendRow(overflow) + assert.ErrorContains(t, err, "overflow") +} + +func TestDecimal128OverflowReturnsError(t *testing.T) { + col := &Decimal{} + _, err := col.parse("Decimal(38, 0)") + require.NoError(t, err) + + // 2^127 exceeds Decimal128 signed range + big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) + overflow := decimal.NewFromBigInt(big2_127, 0) + err = col.AppendRow(overflow) + assert.ErrorContains(t, err, "overflow") +} + +func TestDecimal256OverflowReturnsError(t *testing.T) { + col := &Decimal{} + _, err := col.parse("Decimal(76, 0)") + require.NoError(t, err) + + // 2^255 exceeds Decimal256 signed range + big2_255 := new(big.Int).Lsh(big.NewInt(1), 255) + overflow := decimal.NewFromBigInt(big2_255, 0) + err = col.AppendRow(overflow) + assert.ErrorContains(t, err, "overflow") +} + +func TestBigIntOverflowReturnsError(t *testing.T) { + // Int128: signed 128-bit, max positive is 2^127-1 + col128 := &BigInt{size: 16, chType: "Int128", signed: true, col: &proto.ColInt128{}} + + big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) + err := col128.AppendRow(*big2_127) + assert.ErrorContains(t, err, "overflow") +} + +func TestBigIntValidValuesNoError(t *testing.T) { + col128 := &BigInt{size: 16, chType: "Int128", signed: true, col: &proto.ColInt128{}} + + // 2^127 - 1 is the max valid Int128 value + maxInt128 := new(big.Int).Sub(new(big.Int).Lsh(big.NewInt(1), 127), big.NewInt(1)) + err := col128.AppendRow(*maxInt128) + assert.NoError(t, err) + + // min valid Int128 value is -2^127 + minInt128 := new(big.Int).Neg(new(big.Int).Lsh(big.NewInt(1), 127)) + err = col128.AppendRow(*minInt128) + assert.NoError(t, err) +} From af19df4da0bd332696e1b516cfdbf91635983fe9 Mon Sep 17 00:00:00 2001 From: wucm667 Date: Mon, 11 May 2026 21:05:27 +0800 Subject: [PATCH 2/3] fix(column): address negative overflow and review comments - Add negative signed overflow check in bigIntToRaw: values in (-2^n, -2^(n-1)-1] no longer silently encode incorrectly - Remove defer/recover since explicit BitLen guards now catch all overflow cases - Remove unnecessary new(big.Int).Set(v) defensive copy - Fix error message style: lowercase, no colon, consistent with project conventions - Use NewFromString instead of NewFromFloat for exact boundary test values - Add TestBigIntNegativeOverflowReturnsError - Add regression test tests/issues/issue_1849_test.go Signed-off-by: wucm667 --- lib/column/bigint.go | 35 +++++++------- lib/column/decimal.go | 13 +++-- lib/column/decimal_overflow_test.go | 16 ++++++- tests/issues/issue_1849_test.go | 74 +++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 26 deletions(-) create mode 100644 tests/issues/issue_1849_test.go diff --git a/lib/column/bigint.go b/lib/column/bigint.go index 906c18a16a..be45826a2b 100644 --- a/lib/column/bigint.go +++ b/lib/column/bigint.go @@ -184,7 +184,7 @@ func (col *BigInt) row(i int) *big.Int { func (col *BigInt) append(v *big.Int) error { dest := make([]byte, col.size) - if err := bigIntToRaw(dest, new(big.Int).Set(v), col.signed); err != nil { + if err := bigIntToRaw(dest, v, col.signed); err != nil { return &ColumnConverterError{ Op: "Append", To: string(col.chType), @@ -229,28 +229,29 @@ func (col *BigInt) append(v *big.Int) error { return nil } -func bigIntToRaw(dest []byte, v *big.Int, signed bool) (err error) { - defer func() { - if r := recover(); r != nil { - err = fmt.Errorf("value overflows %d-byte buffer", len(dest)) +func bigIntToRaw(dest []byte, v *big.Int, signed bool) error { + bits := len(dest) * 8 + if signed { + if v.Sign() >= 0 { + if v.BitLen() > bits-1 { + return fmt.Errorf("value overflows %d-byte signed buffer", len(dest)) + } + } else { + if new(big.Int).Not(v).BitLen() > bits-1 { + return fmt.Errorf("value overflows %d-byte signed buffer", len(dest)) + } } - }() - - // For signed types, FillBytes treats the value as unsigned so it won't - // catch values that fit unsigned but exceed the signed range (e.g., 2^127 - // fits in 16 bytes but is out of Int128 range). Check explicitly. - if signed && v.Sign() >= 0 { - maxBits := len(dest)*8 - 1 - if v.BitLen() > maxBits { - return fmt.Errorf("value overflows %d-byte signed buffer", len(dest)) + } else { + if v.Sign() < 0 { + return fmt.Errorf("negative value not allowed for unsigned type") + } + if v.BitLen() > bits { + return fmt.Errorf("value overflows %d-byte unsigned buffer", len(dest)) } } var sign int if v.Sign() < 0 { - if !signed { - return fmt.Errorf("negative value not allowed for unsigned type") - } new(big.Int).Not(v).FillBytes(dest) sign = -1 } else { diff --git a/lib/column/decimal.go b/lib/column/decimal.go index 7ce478967a..aab7e5e010 100644 --- a/lib/column/decimal.go +++ b/lib/column/decimal.go @@ -263,23 +263,22 @@ func (col *Decimal) append(v *decimal.Decimal) error { case *proto.ColDecimal32: scaled := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)) bi := scaled.BigInt() - i64 := bi.Int64() - if !bi.IsInt64() || i64 > math.MaxInt32 || i64 < math.MinInt32 { - return fmt.Errorf("value overflow: %s exceeds Decimal32 range", v.String()) + if !bi.IsInt64() || bi.Int64() > math.MaxInt32 || bi.Int64() < math.MinInt32 { + return fmt.Errorf("value %s overflows decimal32 range", v.String()) } - vCol.Append(proto.Decimal32(uint32(i64))) + vCol.Append(proto.Decimal32(uint32(bi.Int64()))) case *proto.ColDecimal64: scaled := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)) bi := scaled.BigInt() if !bi.IsInt64() { - return fmt.Errorf("value overflow: %s exceeds Decimal64 range", v.String()) + return fmt.Errorf("value %s overflows decimal64 range", v.String()) } vCol.Append(proto.Decimal64(uint64(bi.Int64()))) case *proto.ColDecimal128: bi := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).BigInt() dest := make([]byte, 16) if err := bigIntToRaw(dest, bi, true); err != nil { - return fmt.Errorf("value overflow: %s exceeds Decimal128 range", v.String()) + return fmt.Errorf("value %s overflows decimal128 range", v.String()) } vCol.Append(proto.Decimal128{ Low: binary.LittleEndian.Uint64(dest[0 : 64/8]), @@ -289,7 +288,7 @@ func (col *Decimal) append(v *decimal.Decimal) error { bi := decimal.NewFromBigInt(v.Coefficient(), v.Exponent()+int32(col.scale)).BigInt() dest := make([]byte, 32) if err := bigIntToRaw(dest, bi, true); err != nil { - return fmt.Errorf("value overflow: %s exceeds Decimal256 range", v.String()) + return fmt.Errorf("value %s overflows decimal256 range", v.String()) } vCol.Append(proto.Decimal256{ Low: proto.UInt128{ diff --git a/lib/column/decimal_overflow_test.go b/lib/column/decimal_overflow_test.go index 339e494994..da4d333980 100644 --- a/lib/column/decimal_overflow_test.go +++ b/lib/column/decimal_overflow_test.go @@ -16,7 +16,8 @@ func TestDecimal32OverflowReturnsError(t *testing.T) { require.NoError(t, err) // max int32 is 2147483647; scaled by 10^2 the max representable value is ~21474836.47 - overflow := decimal.NewFromFloat(21474836.48) + overflow, err := decimal.NewFromString("21474836.48") + require.NoError(t, err) err = col.AppendRow(overflow) assert.ErrorContains(t, err, "overflow") } @@ -27,7 +28,8 @@ func TestDecimal64OverflowReturnsError(t *testing.T) { require.NoError(t, err) // max int64 is 9223372036854775807; scaled by 10^2 the max representable value is ~92233720368547758.07 - overflow := decimal.NewFromFloat(92233720368547758.08) + overflow, err := decimal.NewFromString("92233720368547758.08") + require.NoError(t, err) err = col.AppendRow(overflow) assert.ErrorContains(t, err, "overflow") } @@ -78,3 +80,13 @@ func TestBigIntValidValuesNoError(t *testing.T) { err = col128.AppendRow(*minInt128) assert.NoError(t, err) } + +func TestBigIntNegativeOverflowReturnsError(t *testing.T) { + col128 := &BigInt{size: 16, chType: "Int128", signed: true, col: &proto.ColInt128{}} + + // -2^127 - 1 is below the minimum Int128 value (-2^127) + minInt128 := new(big.Int).Neg(new(big.Int).Lsh(big.NewInt(1), 127)) + overflow := new(big.Int).Sub(minInt128, big.NewInt(1)) + err := col128.AppendRow(*overflow) + assert.ErrorContains(t, err, "overflow") +} diff --git a/tests/issues/issue_1849_test.go b/tests/issues/issue_1849_test.go new file mode 100644 index 0000000000..9637fff3ff --- /dev/null +++ b/tests/issues/issue_1849_test.go @@ -0,0 +1,74 @@ +package issues + +import ( + "context" + "math/big" + "testing" + + "github.com/shopspring/decimal" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + clickhouse_tests "github.com/ClickHouse/clickhouse-go/v2/tests" +) + +// TestIssue1849 verifies that appending overflow values to Decimal and BigInt +// columns returns an error instead of panicking. +func TestIssue1849(t *testing.T) { + ctx := context.Background() + + conn, err := clickhouse_tests.GetConnectionTCP("issues", nil, nil, nil) + require.NoError(t, err) + defer conn.Close() + + const ddl = ` + CREATE TABLE test_issue_1849 ( + d128 Decimal(38, 0), + i128 Int128 + ) Engine MergeTree() ORDER BY tuple() + ` + conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_1849") + require.NoError(t, conn.Exec(ctx, ddl)) + defer conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_1849") + + t.Run("Decimal128PositiveOverflow", func(t *testing.T) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") + require.NoError(t, err) + + big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) + overflow := decimal.NewFromBigInt(big2_127, 0) + err = batch.AppendRow(overflow, big.NewInt(0)) + assert.ErrorContains(t, err, "overflow") + }) + + t.Run("Decimal128NegativeOverflow", func(t *testing.T) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") + require.NoError(t, err) + + // -2^127 - 1 is below the minimum Decimal128 signed range + minVal := new(big.Int).Neg(new(big.Int).Lsh(big.NewInt(1), 127)) + overflow := decimal.NewFromBigInt(new(big.Int).Sub(minVal, big.NewInt(1)), 0) + err = batch.AppendRow(overflow, big.NewInt(0)) + assert.ErrorContains(t, err, "overflow") + }) + + t.Run("Int128PositiveOverflow", func(t *testing.T) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") + require.NoError(t, err) + + big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) + err = batch.AppendRow(decimal.NewFromBigInt(big.NewInt(0), 0), *big2_127) + assert.ErrorContains(t, err, "overflow") + }) + + t.Run("Int128NegativeOverflow", func(t *testing.T) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") + require.NoError(t, err) + + // -2^127 - 1 is below the minimum Int128 value (-2^127) + minInt128 := new(big.Int).Neg(new(big.Int).Lsh(big.NewInt(1), 127)) + overflow := new(big.Int).Sub(minInt128, big.NewInt(1)) + err = batch.AppendRow(decimal.NewFromBigInt(big.NewInt(0), 0), *overflow) + assert.ErrorContains(t, err, "overflow") + }) +} From d8fdbe190e052e66c4238542e476793c24a0845c Mon Sep 17 00:00:00 2001 From: wucm667 Date: Tue, 19 May 2026 11:08:13 +0800 Subject: [PATCH 3/3] fix(test): use Batch.Append and add HTTP/std coverage for issue 1849 The regression test called batch.AppendRow which does not exist on the Batch interface (it belongs to BatchColumn), causing a build failure across all CI matrix jobs. Rewrite the test to: - Use batch.Append (the correct Batch method) - Cover both native TCP and HTTP protocols - Cover the database/sql surface via Begin/Prepare/Exec - Test positive and negative overflow for both Decimal128 and Int128 --- tests/issues/issue_1849_test.go | 154 ++++++++++++++++++++++---------- 1 file changed, 107 insertions(+), 47 deletions(-) diff --git a/tests/issues/issue_1849_test.go b/tests/issues/issue_1849_test.go index 9637fff3ff..3a1ad427aa 100644 --- a/tests/issues/issue_1849_test.go +++ b/tests/issues/issue_1849_test.go @@ -2,73 +2,133 @@ package issues import ( "context" + "database/sql" + "fmt" "math/big" + "strconv" "testing" "github.com/shopspring/decimal" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/ClickHouse/clickhouse-go/v2" clickhouse_tests "github.com/ClickHouse/clickhouse-go/v2/tests" + clickhouse_std_tests "github.com/ClickHouse/clickhouse-go/v2/tests/std" ) -// TestIssue1849 verifies that appending overflow values to Decimal and BigInt -// columns returns an error instead of panicking. +// overflow1849Cases captures the four overflow scenarios that previously +// either panicked (BigInt) or silently truncated (Decimal). For each case the +// driver must now return an error containing "overflow". +func overflow1849Cases(t *testing.T) []struct { + name string + d128 any + i128 any +} { + t.Helper() + big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) + min128 := new(big.Int).Neg(big2_127) + belowMin128 := new(big.Int).Sub(min128, big.NewInt(1)) + + return []struct { + name string + d128 any + i128 any + }{ + {"Decimal128PositiveOverflow", decimal.NewFromBigInt(big2_127, 0), big.NewInt(0)}, + {"Decimal128NegativeOverflow", decimal.NewFromBigInt(belowMin128, 0), big.NewInt(0)}, + {"Int128PositiveOverflow", decimal.NewFromBigInt(big.NewInt(0), 0), *big2_127}, + {"Int128NegativeOverflow", decimal.NewFromBigInt(big.NewInt(0), 0), *belowMin128}, + } +} + +// TestIssue1849 verifies that appending overflow values to Decimal and +// BigInt columns returns an error instead of panicking. Covered surfaces: +// - native driver.Conn over TCP +// - native driver.Conn over HTTP +// - database/sql over TCP +// - database/sql over HTTP +// +// Regression test for https://github.com/ClickHouse/clickhouse-go/issues/1849. func TestIssue1849(t *testing.T) { - ctx := context.Background() + const ddl = `CREATE TABLE test_issue_1849 ( + d128 Decimal(38, 0), + i128 Int128 + ) Engine MergeTree() ORDER BY tuple()` - conn, err := clickhouse_tests.GetConnectionTCP("issues", nil, nil, nil) - require.NoError(t, err) - defer conn.Close() - - const ddl = ` - CREATE TABLE test_issue_1849 ( - d128 Decimal(38, 0), - i128 Int128 - ) Engine MergeTree() ORDER BY tuple() - ` - conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_1849") - require.NoError(t, conn.Exec(ctx, ddl)) - defer conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_1849") - - t.Run("Decimal128PositiveOverflow", func(t *testing.T) { - batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") - require.NoError(t, err) + t.Run("Native", func(t *testing.T) { + ctx := context.Background() + for _, protocol := range []clickhouse.Protocol{clickhouse.Native, clickhouse.HTTP} { + t.Run(protocol.String(), func(t *testing.T) { + conn, err := clickhouse_tests.GetConnection(testSet, t, protocol, nil, nil, nil) + require.NoError(t, err) + t.Cleanup(func() { conn.Close() }) - big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) - overflow := decimal.NewFromBigInt(big2_127, 0) - err = batch.AppendRow(overflow, big.NewInt(0)) - assert.ErrorContains(t, err, "overflow") - }) + require.NoError(t, conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_1849")) + require.NoError(t, conn.Exec(ctx, ddl)) + t.Cleanup(func() { _ = conn.Exec(ctx, "DROP TABLE IF EXISTS test_issue_1849") }) - t.Run("Decimal128NegativeOverflow", func(t *testing.T) { - batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") - require.NoError(t, err) + for _, tc := range overflow1849Cases(t) { + t.Run(tc.name, func(t *testing.T) { + batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") + require.NoError(t, err) + t.Cleanup(func() { _ = batch.Abort() }) - // -2^127 - 1 is below the minimum Decimal128 signed range - minVal := new(big.Int).Neg(new(big.Int).Lsh(big.NewInt(1), 127)) - overflow := decimal.NewFromBigInt(new(big.Int).Sub(minVal, big.NewInt(1)), 0) - err = batch.AppendRow(overflow, big.NewInt(0)) - assert.ErrorContains(t, err, "overflow") + err = batch.Append(tc.d128, tc.i128) + assert.ErrorContains(t, err, "overflow") + }) + } + }) + } }) - t.Run("Int128PositiveOverflow", func(t *testing.T) { - batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") + t.Run("Std", func(t *testing.T) { + useSSL, err := strconv.ParseBool(clickhouse_tests.GetEnv("CLICKHOUSE_USE_SSL", "false")) require.NoError(t, err) - big2_127 := new(big.Int).Lsh(big.NewInt(1), 127) - err = batch.AppendRow(decimal.NewFromBigInt(big.NewInt(0), 0), *big2_127) - assert.ErrorContains(t, err, "overflow") - }) + for _, protocol := range []clickhouse.Protocol{clickhouse.Native, clickhouse.HTTP} { + t.Run(protocol.String(), func(t *testing.T) { + db, err := clickhouse_std_tests.GetDSNConnection(testSet, protocol, useSSL, nil) + require.NoError(t, err) + t.Cleanup(func() { db.Close() }) - t.Run("Int128NegativeOverflow", func(t *testing.T) { - batch, err := conn.PrepareBatch(ctx, "INSERT INTO test_issue_1849") - require.NoError(t, err) + _, _ = db.Exec("DROP TABLE IF EXISTS test_issue_1849") + _, err = db.Exec(ddl) + require.NoError(t, err) + t.Cleanup(func() { _, _ = db.Exec("DROP TABLE IF EXISTS test_issue_1849") }) - // -2^127 - 1 is below the minimum Int128 value (-2^127) - minInt128 := new(big.Int).Neg(new(big.Int).Lsh(big.NewInt(1), 127)) - overflow := new(big.Int).Sub(minInt128, big.NewInt(1)) - err = batch.AppendRow(decimal.NewFromBigInt(big.NewInt(0), 0), *overflow) - assert.ErrorContains(t, err, "overflow") + for _, tc := range overflow1849Cases(t) { + t.Run(tc.name, func(t *testing.T) { + appendErr := stdInsertOverflow(db, tc.d128, tc.i128) + assert.ErrorContains(t, appendErr, "overflow") + }) + } + }) + } }) } + +// stdInsertOverflow runs a single-row INSERT through the database/sql +// surface and returns the first error encountered. The overflow check +// fires inside the column converter, so the error normally surfaces from +// ExecContext. We still drive the full Begin → Prepare → Exec → Commit +// flow so any caller-visible regression is caught. +func stdInsertOverflow(db *sql.DB, d128, i128 any) error { + ctx := context.Background() + scope, err := db.BeginTx(ctx, nil) + if err != nil { + return fmt.Errorf("begin: %w", err) + } + defer func() { _ = scope.Rollback() }() + + stmt, err := scope.PrepareContext(ctx, "INSERT INTO test_issue_1849") + if err != nil { + return fmt.Errorf("prepare: %w", err) + } + defer stmt.Close() + + if _, err := stmt.ExecContext(ctx, d128, i128); err != nil { + return err + } + return scope.Commit() +}