Skip to content

ClickHouseDecimal: NumberOfDigits is one too low at powers of ten, so Truncate() and Convert.ChangeType(..., typeof(BigInteger)) return wrong values #642

Closed

Description

@polyglotAI-bot

Describe the bug

ClickHouseDecimal.NumberOfDigits computes ceil(log10(|value|)). When |value| is an exact power of ten, log10 is an integer and ceil does not add one, so the result is one too low: NumberOfDigits(1) = 0, NumberOfDigits(10) = 1, NumberOfDigits(100) = 2 (the correct counts are 1, 2, 3). For values of 16 or more digits, the double result of BigInteger.Log10 also rounds to an integer for values just above a power of ten. For example, NumberOfDigits(10^20 + 5) = 20, but the value has 21 digits. No value got a count that was too high.

NumberOfDigits is public, and it also drives three other public operations:

  • Truncate(precision) removes one digit too few. When the value is below 1 and its mantissa is a power of ten, the result is a different number. new ClickHouseDecimal(0.1m).Truncate() returns 0.1, but new ClickHouseDecimal(0.5m).Truncate() returns 0. For values in the 10^n + small range, the result keeps one more significant digit than precision allows.
  • IConvertible.ToType(typeof(BigInteger)), and so Convert.ChangeType(value, typeof(BigInteger)), calls the same truncation with precision 0 and returns the remaining mantissa. 0.1, 0.01 and 0.100 convert to 1. The correct result is 0.
  • Division (operator /) sizes its result with NumberOfDigits. When the dividend mantissa is a power of ten, the quotient has one digit more than MaxDivisionPrecision: 1 / 3 has 51 significant digits, 2 / 3 has 50. The class docs say that division "never determines more digits than the given precision".

Floor() is not affected: it calls NumberOfDigits twice and the two errors cancel.

Values read with UseCustomDecimals=true (the default) keep the column scale, so Decimal(P, S) values such as 0.1, 0.01 or 0.100 reach these paths directly.

Steps to reproduce

  1. Open a connection with UseCustomDecimals=true (the default).
  2. Read SELECT toDecimal64('0.1', 1). The value is a ClickHouseDecimal with mantissa 1, scale 1.
  3. Call value.Truncate() and Convert.ChangeType(value, typeof(BigInteger)).

Expected behaviour

Truncate() returns 0, and the conversion to BigInteger returns 0. The server truncates these values to 0 in the same way:

SELECT trunc(toDecimal64('0.1', 1)), trunc(toDecimal64('0.01', 2)), trunc(toDecimal64('0.100', 3)), trunc(toDecimal64('0.5', 1))
-- 0  0  0  0
SELECT toInt128(toDecimal64('0.1', 1)), toInt128(toDecimal64('0.01', 2)), toInt128(toDecimal64('0.100', 3)), toInt128(toDecimal64('0.5', 1))
-- 0  0  0  0
SELECT trunc(toDecimal256('0.0000100000000000000012345', 25))
-- 0
SELECT length(toString(1)), length(toString(10)), length(toString(100)), length(toString(toUInt128('100000000000000000001')))
-- 1  2  3  21

Actual: Truncate() returns 0.1, and the conversion to BigInteger returns 1.

Code example

// Through the read path (UseCustomDecimals=true):
using var cmd = connection.CreateCommand();
cmd.CommandText = "SELECT toDecimal64('0.1', 1)";
var value = (ClickHouseDecimal)cmd.ExecuteScalar();                          // 0.1 (mantissa 1, scale 1)
Console.WriteLine(value.Truncate());                                         // 0.1   (expected 0)
Console.WriteLine(Convert.ChangeType(value, typeof(BigInteger), CultureInfo.InvariantCulture)); // 1 (expected 0)

// No server needed:
ClickHouseDecimal.NumberOfDigits(100);                                       // 2  (expected 3)
ClickHouseDecimal.NumberOfDigits(BigInteger.Pow(10, 20) + 5);                // 20 (expected 21)
new ClickHouseDecimal(BigInteger.Pow(10, 20) + 5, 20).Truncate(20);          // 1.00000000000000000005 (21 significant digits)
((ClickHouseDecimal)1 / 3).Scale;                                            // 51 (2 / 3 gives 50)

Observed results (all on main at 3e2f799, against server 26.9.1.1629):

