From ac0679670a961a8a06c3fe7cec6d5d972d7adf12 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Tue, 7 Jul 2026 15:51:57 +0200 Subject: [PATCH 1/2] Fix InvalidCastException when summing double/float columns (#46) `Sum`/`SumAsync` over a `double` or `float` column threw `InvalidCastException`. EF Core wraps a top-level aggregate so the empty case returns 0, supplying that fallback as a boxed `Int32` carrying the `Float64`/`Float32` mapping; `GenerateNonNullSqlLiteral` unboxed it with a hard `(double)`/`(float)` cast and threw. Both float literal generators now convert instead of unboxing. The `Float32` read path also converts (GetValue + Convert.ToSingle), since ClickHouse widens `sum(Float32)` to `Float64` and the driver's `GetFloat()` refuses to downcast the returned Double. Audited the rest of the type system: the Double read path is fine (Float64 is the widest float type) and the integer/decimal mappings already handle boxed fallbacks and aggregate widening. Added regression tests for the fixed mappings plus the adjacent decimal/integer aggregate paths. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + RELEASENOTES.md | 1 + .../Mapping/ClickHouseDoubleTypeMapping.cs | 5 +- .../Mapping/ClickHouseFloatTypeMapping.cs | 23 +++++- .../AllTypesQueryTests.cs | 71 +++++++++++++++++++ .../FloatSpecialValueTests.cs | 56 +++++++++++++++ .../TypeMappingLiteralTests.cs | 24 +++++++ 7 files changed, 179 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a13c5dc..b999728 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ v0.3.0 ### Bug fixes * Preserve `LowCardinality(...)` and `Nullable(...)` wrappers from `HasColumnType(...)` in generated migration DDL. Previously the wrapper was stripped during type-mapping resolution, so the migration emitted the inner type. ([#18](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/18)) * Preserve explicit `HasColumnType(...)` text whenever the resolved mapping's canonical store type differs from the user's input — fixes `Enum8(...)` and `AggregateFunction(...)` columns silently emitting `String` in generated DDL. Also covers `Enum16`, `SimpleAggregateFunction`, `Nested`, and the parameter-bearing forms (`Decimal128(S)`, `Json(...)` with type hints, etc.). ([#24](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/24)) +* `Sum`/`SumAsync` over a `double` or `float` column no longer throws `InvalidCastException`. EF Core wraps a top-level aggregate so the empty case returns `0`, supplying that fallback as a boxed `Int32` carrying the `Float64`/`Float32` mapping; the literal generators now convert rather than unbox. The `Float32` read path also converts, since ClickHouse widens `sum(Float32)` to `Float64` (which the driver's `GetFloat()` refuses to downcast). ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) v0.2.0 --- diff --git a/RELEASENOTES.md b/RELEASENOTES.md index bfcb146..e131a34 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -18,6 +18,7 @@ v0.3.0 ### Bug fixes * `HasColumnType("Enum8(...)")`, `HasColumnType("AggregateFunction(...)")`, and similar parameterized or aliased store types are now preserved verbatim in generated migration DDL. Previously these silently emitted `String` because the resolver canonicalized to a generic fallback mapping. ([#24](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/24)) (Thanks to @Felixzed!) +* Summing a `double` or `float` column (`.SumAsync(x => x.Value)`) no longer throws `InvalidCastException`. Two ClickHouse-specific mismatches were biting: EF Core hands the float literal generator a boxed `Int32` `0` as the empty-result fallback, and ClickHouse widens `sum(Float32)` to `Float64` so the driver couldn't read it back as a `float`. Both the literal generation and the `Float32` read path now convert instead of hard-casting. ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) (Thanks to @HotTotem!) v0.2.0 --- diff --git a/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseDoubleTypeMapping.cs b/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseDoubleTypeMapping.cs index 51133a0..c9b0412 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseDoubleTypeMapping.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseDoubleTypeMapping.cs @@ -20,7 +20,10 @@ protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters p protected override string GenerateNonNullSqlLiteral(object value) { - var d = (double)value; + // EF Core can hand us a boxed value whose runtime type differs from double + // (e.g. an Int32 0 from SUM's COALESCE(SUM(x), 0) rewrite), so convert + // rather than unbox to avoid an InvalidCastException. + var d = Convert.ToDouble(value, CultureInfo.InvariantCulture); return d switch { double.NaN => "CAST('NaN' AS Float64)", diff --git a/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseFloatTypeMapping.cs b/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseFloatTypeMapping.cs index 39fa669..08bbf98 100644 --- a/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseFloatTypeMapping.cs +++ b/src/EFCore.ClickHouse/Storage/Internal/Mapping/ClickHouseFloatTypeMapping.cs @@ -1,10 +1,19 @@ +using System.Data.Common; using System.Globalization; +using System.Linq.Expressions; +using System.Reflection; using Microsoft.EntityFrameworkCore.Storage; namespace ClickHouse.EntityFrameworkCore.Storage.Internal.Mapping; public class ClickHouseFloatTypeMapping : RelationalTypeMapping { + private static readonly MethodInfo GetValueMethod = + typeof(DbDataReader).GetRuntimeMethod(nameof(DbDataReader.GetValue), [typeof(int)])!; + + private static readonly MethodInfo ConvertToSingleMethod = + typeof(Convert).GetMethod(nameof(Convert.ToSingle), [typeof(object)])!; + public ClickHouseFloatTypeMapping() : base("Float32", typeof(float), System.Data.DbType.Single) { @@ -18,9 +27,21 @@ protected ClickHouseFloatTypeMapping(RelationalTypeMappingParameters parameters) protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters) => new ClickHouseFloatTypeMapping(parameters); + // ClickHouse widens Float32 aggregates to Float64 (e.g. sum(Float32) returns + // Float64), so the driver's GetFloat() would throw when downcasting. Read the + // raw value and convert instead. + public override MethodInfo GetDataReaderMethod() + => GetValueMethod; + + public override Expression CustomizeDataReaderExpression(Expression expression) + => Expression.Call(ConvertToSingleMethod, expression); + protected override string GenerateNonNullSqlLiteral(object value) { - var f = (float)value; + // EF Core can hand us a boxed value whose runtime type differs from float + // (e.g. an Int32 0 from SUM's COALESCE(SUM(x), 0) rewrite), so convert + // rather than unbox to avoid an InvalidCastException. + var f = Convert.ToSingle(value, CultureInfo.InvariantCulture); return f switch { float.NaN => "CAST('NaN' AS Float32)", diff --git a/test/EFCore.ClickHouse.Tests/AllTypesQueryTests.cs b/test/EFCore.ClickHouse.Tests/AllTypesQueryTests.cs index c614b5d..04cccbd 100644 --- a/test/EFCore.ClickHouse.Tests/AllTypesQueryTests.cs +++ b/test/EFCore.ClickHouse.Tests/AllTypesQueryTests.cs @@ -361,6 +361,77 @@ public async Task Count_WithDecimalPredicate() Assert.Equal(3, count); // rows 3, 4, 5 have positive decimals } + // The following aggregate tests guard against the #46 class of bug — EF Core + // wraps a top-level Sum so the empty case returns 0, handing the column's type + // mapping a boxed Int32 fallback. #46 was Float64/Float32; these cover the + // adjacent decimal and integer paths and the ClickHouse aggregate-widening + // read path (sum(Decimal(18,4)) -> Decimal128, sum(Int32) -> Int64). + + [Fact] + public async Task Sum_OverDecimal_ReturnsCorrectTotal() + { + await using var ctx = new AllTypesDbContext(_fixture.ConnectionString); + + // Rows 4,5: 0.0001 + 42.0000 = 42.0001 + var total = await ctx.AllTypes + .Where(e => e.Id >= 4) + .SumAsync(e => e.ValDecimal); + + Assert.Equal(42.0001m, total); + } + + [Fact] + public async Task Sum_OverDecimal_NoMatchingRows_ReturnsZero() + { + await using var ctx = new AllTypesDbContext(_fixture.ConnectionString); + + var total = await ctx.AllTypes + .Where(e => e.Id > 1000) + .SumAsync(e => e.ValDecimal); + + Assert.Equal(0m, total); + } + + [Fact] + public async Task Sum_OverInt32_WidensToInt64AndReads() + { + await using var ctx = new AllTypesDbContext(_fixture.ConnectionString); + + // ClickHouse widens sum(Int32) to Int64; the integer mapping reads it back. + // Rows 4,5: -1 + 42 = 41 + var total = await ctx.AllTypes + .Where(e => e.Id >= 4) + .SumAsync(e => e.ValInt32); + + Assert.Equal(41, total); + } + + [Fact] + public async Task Sum_OverInt64_NoMatchingRows_ReturnsZero() + { + await using var ctx = new AllTypesDbContext(_fixture.ConnectionString); + + var total = await ctx.AllTypes + .Where(e => e.Id > 1000) + .SumAsync(e => e.ValInt64); + + Assert.Equal(0L, total); + } + + [Fact] + public async Task Average_OverInt32_ReturnsDouble() + { + await using var ctx = new AllTypesDbContext(_fixture.ConnectionString); + + // ClickHouse avg over any numeric type returns Float64, materialized as double. + // Rows 4,5: (-1 + 42) / 2 = 20.5 + var avg = await ctx.AllTypes + .Where(e => e.Id >= 4) + .AverageAsync(e => e.ValInt32); + + Assert.Equal(20.5, avg, 1e-10); + } + [Fact] public async Task Where_EmptyString() { diff --git a/test/EFCore.ClickHouse.Tests/FloatSpecialValueTests.cs b/test/EFCore.ClickHouse.Tests/FloatSpecialValueTests.cs index 3bfe284..cd96daa 100644 --- a/test/EFCore.ClickHouse.Tests/FloatSpecialValueTests.cs +++ b/test/EFCore.ClickHouse.Tests/FloatSpecialValueTests.cs @@ -214,6 +214,62 @@ public async Task Comparison_NegativeInfinityIsLessThanAll() Assert.True(double.IsNegativeInfinity(results[0].ValFloat64)); } + [Fact] + public async Task Sum_OverFloat64_ReturnsCorrectTotal() + { + // Regression test for #46: a top-level Sum over a non-nullable Float64 + // column is translated as sum(...) wrapped so the empty case returns 0. + // EF Core supplies that fallback 0 as a boxed Int32 carrying the Float64 + // mapping, which previously threw InvalidCastException in + // ClickHouseDoubleTypeMapping.GenerateNonNullSqlLiteral. + await using var ctx = new FloatSpecialDbContext(_fixture.ConnectionString); + + // Rows 4,5,6 are finite: 0.0 + 2.718281828459045 + (-1.5) = 1.218281828459045 + var total = await ctx.FloatSpecials + .Where(e => e.Id >= 4) + .SumAsync(e => e.ValFloat64); + + Assert.Equal(1.218281828459045, total, 1e-10); + } + + [Fact] + public async Task Sum_OverFloat64_NoMatchingRows_ReturnsZero() + { + // The exact shape reported in #46: a predicate that matches no rows. + await using var ctx = new FloatSpecialDbContext(_fixture.ConnectionString); + + var total = await ctx.FloatSpecials + .Where(e => e.Id > 1000) + .SumAsync(e => e.ValFloat64); + + Assert.Equal(0.0, total); + } + + [Fact] + public async Task Sum_OverFloat32_ReturnsCorrectTotal() + { + await using var ctx = new FloatSpecialDbContext(_fixture.ConnectionString); + + // Rows 4,5,6 are finite: 0.0 + 3.14 + (-1.5) = 1.64 + var total = await ctx.FloatSpecials + .Where(e => e.Id >= 4) + .SumAsync(e => e.ValFloat32); + + Assert.Equal(1.64f, total, 0.001f); + } + + [Fact] + public async Task Sum_OverFloat32_NoMatchingRows_ReturnsZero() + { + await using var ctx = new FloatSpecialDbContext(_fixture.ConnectionString); + + var total = await ctx.FloatSpecials + .Where(e => e.Id > 1000) + .SumAsync(e => e.ValFloat32); + + Assert.Equal(0.0f, total); + } + [Fact] public async Task OrderBy_SpecialValuesSort() { diff --git a/test/EFCore.ClickHouse.Tests/TypeMappingLiteralTests.cs b/test/EFCore.ClickHouse.Tests/TypeMappingLiteralTests.cs index 70cc3ea..7778947 100644 --- a/test/EFCore.ClickHouse.Tests/TypeMappingLiteralTests.cs +++ b/test/EFCore.ClickHouse.Tests/TypeMappingLiteralTests.cs @@ -66,6 +66,18 @@ public void Float_Null_GeneratesNullLiteral() Assert.Equal("NULL", literal); } + [Fact] + public void Float_BoxedInt32_GeneratesNumericLiteral() + { + // Regression test for #46: EF Core rewrites SUM as COALESCE(SUM(x), 0) + // and hands the Float32 mapping a boxed Int32 zero. Unboxing directly + // to float would throw InvalidCastException. + var mapping = new ClickHouseFloatTypeMapping(); + var literal = mapping.GenerateSqlLiteral((object)0); + Assert.Equal("0", literal); + Assert.DoesNotContain("CAST", literal); + } + // --- Float64 (ClickHouseDoubleTypeMapping) --- [Fact] @@ -117,6 +129,18 @@ public void Double_Null_GeneratesNullLiteral() Assert.Equal("NULL", literal); } + [Fact] + public void Double_BoxedInt32_GeneratesNumericLiteral() + { + // Regression test for #46: EF Core rewrites SUM as COALESCE(SUM(x), 0) + // and hands the Float64 mapping a boxed Int32 zero. Unboxing directly + // to double would throw InvalidCastException. + var mapping = new ClickHouseDoubleTypeMapping(); + var literal = mapping.GenerateSqlLiteral((object)0); + Assert.Equal("0", literal); + Assert.DoesNotContain("CAST", literal); + } + // --- BigInteger (ClickHouseBigIntegerTypeMapping) --- [Fact] From 8bc085cec6aabc98568d0145b4499e84622b3866 Mon Sep 17 00:00:00 2001 From: Alex Soffronow-Pagonidis Date: Tue, 7 Jul 2026 16:50:05 +0200 Subject: [PATCH 2/2] Move #46 changelog entry to new v0.3.1 (Unreleased) section v0.3.0 is already released, so the bug-fix entry belongs under the next version rather than the shipped one. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 6 +++++- RELEASENOTES.md | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b999728..dff11e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,8 @@ +v0.3.1 (Unreleased) +--- +### Bug fixes +* `Sum`/`SumAsync` over a `double` or `float` column no longer throws `InvalidCastException`. EF Core wraps a top-level aggregate so the empty case returns `0`, supplying that fallback as a boxed `Int32` carrying the `Float64`/`Float32` mapping; the literal generators now convert rather than unbox. The `Float32` read path also converts, since ClickHouse widens `sum(Float32)` to `Float64` (which the driver's `GetFloat()` refuses to downcast). ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) + v0.3.0 --- ### Advanced queries @@ -19,7 +24,6 @@ v0.3.0 ### Bug fixes * Preserve `LowCardinality(...)` and `Nullable(...)` wrappers from `HasColumnType(...)` in generated migration DDL. Previously the wrapper was stripped during type-mapping resolution, so the migration emitted the inner type. ([#18](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/18)) * Preserve explicit `HasColumnType(...)` text whenever the resolved mapping's canonical store type differs from the user's input — fixes `Enum8(...)` and `AggregateFunction(...)` columns silently emitting `String` in generated DDL. Also covers `Enum16`, `SimpleAggregateFunction`, `Nested`, and the parameter-bearing forms (`Decimal128(S)`, `Json(...)` with type hints, etc.). ([#24](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/24)) -* `Sum`/`SumAsync` over a `double` or `float` column no longer throws `InvalidCastException`. EF Core wraps a top-level aggregate so the empty case returns `0`, supplying that fallback as a boxed `Int32` carrying the `Float64`/`Float32` mapping; the literal generators now convert rather than unbox. The `Float32` read path also converts, since ClickHouse widens `sum(Float32)` to `Float64` (which the driver's `GetFloat()` refuses to downcast). ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) v0.2.0 --- diff --git a/RELEASENOTES.md b/RELEASENOTES.md index e131a34..dd56054 100644 --- a/RELEASENOTES.md +++ b/RELEASENOTES.md @@ -1,3 +1,8 @@ +v0.3.1 (Unreleased) +--- +### Bug fixes +* Summing a `double` or `float` column (`.SumAsync(x => x.Value)`) no longer throws `InvalidCastException`. Two ClickHouse-specific mismatches were biting: EF Core hands the float literal generator a boxed `Int32` `0` as the empty-result fallback, and ClickHouse widens `sum(Float32)` to `Float64` so the driver couldn't read it back as a `float`. Both the literal generation and the `Float32` read path now convert instead of hard-casting. ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) (Thanks to @HotTotem!) + v0.3.0 --- ### Advanced queries @@ -18,7 +23,6 @@ v0.3.0 ### Bug fixes * `HasColumnType("Enum8(...)")`, `HasColumnType("AggregateFunction(...)")`, and similar parameterized or aliased store types are now preserved verbatim in generated migration DDL. Previously these silently emitted `String` because the resolver canonicalized to a generic fallback mapping. ([#24](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/24)) (Thanks to @Felixzed!) -* Summing a `double` or `float` column (`.SumAsync(x => x.Value)`) no longer throws `InvalidCastException`. Two ClickHouse-specific mismatches were biting: EF Core hands the float literal generator a boxed `Int32` `0` as the empty-result fallback, and ClickHouse widens `sum(Float32)` to `Float64` so the driver couldn't read it back as a `float`. Both the literal generation and the `Float32` read path now convert instead of hard-casting. ([#46](https://github.com/ClickHouse/ClickHouse.EntityFrameworkCore/issues/46)) (Thanks to @HotTotem!) v0.2.0 ---