ClickHouse / ClickHouse/clickhouse-cs

Decimal values written to a Dynamic column are silently truncated to scale 9

Open
#466 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
C#
Stars
94
Forks
22
Avg merge
11h 26m
Merged PRs (30d)
22

Description

## Description

When a `decimal` or `ClickHouseDecimal` value is written into a `Dynamic` column, the ClickHouse type is inferred from the .NET **type**, never from the value:

- `DynamicType.Write` (`ClickHouse.Driver/Types/DynamicType.cs:29-45`) calls `GetCachedInferredType(value.GetType())`, which caches `TypeConverter.ToClickHouseType` results per `System.Type` — so the inferred type cannot depend on the value at all.
- `TypeConverter.cs:194-195` maps both `ClickHouseDecimal` and `decimal` to a hardcoded `new Decimal128Type { Scale = 9 }`, i.e. `Decimal128(38, 9)`.

Any value whose scale exceeds 9 is therefore silently reduced to 9 fractional digits on write: `DecimalType.WriteScaled` (`Types/DecimalType.cs:124-128`) calls `ClickHouseDecimal.ScaleMantissa(value, 9)`, which is plain integer division (`Numerics/ClickHouseDecimal.cs:414-421`) — **truncation toward zero, no rounding, no error**. `Decimal128` has room for 38 digits, so this is pure data loss with no protocol justification. A `decimal` can carry scale up to 28, and small-magnitude values lose everything: `0.0000000001m` is written as `0`.

Nothing warns the caller — no exception, no `OverflowException`, no log. The existing `Write_Decimal_ShouldPreservePrecision` test in `ClickHouse.Driver.Tests/Types/DynamicTests.cs` uses `123.456789m` (scale 6), which is inside the fixed scale and so never exercises this.

Same root cause as clickhouse-java's Dynamic decimal inference (scale fixed to the width's capacity rather than derived from the value's own scale).

## ClickHouse server version

`26.7.1.1315` (local server at `http://localhost:8123`), verified against a running server.

## Reproduction

```csharp
[Test]
[RequiredFeature(Feature.Dynamic)]
public async Task Write_DecimalWithScaleAbove9_ShouldRoundTrip()
{
var targetTable = "test.scratch_dynamic_decimal";
await connection.ExecuteStatementAsync(
$"CREATE OR REPLACE TABLE {targetTable} (id UInt32, value Dynamic) ENGINE = Memory");

var decimalValue = 0.0123456789012345m; // scale 16, easily fits Decimal128(38, 16)

using var bulkCopy = new ClickHouseBulkCopy(connection) { DestinationTableName = targetTable };
await bulkCopy.WriteToServerAsync([new object[] { 1u, decimalValue }]);

using var reader = await connection.ExecuteReaderAsync($"SELECT value FROM {targetTable}");
ClassicAssert.IsTrue(reader.Read());
var result = (ClickHouseDecimal)reader.GetValue(0);
Assert.That(result, Is.EqualTo(new ClickHouseDecimal(decimalValue)));
}
```

Result (`dotnet test --filter FullyQualifiedName~ScratchDynamicDecimalTests`, both net9.0 and net10.0):

```
Expected: 0.0123456789012345
But was: 0.012345678
```

Server-side check confirms the loss happened on the wire, not on read:

```
server type = Dynamic, server value = 0.012345678
```

Expected: the value round-trips as `0.0123456789012345`. Actual: truncated to 9 fractional digits (note truncation, not rounding — the trailing `9012345` is dropped rather than rounding `...678` up to `...679`).

## Suggested fix

Make the Dynamic write path value-aware for decimals rather than type-aware:

- `ClickHouse.Driver/Types/DynamicType.cs:44-45` — the per-`Type` `InferredTypeCache` is the structural blocker; decimals need a value-dependent branch that derives `Scale` from `ClickHouseDecimal.Scale` (and picks the narrowest width whose precision covers `scale + integer digits`) instead of the cached `Decimal128(38, 9)` from `TypeConverter.cs:194-195`.
- Whatever scale is chosen, it must be >= the value's own scale so `ClickHouseDecimal.ScaleMantissa` never divides. If a value genuinely cannot be represented (scale > 76), throwing beats silently dropping digits.
- Worth extending `Write_Decimal_ShouldPreservePrecision` with high-scale samples (scale 16/20/28) once fixed.

## Link

Related upstream issue: https://github.com/ClickHouse/clickhouse-java/issues/2964

Contributor guide

Open the contributing guide

Research direction

Start with DynamicType.Write in ClickHouse.Driver/Types/DynamicType.cs and the decimal mappings in TypeConverter.cs, then inspect DecimalType.WriteScaled and ClickHouseDecimal.ScaleMantissa. Extend DynamicTests.cs, especially Write_Decimal_ShouldPreservePrecision, with high-scale values and verify they round-trip without truncation. Done means decimal values retain their original precision and unrepresentable values do not lose digits silently.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
databases
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
58/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.