From 7b27fafa2d16b4d555eb9d3ea619edec60be9967 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Thu, 1 Oct 2026 15:50:03 +0000 Subject: [PATCH 1/2] Fix ClickHouseDecimal: IConvertible narrowing and ChangeType recursion ToInt32 cut the value to 16 bits, so InsertBinaryAsync stored a wrong value in an Int32 column. ToChar/ToSByte/ToByte/ToInt16/ToUInt16 wrapped an out-of-range value instead of throwing OverflowException. ToType fell back to Convert.ChangeType, which calls ToType again for every type it does not convert itself, so an unsupported target type overflowed the stack. ToType now maps the IConvertible types to the matching members and throws InvalidCastException for the others, as decimal does. Fixes: https://github.com/ClickHouse/clickhouse-cs/issues/646 Co-Authored-By: Claude Opus 5.5 --- .../ClickHouseDecimalIntegerColumnTests.cs | 55 +++++++++ .../Numerics/ClickHouseDecimalTests.cs | 109 ++++++++++++++++++ .../Numerics/ClickHouseDecimal.cs | 42 +++++-- ...46-clickhousedecimal-iconvertible.fixes.md | 6 + 4 files changed, 205 insertions(+), 7 deletions(-) create mode 100644 ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalIntegerColumnTests.cs create mode 100644 changelog.d/646-clickhousedecimal-iconvertible.fixes.md diff --git a/ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalIntegerColumnTests.cs b/ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalIntegerColumnTests.cs new file mode 100644 index 000000000..1c348f64b --- /dev/null +++ b/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; + +/// +/// Binary inserts of values into integer columns. The integer types +/// convert the value through its members. +/// +[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 + { + 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(); + 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(async () => + await client.InsertBinaryAsync(table, new[] { "v" }, new[] { new object[] { new ClickHouseDecimal(value) } })); + + Assert.That(ex.InnerException, Is.TypeOf()); + } +} diff --git a/ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalTests.cs b/ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalTests.cs index 7615cabb7..a1af31377 100644 --- a/ClickHouse.Driver.Tests/Numerics/ClickHouseDecimalTests.cs +++ b/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 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 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(() => 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(() => Convert.ChangeType(@decimal, type, CultureInfo.InvariantCulture)); + } + + [Test] + public void ToType_NullType_ThrowsArgumentNullException() + { + var @decimal = new ClickHouseDecimal(5m); + Assert.Throws(() => @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) diff --git a/ClickHouse.Driver/Numerics/ClickHouseDecimal.cs b/ClickHouse.Driver/Numerics/ClickHouseDecimal.cs index d7e91a7f4..916552f79 100644 --- a/ClickHouse.Driver/Numerics/ClickHouseDecimal.cs +++ b/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,6 +414,8 @@ 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; @@ -421,7 +423,33 @@ public object ToType(Type conversionType, IFormatProvider provider) 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); diff --git a/changelog.d/646-clickhousedecimal-iconvertible.fixes.md b/changelog.d/646-clickhousedecimal-iconvertible.fixes.md new file mode 100644 index 000000000..44552b924 --- /dev/null +++ b/changelog.d/646-clickhousedecimal-iconvertible.fixes.md @@ -0,0 +1,6 @@ +* Fixed `ClickHouseDecimal` conversions through `Convert` ([#646](https://github.com/ClickHouse/clickhouse-cs/issues/646)). + `Convert.ToInt32` kept only the low 16 bits, so `InsertBinaryAsync` stored a wrong value in an + `Int32` column; conversions to `sbyte`, `byte`, `short`, `ushort` and `char` wrapped instead of + throwing `OverflowException` when the integer part is out of range; and `Convert.ChangeType` to + a type such as `decimal?`, `Guid` or an enum crashed the process with a stack overflow instead of + throwing `InvalidCastException`. From a178647ab8fafbe4c7d3aee6b774cf73cb07fc56 Mon Sep 17 00:00:00 2001 From: Polyglot AI <293096396+polyglotAI-bot@users.noreply.github.com> Date: Thu, 1 Oct 2026 16:02:19 +0000 Subject: [PATCH 2/2] Document ClickHouseDecimal integer conversions; shorten changelog fragment Add a note to the integer write section of docs/http.mdx: writing a ClickHouseDecimal drops the fractional part, an out-of-range integer part throws OverflowException, and Convert.ChangeType to an unsupported type throws InvalidCastException. Condense the #646 fragment to the user-visible outcomes. Co-Authored-By: Claude Opus 5.5 --- .../646-clickhousedecimal-iconvertible.fixes.md | 10 ++++------ docs/http.mdx | 6 ++++++ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/changelog.d/646-clickhousedecimal-iconvertible.fixes.md b/changelog.d/646-clickhousedecimal-iconvertible.fixes.md index 44552b924..cb8745e7c 100644 --- a/changelog.d/646-clickhousedecimal-iconvertible.fixes.md +++ b/changelog.d/646-clickhousedecimal-iconvertible.fixes.md @@ -1,6 +1,4 @@ -* Fixed `ClickHouseDecimal` conversions through `Convert` ([#646](https://github.com/ClickHouse/clickhouse-cs/issues/646)). - `Convert.ToInt32` kept only the low 16 bits, so `InsertBinaryAsync` stored a wrong value in an - `Int32` column; conversions to `sbyte`, `byte`, `short`, `ushort` and `char` wrapped instead of - throwing `OverflowException` when the integer part is out of range; and `Convert.ChangeType` to - a type such as `decimal?`, `Guid` or an enum crashed the process with a stack overflow instead of - throwing `InvalidCastException`. +* 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. diff --git a/docs/http.mdx b/docs/http.mdx index dac5be7c7..3b517846c 100644 --- a/docs/http.mdx +++ b/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 | | + +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`. + + --- #### Floating point types {#type-map-writing-floating-point}