Input Operation Actual Expected
toDecimal64('0.1', 1) (read) Truncate() 0.1 0
toDecimal64('0.01', 2) (read) Truncate() 0.01 0
toDecimal64('0.100', 3) (read) Truncate() 0.1 0
toDecimal64('0.5', 1) (read) Truncate() 0 0 (correct, mantissa is not a power of ten)
toDecimal64('0.1', 1), ('0.01', 2), ('0.100', 3) (read) Convert.ChangeType(…, typeof(BigInteger)) 1 0
toDecimal256('0.0000100000000000000012345', 25) (read) Truncate() / to BigInteger 0.00001 / 1 0 / 0
1.00000000000000012345 (mantissa 10^20 + 12345, scale 20) Truncate(18) 1.000000000000000123 (19 digits) 1.00000000000000012 (18 digits)
1.00 (mantissa 100, scale 2) Truncate(2) 1.00 1.0 (same value, one trailing zero too many)
1 / 3, 10 / 3, 100 / 7 division 51 significant digits 50 (MaxDivisionPrecision)

A sweep over 10^n - 1, 10^n, 10^n + 1, 2·10^n and 5·10^n - 1 for n = 0..76 gave 139 wrong counts, all too low. In a sample of 152,000 random values (1 to 76 digits), 238 got a count that was too low and none got a count that was too high. All the wrong counts are at exact powers of ten, or (from 16 digits) at values just above one.

Root cause

ClickHouse.Driver/Numerics/ClickHouseDecimal.cs at 3e2f799:

  • Line 123: NumberOfDigits(BigInteger value) => value == 0 ? 0 : (int)Math.Ceiling(BigInteger.Log10(value * value.Sign)). For 10^n, ceil(log10) is n, not n + 1. For large values just above 10^n, BigInteger.Log10 returns exactly n as a double, so the same thing happens.
  • Lines 100-108 (private Truncate): digitsToRemove = min(max(digits - precision, 0), scale). With digits one too low, it removes one digit too few. When the value is below 1, the leading digit stays in the mantissa instead of being removed.
  • Lines 415-423 (ToType, typeof(BigInteger) branch): it calls Truncate(ref mantissa, ref scale, 0) and returns mantissa without checking the remaining scale. The result is correct only when the digit count is correct.
  • Line 244 (operator /): bias = MaxDivisionPrecision - (NumberOfDigits(dividend) - NumberOfDigits(divisor)). A count that is one too low for the dividend gives one more digit.

Suggested fix

Count the digits exactly. Keep Log10 only as an estimate, and correct it with an integer comparison:

public static int NumberOfDigits(BigInteger value)
{
    if (value.IsZero)
        return 0;
    var abs = BigInteger.Abs(value);
    int digits = (int)Math.Floor(BigInteger.Log10(abs)) + 1;
    if (abs >= BigInteger.Pow(10, digits))
        digits++;
    else if (abs < BigInteger.Pow(10, digits - 1))
        digits--;
    return digits;
}

This sketch returned the correct count for all 317,187 values in a check over 1 to 80 digits (values within ±1000 of every power of ten, plus random values, both signs).

Effects of the fix that tests must cover:

  • Truncate() on 0.1 / 0.01 / 0.100 returns 0, and Convert.ChangeType(…, typeof(BigInteger)) returns 0.
  • Division with a power-of-ten dividend returns 50 significant digits, not 51. Its value changes in the last digit only.
  • Keep as-is: Floor() results, Truncate(precision) for mantissas that are not powers of ten (the existing ShouldTruncate cases), and NumberOfDigits(0) = 0.

Error log

No exception. The results are wrong values.

Configuration

Environment

  • Client version: main at 3e2f799 (1.5.0 + 7 commits)
  • Language version: C#
  • .NET version: .NET 10 (SDK 10.0.401), console app on net10.0 that references ClickHouse.Driver
  • OS: Linux (Docker)

ClickHouse server

  • ClickHouse Server version: 26.9.1.1629
  • ClickHouse Server non-default settings, if any: none
  • CREATE TABLE statements for tables involved: none (SELECT of literals only)

Found by automated analysis of the client while working on #640. Verified with a test run against a live ClickHouse server, not by inspection only.

Activity

  1. alex-clickhouse commented on Oct 1, 2026

    @alex-clickhouse
    Collaborator

    @polyglotAI-bot pick this up pls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions