Fix Dynamic: infer Decimal scale from the value to prevent silent truncation

[polyglotAI-bot]

Description

Fixes #466.

When a decimal or ClickHouseDecimal was written into a Dynamic column, the ClickHouse type was inferred from the .NET type, never the value. DynamicType.Write used a per-System.Type cache (GetCachedInferredType), and TypeConverter maps both decimal and ClickHouseDecimal to a hardcoded Decimal128(38, 9). Any value with scale > 9 was therefore silently reduced to 9 fractional digits on write — DecimalType.WriteScaled calls ClickHouseDecimal.ScaleMantissa(value, 9), which is integer division (truncation toward zero, no rounding, no error). Decimal128 has room for 38 digits, so this was pure data loss: 0.0000000001m was stored as 0.

The type header and the payload in a Dynamic value are both produced from the same inferred type object, so there is a single source of truth to fix — no scale/width mismatch between header and data.

Changes

• ClickHouse.Driver/Types/TypeConverter.cs — new internal static InferDecimalType(ClickHouseDecimal) that derives the scale from the value's own scale and picks the narrowest Decimal width whose precision (max(significant-digits, scale)) covers the value: Decimal32 (≤9), Decimal64 (≤18), Decimal128 (≤38), Decimal256 (≤76). It throws ArgumentOutOfRangeException (matching the sibling throw in DecimalType.Write) when the value needs more than the 76 digits ClickHouse supports. Because the chosen scale always equals the value's scale, ScaleMantissa never divides, so no digits are dropped.
• ClickHouse.Driver/Types/DynamicType.cs — Write now routes a scalar decimal/ClickHouseDecimal value through InferDecimalType (value-aware) instead of the per-Type cache; all other types keep using the cache unchanged.

No public API surface changes (both new/changed members are internal).

Test

Write_DecimalWithScaleAbove9_ShouldRoundTripWithoutTruncation (parametrized TestCaseSource in DynamicTests.cs) writes each value into a Dynamic column via ClickHouseBulkCopy and asserts the read-back ClickHouseDecimal equals the original. Cases span the value-axes and all four widths:

• scale 16 (the issue's example, Decimal64), tiny scale-10 magnitude (0.0000000001, previously stored as 0), an integer-part + scale-10 value, a negative scale-16 value, a max-System.Decimal-scale value (scale 28, Decimal128), and a ClickHouseDecimal of scale 40 that only fits Decimal256;
• a digits-dominated case (28 significant digits, scale 10) whose width must be chosen by the digit count (Decimal128), not the scale alone;
• a low-scale contrast case (scale 6) that already round-tripped and must keep round-tripping unchanged (now via a narrower Decimal32).

Verified: all high-scale cases FAIL on main (truncated to scale 9) and PASS with the fix; the contrast case passes on both. The full DynamicTests + all Decimal tests (3283) pass, and the Release build is clean.

Pre-PR validation gate

• Deterministic repro confirmed (6/7 cases fail on main, all pass with the fix)
• Root cause documented above
• Fix targets the root cause (value-aware scale inference, not a symptom guard)
• Test fails without fix, passes with fix; expected values derived from the intended round-trip
• No existing tests broken; no existing test weakened
• Convention compliance verified per AGENTS.md (module, randomized qualified test tables, parametrized TestCaseSource, test naming); CHANGELOG.md + RELEASENOTES.md updated

Notes

• Scope is deliberately the scalar decimal write path reported in Decimal values written to a Dynamic column are silently truncated to scale 9 #466. A collection of decimals written to a Dynamic column (e.g. List<decimal>) still routes through the type cache and would need the broader value-aware collection-inference change; that is tracked separately and intentionally not bundled here.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Fixes silent precision loss when writing decimal/ClickHouseDecimal values into ClickHouse Dynamic columns by making decimal type inference value-aware (scale derived from the value, and width chosen to fit the required precision), preventing truncation to a fixed Decimal128(38, 9).

Changes:

• Added TypeConverter.InferDecimalType(ClickHouseDecimal) to compute scale/precision from the value and select Decimal32/64/128/256 accordingly (throwing when exceeding ClickHouse’s 76-digit limit).
• Updated DynamicType.Write to bypass the per-Type inference cache for scalar decimals and use the new value-aware inference.
• Added regression coverage in DynamicTests to ensure round-tripping works for scales > 9 across all decimal widths; documented the fix in changelog/release notes.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
RELEASENOTES.md	Documents the Dynamic decimal truncation fix and the new inference behavior.
ClickHouse.Driver/Types/TypeConverter.cs	Introduces value-aware decimal type inference selecting the narrowest compatible ClickHouse decimal width.
ClickHouse.Driver/Types/DynamicType.cs	Uses per-value inference for scalar decimals to avoid cached fixed-scale truncation.
ClickHouse.Driver.Tests/Types/DynamicTests.cs	Adds regression test cases covering high-scale and width-selection scenarios for Dynamic decimals.
CHANGELOG.md	Records the bug fix for silent decimal precision loss in Dynamic writes.

[alex-clickhouse]

Would it make sense to just have a single default that can encompass the entire possible range of C# decimals, instead of inspecting the value?

[polyglotAI-bot]

Good question — I looked into whether a single fixed default could replace the per-value inference. It works for System.Decimal on its own, but the same write path also serializes ClickHouseDecimal, which is arbitrary-precision, and no fixed type can cover that without re-introducing the truncation #466 is about. Details below (verified against a 26.5 server):

1. A single default that losslessly covers every System.Decimal has to be Decimal256.
It must never truncate, so the fixed scale has to be ≥ 28 (the max C# scale); the widest C# decimal — decimal.MaxValue, 29 integer digits at scale 0 — then needs 29 + 28 = 57 digits of precision. Decimal128(38, 28) overflows on it; only Decimal256 has room:

SELECT toDecimal128('79228162514264337593543950335', 28)
  -- Code: 69. ARGUMENT_OUT_OF_BOUND: 29 digits, expected scale 28 and precision 38
SELECT toDecimal256('79228162514264337593543950335', 28)   -- ok

So a single default makes every decimal in a Dynamic column 32 bytes on the wire, versus the 4/8/16 the value-aware path uses for the common smaller values.

2. The real blocker: ClickHouseDecimal is arbitrary-precision.
DynamicType.Write serializes both decimal and ClickHouseDecimal (a public struct with a BigInteger mantissa and an unbounded Scale); #466 names both, and the regression test round-trips a scale-40 value. There is no fixed Decimal(P, S) that holds every ClickHouseDecimal — you'd need S ≥ 76 and P − S ≥ 76 at the same time, i.e. P ≥ 152 > 76. A single Decimal256(76, 28) default silently truncates anything with scale > 28 — the same data loss, just at scale 28 instead of 9:

-- value-aware Decimal256(76, 40):  0.000000000012345678901234567890123456789   (round-trips)
-- single default Decimal256(76, 28): 0.0000000000123456789012345678             (12 digits dropped)

3. Deriving the scale from the value is already the established pattern here.
decimal query parameters are resolved as Decimal128(scale) with the scale read from the value's bits (parameter type-resolution rule #4 in AGENTS.md), so per-value scale on the write path isn't a new concept.

Given #2, I'd keep the value-aware inference: the value-derived scale is required for the ClickHouseDecimal path, and it also keeps the common decimal cases at the narrowest width. If the four-way width switch is the concern, I can collapse it — every System.Decimal fits Decimal128, so I'd use a fixed Decimal128 width + the value's scale for decimal and widen to Decimal256 only for the larger ClickHouseDecimal values (one branch instead of four). Happy to do that if you'd prefer — just let me know which direction you'd like.