dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.68k stars 3.16k forks source link

Error generating query with nullable decimal sum #34753

Open matheus-inacio opened 3 days ago

matheus-inacio commented 3 days ago

File a bug

When handling nullability with Sum from LINQ, EfCore surrounds the SUM SQL command with COALESCE (for relational databases).

The problem is that EfCore always define the 'right' parameter for the coalesce as Int (even if the sum is resulting with decimal places).

This can cause errors generating the SQL query when you have converters, because efcore tries to pass a int to a non-int converter.

image image

Decimal converter

image

StackTrace

fail: Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddleware[1]
      An unhandled exception has occurred while executing the request.
      System.InvalidCastException: Unable to cast object of type 'System.Int32' to type 'System.Nullable`1[System.Decimal]'.
         at Microsoft.EntityFrameworkCore.Storage.ValueConversion.ValueConverter`2.<>c__DisplayClass6_0`2.<SanitizeConverter>b__0(Object v)
         at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapping.GenerateSqlLiteral(Object value)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlConstant(SqlConstantExpression sqlConstantExpression)
         at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerQuerySqlGenerator.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.<VisitSqlFunction>b__26_0(SqlExpression e)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.GenerateList[T](IReadOnlyList`1 items, Action`1 generationAction, Action`1 joinAction)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlFunction(SqlFunctionExpression sqlFunctionExpression)
         at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerQuerySqlGenerator.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitProjection(ProjectionExpression projectionExpression)
         at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerQuerySqlGenerator.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.<VisitSelect>b__21_0(ProjectionExpression e)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.GenerateList[T](IReadOnlyList`1 items, Action`1 generationAction, Action`1 joinAction)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSelect(SelectExpression selectExpression)
         at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerQuerySqlGenerator.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitScalarSubquery(ScalarSubqueryExpression scalarSubqueryExpression)
         at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerQuerySqlGenerator.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitProjection(ProjectionExpression projectionExpression)
         at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.SqlServer.Query.Internal.SqlServerQuerySqlGenerator.VisitExtension(Expression extensionExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.<VisitSelect>b__21_0(ProjectionExpression e)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.GenerateList[T](IReadOnlyList`1 items, Action`1 generationAction, Action`1 joinAction)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSelect(SelectExpression selectExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.GenerateRootCommand(Expression queryExpression)
         at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.GetCommand(Expression queryExpression)
         at Microsoft.EntityFrameworkCore.Query.Internal.RelationalCommandCache.GetRelationalCommandTemplate(IReadOnlyDictionary`2 parameters)
         at Microsoft.EntityFrameworkCore.Internal.RelationCommandCacheExtensions.RentAndPopulateRelationalCommand(RelationalCommandCache relationalCommandCache, RelationalQueryContext queryContext)
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.InitializeReaderAsync(AsyncEnumerator enumerator, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
         at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)
         at Microsoft.EntityFrameworkCore.EntityFrameworkQueryableExtensions.ToListAsync[TSource](IQueryable`1 source, CancellationToken cancellationToken)

Query

My query looks something like this:

var select = await unitOfWork.GetRepository<Product>()
            .GetAll()
            .Select(p => new GetProductsReadyForUseDto
            {
                ProductId = p.Id,
                TotalBalance = p.StockMovements.Sum(m => m.Quantity),
            })
            .ToListAsync(cancellationToken);

The ‘Quantity’ property (which is nullable) has the converter, which is shown in the text below, configured for it.

Converter

In my use case i am allowing the converter handle nulls:

public class DecimalToDefaultConverter : ValueConverter<decimal?, decimal>
{
    public DecimalToDefaultConverter()
        : base(
            v => v ?? default,
            v => v == default ? null : v,
            convertsNulls: true)
    {
    }
}

PS: In my case, my converter will actually change the right ‘0’ from the COALESCE into a NULL, which doesn’t make much sense (maybe the zero of the coalesce, added by EF Core, should skip the converter altogether?).

Suggestion

It seems that EF Core uses the converter from the property present in the sum expression and applies it to the zero from the COALESCE function, which is automatically added by EF Core itself.

I think that EF Core should either prevent the right zero from the COALESCE function from being passed to the converter, or perhaps passing the proper type when creating the COALESCE would fix the issue (?).

The following code passes the type to the right side of the COALESCE function instead of letting it be inferred by the type of the value.

image

https://github.com/dotnet/efcore/commit/fa3214023f3f15b5b2b4b5ee80b1e203b9c318a6

Include provider and version information

EF Core version: 8.0.6 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 8.0 Operating system: MacOS IDE: Rider

maumar commented 2 days ago

Verified it still repros on main.

    [ConditionalFact]
    public async Task Repro34753()
    {
        using (var ctx = new MyContext())
        {
            await ctx.Database.EnsureDeletedAsync();
            await ctx.Database.EnsureCreatedAsync();

            var s11 = new StockMovement { Quantity = 5.5M };
            var s12 = new StockMovement { Quantity = 15.5M };
            var s21 = new StockMovement { Quantity = 55.0M };
            var s22 = new StockMovement { Quantity = 115.0M };
            var p1 = new Product { Name = "p1", StockMovements = [s11, s12] };
            var p2 = new Product { Name = "p2", StockMovements = [s21, s22] };

            ctx.Products.AddRange(p1, p2);
            ctx.SaveChanges();
        }

        using (var ctx = new MyContext())
        {
            var select = await ctx.Products
            .Select(p => new 
            {
                ProductId = p.Id,
                TotalBalance = p.StockMovements.Sum(m => m.Quantity),
            })
            .ToListAsync();
        }
    }

    public class MyContext : DbContext
    {
        public DbSet<Product> Products { get; set; }
        public DbSet<StockMovement> StockMovements { get; set; }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<StockMovement>().Property(x => x.Quantity).HasConversion(new DecimalToDefaultConverter());
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder
                    .UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=Repro;Trusted_Connection=True;MultipleActiveResultSets=true")
                    .LogTo(Console.WriteLine, LogLevel.Information)
                    .EnableSensitiveDataLogging();
        }
    }

    public class DecimalToDefaultConverter : ValueConverter<decimal?, decimal>
    {
        public DecimalToDefaultConverter()
            : base(
                v => v ?? default,
                v => v == default ? null : v,
                convertsNulls: true)
        {
        }
    }

    public class Product
    {
        public int Id { get; set; }
        public string Name { get; set; }

        public List<StockMovement> StockMovements { get; set; }
    }

    public class StockMovement
    {
        public int Id { get; set; }
        public decimal Quantity { get; set; }
    }