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.78k stars 3.19k forks source link

Unnecessary CAST/convert static value to incorrect type. SqlServer. #23072

Open rtynski opened 4 years ago

rtynski commented 4 years ago

Example code to reproducing error.

using System;
using System.ComponentModel.DataAnnotations;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

public class Program
{
    static void Main(string[] args)
    {
        using (var context = new MainContext())
        {
            context
                .People
                .Select(t => Convert.ToInt32(t.PersonId) * 500 + t.PersonType)
                .ToList();
        }
    }
}

public class Person
{
    [Key]
    public int PersonId { get; set; }
    public byte PersonType { get; set; }
}
public class MainContext : DbContext
{
    public DbSet<Person> People { get; set; }
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer("Server=localhost;Database=test01;User Id=sa;Password=@Passw0rd");
        optionsBuilder.UseLoggerFactory(LoggerFactory.Create(builder =>
        {
            builder.AddConsole();
        }));
    }
}

SQL look like (in this code is error):

SELECT (CONVERT(int, [p].[PersonId]) * CAST(500 AS tinyint)) + [p].[PersonType]
FROM [People] AS [p]

In this case we get error, because 500 is out of range tinyint. This case is also for other types who we can send as value, and we can exceed the range.

EF Core version: 3.1.9, 5.0#73566d1fb936ebccd9e8c476fb286ffd451bdd46 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 5.0, .NET 3.1 Operating system: Windows 10 IDE: Visual Studio 2019 16.8.0 Preview 5.0,

smitpatel commented 4 years ago

Convert.ToInt(a) and 500 has no intrinsic type mapping associated with it. So it is inferred from type of t.PersonType which is tinyint. Since the result does not fit in tinyint, try casting t.PersonType to int.

This is another case of #14719 In this case, it actually overflows and throws.

rtynski commented 4 years ago

This case is similar but not the same. To resolve my example from title message you can cast other value in this query.

I thinking about correct implementation "maths operation".

For example for integers, we have:

DECLARE @TinyInt TINYINT = 255
DECLARE @SmallInt SMALLINT = 32767
DECLARE @StandardInt INT = 2147483647
DECLARE @BigInt BIGINT = 9223372036854775807

SELECT 
    @TinyInt as [TinyInt],
    @SmallInt as [SmallInt],
    @StandardInt as [StandardInt],
    @BigInt as [BigInt],
    -- for operation where the biggest integer type is BigInt result is BigInt
    @BigInt - @StandardInt as [BigInt_1],
    -- for operation where the biggest integer type is Int result is Int
    @StandardInt - @SmallInt as [StandardInt_1],
    -- for operation where the biggest integer type is SmallInt result is SmallInt
    @SmallInt - @TinyInt as [SmallInt_1]
    -- Any used types can return "Arithmetic overflow error converting expression to data type ...." for incorect operation
    --,@TinyInt + @TinyInt as [Error]
INTO [__TEST01]

GO

SELECT A1.name as ColumnName,A3.name
FROM sys.all_columns A1
INNER JOIN
sys.tables A2 ON A1.object_id = A2.object_id
INNER JOIN
sys.types A3 ON A1.system_type_id = A3.system_type_id
WHERE A2.name = '__TEST01'
ORDER BY A1.column_id

GO

DROP TABLE [__TEST01]

Return types must be the same for all kinds database.

I think, (but i do not have 100% certainty) other database engine calculate identical.

smitpatel commented 4 years ago

@rafalt - What you described is a different issue being tracked at https://github.com/dotnet/efcore/issues/15586 When doing an operation between 2 different types which are compatible, most database engine will upcast to make types same and then apply operation and that would be resulting type after operation. Generally this mismatch does not affect queries since in C# math operations are also defined over same type, so if you write int + long in a LINQ query, compiler introduces a conversion from int to long before applying addition and which ends up reflecting in SQL. Which is something can be removed if EF Core knew all type casting rules for particular database.

Why this case matches #14719, is that, we decided to utilize type mapping which was inferred from the column and apply it to constant. But the type mapping is not large enough for the computation. The example in #14719 explains that for string type mapping. If string type mapping threw exception or truncated value when larger than specified type, it would have thrown error too. Basically in either case, EF Core need to figure out semantics around how to apply inferred type mapping based on operation (regardless of server upcasting rules). Further, even with those semantics, it may not always work. An example would be addition of long + int if the computed value causes overflow for long, then even with type upcasting of int to long, it would not work.