Fix ClickHouseDecimal: IConvertible narrowing and ChangeType stack overflow

ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalIntegerColumnTests.cs

@@ -0,0 +1,55 @@
+using System;
+using System.Collections.Generic;
+using System.Threading.Tasks;
+using ClickHouse.Driver.Copy;
+using ClickHouse.Driver.Numerics;
+using NUnit.Framework;
+
+namespace ClickHouse.Driver.Tests.Numerics;
+
+/// <summary>
+/// Binary inserts of <see cref="ClickHouseDecimal"/> values into integer columns. The integer types
+/// convert the value through its <see cref="IConvertible"/> members.
+/// </summary>
+[TestFixture]
+[Category("ClickHouseDecimal")]
+public class ClickHouseDecimalIntegerColumnTests : AbstractConnectionTestFixture
+{
+    [Test]
+    public async Task InsertBinaryAsync_ClickHouseDecimalIntoInt32Column_StoresExactValue()
+    {
+        var table = CreateTableName();
+        await client.ExecuteNonQueryAsync($"CREATE TABLE {table} (id UInt8, v Int32) ENGINE = Memory");
+
+        await client.InsertBinaryAsync(table, new[] { "id", "v" }, new List<object[]>
+        {
+            new object[] { (byte)1, new ClickHouseDecimal(100000.00m) },
+            new object[] { (byte)2, new ClickHouseDecimal(-40000m) },
+            new object[] { (byte)3, new ClickHouseDecimal(int.MinValue) },
+            new object[] { (byte)4, new ClickHouseDecimal(int.MaxValue) },
+        });
+
+        using var reader = await client.ExecuteReaderAsync($"SELECT v FROM {table} ORDER BY id");
+        var stored = new List<int>();
+        while (reader.Read())
+            stored.Add(reader.GetInt32(0));
+
+        Assert.That(stored, Is.EqualTo(new[] { 100000, -40000, int.MinValue, int.MaxValue }));
+    }
+
+    [Test]
+    [TestCase("Int8", 128)]
+    [TestCase("UInt8", -1)]
+    [TestCase("Int16", 40000)]
+    [TestCase("UInt16", 70000)]
+    public async Task InsertBinaryAsync_ClickHouseDecimalOutOfColumnRange_ThrowsOverflowException(string columnType, int value)
+    {
+        var table = CreateTableName($"decimal_overflow_{columnType}");
+        await client.ExecuteNonQueryAsync($"CREATE TABLE {table} (v {columnType}) ENGINE = Memory");
+
+        var ex = Assert.ThrowsAsync<ClickHouseBulkCopySerializationException>(async () =>
+            await client.InsertBinaryAsync(table, new[] { "v" }, new[] { new object[] { new ClickHouseDecimal(value) } }));
+
+        Assert.That(ex.InnerException, Is.TypeOf<OverflowException>());
+    }
+}

ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalTests.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
 using System.Numerics;
@@ -254,6 +255,114 @@ public void ShouldConvertToBigInteger()
         Assert.That(actual, Is.EqualTo(expected));
     }
 
