Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
5 changes: 5 additions & 0 deletions RELEASENOTES.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
Expand Down
Original file line number Diff line number Diff line change
@@ -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)
{
Expand All @@ -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)",
Expand Down
71 changes: 71 additions & 0 deletions test/EFCore.ClickHouse.Tests/AllTypesQueryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
56 changes: 56 additions & 0 deletions test/EFCore.ClickHouse.Tests/FloatSpecialValueTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
24 changes: 24 additions & 0 deletions test/EFCore.ClickHouse.Tests/TypeMappingLiteralTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down
Loading