Fix ClickHouseDecimal: IConvertible narrowing and ChangeType stack overflow

[polyglotAI-bot]

Summary

Fixes #646.

The IConvertible members of ClickHouseDecimal had three defects:

• ToInt32 was (short)(int)this. The extra cast kept only 16 bits, so Convert.ToInt32(new ClickHouseDecimal(100000m)) returned -31072. Int32Type.Write calls Convert.ToInt32(object), so InsertBinaryAsync stored that wrong value in an Int32 column.
• ToChar, ToSByte, ToByte, ToInt16 and ToUInt16 narrowed the result of explicit operator int/uint with an unchecked cast, so an out-of-range value wrapped (40000 → Int16 gave -25536). Convert.ToInt16(decimal) and the server (DECIMAL_OVERFLOW) reject it.
• ToType fell back to Convert.ChangeType(this, conversionType, provider). Convert.ChangeType converts the primitive types itself and calls IConvertible.ToType for all other types. Thus a target such as decimal?, int?, Guid, DateTimeOffset, TimeSpan, an enum or ClickHouseDecimal itself recursed until a StackOverflowException, which stops the process.

Changes

• ClickHouse.Driver/Numerics/ClickHouseDecimal.cs:
  • ToInt32 returns (int)this.
  • The five narrowing members use checked(...), so they throw OverflowException when the integer part is out of range. The fractional part is still truncated toward zero first, as the server does (toUInt8(toDecimal32(255.9, 1)) is 255, and toUInt8(toDecimal32(256.5, 1)) overflows).
  • ToType keeps the BigInteger branch. It returns the value for typeof(ClickHouseDecimal) and typeof(object), and sends each IConvertible type to its ToXxx member. For all other types it throws InvalidCastException, as decimal does (Convert.DefaultToType). Enums are excluded explicitly, because they report the TypeCode of their underlying type. A null type still throws ArgumentNullException.
• changelog.d/646-clickhousedecimal-iconvertible.fixes.md: changelog fragment.

Not changed: in-range results, ToUInt32/ToInt64/ToUInt64/ToSingle/ToDouble/ToDecimal, ToDateTime (still NotSupportedException), and ChangeType to string and BigInteger. The TCP client is not affected (ClickHouseTcpDecimal does not implement IConvertible).

Other driver paths that reach these members get the same fix with no further change: Int8Type/UInt8Type/Int16Type/UInt16Type writes, Enum8Type/Enum16Type writes (now throw instead of writing a wrapped index), and the Time parameter path in HttpParameterFormatter (Convert.ToInt32).

PR #647 (for #642) edits other lines of the same two files. The two PRs do not overlap. ToType(typeof(BigInteger)) for values below 1 depends on the NumberOfDigits fix in #647, so this PR does not change it.

Test

• ClickHouseDecimalIntegerColumnTests (real server):
  • InsertBinaryAsync_ClickHouseDecimalIntoInt32Column_StoresExactValue: inserts 100000.00, -40000, int.MinValue and int.MaxValue into an Int32 column and reads them back. On main the stored values are -31072, 25536, 0 and -1.
  • InsertBinaryAsync_ClickHouseDecimalOutOfColumnRange_ThrowsOverflowException: for Int8, UInt8, Int16 and UInt16. On main the insert succeeds with a wrapped value.
• ClickHouseDecimalTests:
  • ChangeType_NarrowIntegralTypeInRange_ReturnsValue and ChangeType_NarrowIntegralTypeOutOfRange_ThrowsOverflowException: the boundaries of each narrowed type, fractional values, and a 70-digit mantissa. The expected values for fractional input come from the server.
  • ChangeType_TypeWithoutConversion_ThrowsInvalidCastException: decimal?, int?, Guid, DateTimeOffset, TimeSpan, an enum and IntPtr. On main the test host crashes with a stack overflow.
  • ToType_OwnTypeOrObject_ReturnsSameValue, ToType_ConvertibleType_MatchesChangeType and ToType_NullType_ThrowsArgumentNullException: direct calls to ToType. ChangeType never calls ToType for the primitive types, so these tests are the only coverage of the new mapping. They also check that each type is boxed as itself.

The full ClickHouse.Driver.Tests suite passes on net10.0 against ClickHouse 26.9 (11125 passed, 0 failed). No existing test was changed.

Checklist

• Unit and integration tests covering the common scenarios were added
• A human-readable description of the changes was provided to include in CHANGELOG
• Test fails on main and passes with the fix
• Fix is on the live write path (Int32Type.Write → Convert.ToInt32 → ToInt32)
• No public API signature change

🤖 Generated with Claude Code

[Copilot]

Copilot review overview

🟡 Changes recommended

The mandatory HTTP documentation update is missing, and the changelog entry needs condensation.

Review effort: Balanced
Findings: 2

Open (2)

• Document ClickHouseDecimal conversion behavior in docs/http.mdx · New
• Condense fragment to describe only corrected outcomes · New

What changed in this PR

Fixes ClickHouseDecimal conversions in the HTTP client to prevent value wrapping and recursive stack overflow.

Changes:

• Adds checked narrowing conversions and corrects ToInt32.
• Makes ToType safely dispatch supported conversions.
• Adds unit, integration, and changelog coverage.

File	Description
ClickHouse.Driver/​Numerics/​ClickHouseDecimal.cs	Corrects IConvertible behavior.
ClickHouse.Driver.Tests/​Numerics/​ClickHouseDecimalTests.cs	Covers conversion boundaries and unsupported types.
ClickHouse.Driver.Tests/​Numerics/​ClickHouseDecimalIntegerColumnTests.cs	Verifies integer-column writes against ClickHouse.
changelog.d/​646-clickhousedecimal-iconvertible.fixes.md	Records the conversion fixes.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codecov]

Codecov Report

❌ Patch coverage is 93.10345% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
ClickHouse.Driver/Numerics/ClickHouseDecimal.cs	93.10%	1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!