+    // The fractional part is truncated toward zero, then the integer part must fit the target type,
+    // as the server does: toUInt8(toDecimal32(255.9, 1)) is 255, toUInt8(toDecimal32(256.5, 1)) overflows.
+    public static IEnumerable<TestCaseData> NarrowIntegralInRangeCases()
+    {
+        yield return new TestCaseData(new ClickHouseDecimal(-128m), sbyte.MinValue);
+        yield return new TestCaseData(new ClickHouseDecimal(127m), sbyte.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(-128.9m), sbyte.MinValue);
+        yield return new TestCaseData(new ClickHouseDecimal(0m), byte.MinValue);
+        yield return new TestCaseData(new ClickHouseDecimal(255m), byte.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(255.9m), byte.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(-0.5m), byte.MinValue);
+        yield return new TestCaseData(new ClickHouseDecimal(5 * BigInteger.Pow(10, 70), 70), (byte)5);
+        yield return new TestCaseData(new ClickHouseDecimal(-32768m), short.MinValue);
+        yield return new TestCaseData(new ClickHouseDecimal(32767m), short.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(65535m), ushort.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(65535m), char.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(-2147483648m), int.MinValue);
+        yield return new TestCaseData(new ClickHouseDecimal(2147483647m), int.MaxValue);
+        yield return new TestCaseData(new ClickHouseDecimal(100000.00m), 100000);
+    }
+
+    [Test]
+    [TestCaseSource(nameof(NarrowIntegralInRangeCases))]
+    public void ChangeType_NarrowIntegralTypeInRange_ReturnsValue(ClickHouseDecimal value, object expected)
+    {
+        var actual = Convert.ChangeType(value, expected.GetType(), CultureInfo.InvariantCulture);
+        Assert.That(actual, Is.EqualTo(expected).And.TypeOf(expected.GetType()));
+    }
+
+    public static IEnumerable<TestCaseData> NarrowIntegralOutOfRangeCases()
+    {
+        yield return new TestCaseData(new ClickHouseDecimal(-129m), typeof(sbyte));
+        yield return new TestCaseData(new ClickHouseDecimal(128m), typeof(sbyte));
+        yield return new TestCaseData(new ClickHouseDecimal(-1m), typeof(byte));
+        yield return new TestCaseData(new ClickHouseDecimal(256m), typeof(byte));
+        yield return new TestCaseData(new ClickHouseDecimal(256.5m), typeof(byte));
+        yield return new TestCaseData(new ClickHouseDecimal(-32769m), typeof(short));
+        yield return new TestCaseData(new ClickHouseDecimal(32768m), typeof(short));
+        yield return new TestCaseData(new ClickHouseDecimal(-1m), typeof(ushort));
+        yield return new TestCaseData(new ClickHouseDecimal(-1.5m), typeof(ushort));
+        yield return new TestCaseData(new ClickHouseDecimal(65536m), typeof(ushort));
+        yield return new TestCaseData(new ClickHouseDecimal(-1m), typeof(char));
+        yield return new TestCaseData(new ClickHouseDecimal(65536m), typeof(char));
+        yield return new TestCaseData(new ClickHouseDecimal(-2147483649m), typeof(int));
+        yield return new TestCaseData(new ClickHouseDecimal(2147483648m), typeof(int));
+        yield return new TestCaseData(new ClickHouseDecimal(BigInteger.Pow(10, 70), 0), typeof(int));
+    }
+
+    [Test]
+    [TestCaseSource(nameof(NarrowIntegralOutOfRangeCases))]
+    public void ChangeType_NarrowIntegralTypeOutOfRange_ThrowsOverflowException(ClickHouseDecimal value, Type type)
+    {
+        Assert.Throws<OverflowException>(() => Convert.ChangeType(value, type, CultureInfo.InvariantCulture));
+    }
+
+    [Test]
+    [TestCase(typeof(decimal?))]
+    [TestCase(typeof(int?))]
+    [TestCase(typeof(Guid))]
+    [TestCase(typeof(DateTimeOffset))]
+    [TestCase(typeof(TimeSpan))]
+    [TestCase(typeof(DayOfWeek))]
+    [TestCase(typeof(IntPtr))]
+    public void ChangeType_TypeWithoutConversion_ThrowsInvalidCastException(Type type)
+    {
+        var @decimal = new ClickHouseDecimal(5m);
+        Assert.Throws<InvalidCastException>(() => Convert.ChangeType(@decimal, type, CultureInfo.InvariantCulture));
+    }
+
+    [Test]
+    public void ToType_NullType_ThrowsArgumentNullException()
+    {
+        var @decimal = new ClickHouseDecimal(5m);
+        Assert.Throws<ArgumentNullException>(() => @decimal.ToType(null, CultureInfo.InvariantCulture));
+    }
+
+    [Test]
+    [TestCase(typeof(ClickHouseDecimal))]
+    [TestCase(typeof(object))]
+    public void ToType_OwnTypeOrObject_ReturnsSameValue(Type type)
+    {
+        var @decimal = new ClickHouseDecimal(123.45m);
+        Assert.That(@decimal.ToType(type, CultureInfo.InvariantCulture), Is.EqualTo(@decimal));
+    }
+
+    [Test]
+    [TestCase(typeof(bool))]
+    [TestCase(typeof(char))]
+    [TestCase(typeof(byte))]
+    [TestCase(typeof(sbyte))]
+    [TestCase(typeof(short))]
+    [TestCase(typeof(ushort))]
+    [TestCase(typeof(int))]
+    [TestCase(typeof(uint))]
+    [TestCase(typeof(long))]
+    [TestCase(typeof(ulong))]
+    [TestCase(typeof(float))]
+    [TestCase(typeof(double))]
+    [TestCase(typeof(decimal))]
+    [TestCase(typeof(string))]
+    public void ToType_ConvertibleType_MatchesChangeType(Type type)
+    {
+        var @decimal = new ClickHouseDecimal(5.00m);
+        var expected = Convert.ChangeType(@decimal, type, CultureInfo.InvariantCulture);
+        var actual = @decimal.ToType(type, CultureInfo.InvariantCulture);
+        Assert.That(actual, Is.EqualTo(expected).And.TypeOf(type));
+    }
+
     [Test]
     [RequiredFeature(Feature.WideTypes)]
     public async Task ValuesFromClickHouseShouldMatch([ValueSource(typeof(ClickHouseDecimalTests), nameof(DecimalsWithExtremeValues))] decimal value)

ClickHouse.Driver/Numerics/ClickHouseDecimal.cs

@@ -386,17 +386,17 @@ public static ClickHouseDecimal Parse(string input, IFormatProvider provider)
 
     public bool ToBoolean(IFormatProvider provider) => !Mantissa.IsZero;
 
-    public char ToChar(IFormatProvider provider) => (char)(int)this;
+    public char ToChar(IFormatProvider provider) => checked((char)(int)this);
 
-    public sbyte ToSByte(IFormatProvider provider) => (sbyte)(int)this;
+    public sbyte ToSByte(IFormatProvider provider) => checked((sbyte)(int)this);
 
-    public byte ToByte(IFormatProvider provider) => (byte)(int)this;
+    public byte ToByte(IFormatProvider provider) => checked((byte)(int)this);
 
-    public short ToInt16(IFormatProvider provider) => (short)(int)this;
+    public short ToInt16(IFormatProvider provider) => checked((short)(int)this);
 
-    public ushort ToUInt16(IFormatProvider provider) => (ushort)(uint)this;
+    public ushort ToUInt16(IFormatProvider provider) => checked((ushort)(uint)this);
 
-    public int ToInt32(IFormatProvider provider) => (short)(int)this;
+    public int ToInt32(IFormatProvider provider) => (int)this;
 
     public uint ToUInt32(IFormatProvider provider) => (uint)this;
 
@@ -414,14 +414,42 @@ public static ClickHouseDecimal Parse(string input, IFormatProvider provider)
 
     public object ToType(Type conversionType, IFormatProvider provider)
     {
+        ArgumentNullException.ThrowIfNull(conversionType);
+
         if (conversionType == typeof(BigInteger))
         {
             var mantissa = this.Mantissa;
             var scale = this.Scale;
             Truncate(ref mantissa, ref scale, 0);
             return mantissa;
         }
-        return Convert.ChangeType(this, conversionType, provider);
+
+        if (conversionType == typeof(ClickHouseDecimal) || conversionType == typeof(object))
+            return this;
+
+        // Convert.ChangeType calls ToType for every type it does not convert itself, so calling back
+        // into it from here recurses without end. An enum reports the TypeCode of its underlying type,
+        // but Convert has no conversion to it (the same as for decimal).
+        var typeCode = conversionType.IsEnum ? TypeCode.Object : Type.GetTypeCode(conversionType);
+        return typeCode switch
+        {
+            TypeCode.Boolean => ToBoolean(provider),
+            TypeCode.Char => ToChar(provider),
+            TypeCode.SByte => ToSByte(provider),
+            TypeCode.Byte => ToByte(provider),
+            TypeCode.Int16 => ToInt16(provider),
+            TypeCode.UInt16 => ToUInt16(provider),
+            TypeCode.Int32 => ToInt32(provider),
+            TypeCode.UInt32 => ToUInt32(provider),
+            TypeCode.Int64 => ToInt64(provider),
+            TypeCode.UInt64 => ToUInt64(provider),
+            TypeCode.Single => ToSingle(provider),
+            TypeCode.Double => ToDouble(provider),
+            TypeCode.Decimal => ToDecimal(provider),
+            TypeCode.DateTime => ToDateTime(provider),
+            TypeCode.String => ToString(provider),
+            _ => throw new InvalidCastException($"Invalid cast from '{typeof(ClickHouseDecimal).FullName}' to '{conversionType.FullName}'."),
+        };
     }
 
     public int CompareTo(decimal other) => CompareTo((ClickHouseDecimal)other);

changelog.d/646-clickhousedecimal-iconvertible.fixes.md

@@ -0,0 +1,4 @@
+* Fixed `Convert` conversions of `ClickHouseDecimal` ([#646](https://github.com/ClickHouse/clickhouse-cs/issues/646)).
+  `InsertBinaryAsync` now stores the correct value in an `Int32` column, conversions to smaller
+  integer types throw `OverflowException` instead of wrapping, and `Convert.ChangeType` to an
+  unsupported type throws `InvalidCastException` instead of crashing the process.

docs/http.mdx

@@ -1890,6 +1890,12 @@ When inserting data, the driver converts .NET types to their corresponding Click
 | Int256 | `BigInteger`, `decimal`, `double`, `float`, `int`, `uint`, `long`, `ulong`, any `Convert.ToInt64()` compatible | |
 | UInt256 | `BigInteger`, `decimal`, `double`, `float`, `int`, `uint`, `long`, `ulong`, any `Convert.ToInt64()` compatible | |
 
+<Note>
+When you write a `ClickHouseDecimal` to an `Int8`, `UInt8`, `Int16`, `UInt16`, `Int32`, `UInt32`, `Int64` or `UInt64` column, the driver drops the fractional part (it rounds toward zero). If the integer part is out of the range of the column type, the conversion throws `OverflowException`: `255.9` is stored as `255` in a `UInt8` column, and `256.5` is not written. `InsertBinaryAsync` reports this as a `ClickHouseBulkCopySerializationException` with the `OverflowException` as its `InnerException`.
+
+`Convert.ToSByte` through `Convert.ToUInt64` and `Convert.ChangeType` on a `ClickHouseDecimal` use the same rules. `Convert.ChangeType` to a type that has no conversion from `ClickHouseDecimal`, such as `Guid`, `TimeSpan`, an enum or a nullable type, throws `InvalidCastException`.
+</Note>
+
 ---
 
 #### Floating point types {#type-map-writing-floating-point}