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

DateTime AddXXX methods translated incorrectly for SQL Server #20901

Open dbrownems opened 4 years ago

dbrownems commented 4 years ago

Expressions involving DateTime.AddDays, DateTime.AddAddHours, etc are translated incorrectly, leading to errors or wrong results

Steps to reproduce

This program

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

namespace EfCore3Test
{

    public class Person
    {
        public int Id { get; set; }
        public string Name { get; set; }
        public DateTime ModifiedAt { get; set; }
    }

    public class Db : DbContext
    {

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            base.OnModelCreating(modelBuilder);
            modelBuilder.Entity<Person>();
        }
        private static readonly ILoggerFactory loggerFactory = LoggerFactory.Create(builder =>
        {
            builder.AddFilter((category, level) =>
               category == DbLoggerCategory.Database.Command.Name
               && level == LogLevel.Information).AddConsole();
        });

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseLoggerFactory(loggerFactory)
                          .UseSqlServer("Server=.;database=EfCore3Test;Integrated Security=true",
                                        o => o.UseRelationalNulls());

            base.OnConfiguring(optionsBuilder);
        }
    }

    class Program
    {

        static void Main(string[] args)
        {

            using var db = new Db();

            db.Database.EnsureDeleted();
            db.Database.EnsureCreated();

            var fred = new Person() { Name = "Fred" };
            db.Set<Person>().Add(fred);
            db.SaveChanges();

            var q = from p in db.Set<Person>()
                    where p.ModifiedAt < DateTime.Today
                       || p.ModifiedAt < DateTime.Today.AddDays(1.5)
//                       || p.ModifiedAt < DateTime.Today + TimeSpan.FromDays(1.5)
                       || p.ModifiedAt < DateTime.Today.AddHours(1.5)
                       || p.ModifiedAt < DateTime.Today.AddSeconds(1.5)
                       || p.ModifiedAt < DateTime.Today.AddMinutes(1.5)
                    select p;

            var r = q.ToList();

        }
    }

}

generates this query

SELECT [p].[Id], [p].[ModifiedAt], [p].[Name]
      FROM [Person] AS [p]
      WHERE (
      ((([p].[ModifiedAt] < CONVERT(date, GETDATE())) 
      OR ([p].[ModifiedAt] < DATEADD(day, CAST(1.5E0 AS int), CONVERT(date, GETDATE())))) 
      OR ([p].[ModifiedAt] < DATEADD(hour, CAST(1.5E0 AS int), CONVERT(date, GETDATE())))) 
      OR ([p].[ModifiedAt] < DATEADD(second, CAST(1.5E0 AS int), CONVERT(date, GETDATE())))) 
      OR ([p].[ModifiedAt] < DATEADD(minute, CAST(1.5E0 AS int), CONVERT(date, GETDATE())))

3 issues.

1. if you uncomment the line involving the timespan addition query generation fails with:

System.FormatException: 'Input string was not in a correct format.'

   at System.Globalization.TimeSpanFormat.FormatCustomized(TimeSpan value, ReadOnlySpan`1 format, DateTimeFormatInfo dtfi, StringBuilder result)
   at System.Globalization.TimeSpanFormat.TryFormat(TimeSpan value, Span`1 destination, Int32& charsWritten, ReadOnlySpan`1 format, IFormatProvider formatProvider)
 . . .

2. The fractional periods are truncated by the CAST.

This doesn't match the behavior of DateTime.AddXXX(double) which will add fractional days, hours or seconds.

3. CONVERT(date,GETDATE()) is not a the correct translation of DateTime.Today.

Changing the expression type from DATETIME to DATE causes this query fails in SQL Server with

Msg 9810, Level 16, State 1, Line 1
The datepart hour is not supported by date function dateadd for data type date.

Furthermore SQL Server's GETDATE() and the client's DateTime.Now are simply different times. EG Azure SQL Database is always in UTC.

It might be better if expressions like this not involving entity properties were simply evaluated on the client. Apart from bypassing these bugs, the database engine may better share and optimize queries with simple scalar parameters.

Further technical details

EF Core version: 3.1.3 Database provider: ( Microsoft.EntityFrameworkCore.SqlServer) Target framework: ( .NET Core 3.1) Operating system: Windows IDE: ( Visual Studio 2019 )

ajcvickers commented 4 years ago

@dbrownems

  1. Looks like a bug; full stack trace:
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (23ms) [Parameters=[@p0='?' (DbType = DateTime2), @p1='?' (Size = 4000)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      INSERT INTO [Person] ([ModifiedAt], [Name])
      VALUES (@p0, @p1);
      SELECT [Id]
      FROM [Person]
      WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();
Unhandled exception. System.FormatException: Input string was not in a correct format.
   at System.Globalization.TimeSpanFormat.FormatCustomized(TimeSpan value, ReadOnlySpan`1 format, DateTimeFormatInfo dtfi, StringBuilder result)
   at System.Globalization.TimeSpanFormat.TryFormat(TimeSpan value, Span`1 destination, Int32& charsWritten, ReadOnlySpan`1 format, IFormatProvider formatProvider)
   at System.TimeSpan.TryFormat(Span`1 destination, Int32& charsWritten, ReadOnlySpan`1 format, IFormatProvider formatProvider)
   at System.Text.StringBuilder.AppendFormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.FormatHelper(IFormatProvider provider, String format, ParamsArray args)
   at System.String.Format(IFormatProvider provider, String format, Object arg0)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapping.GenerateNonNullSqlLiteral(Object value)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapping.GenerateProviderValueSqlLiteral(Object value)
   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 System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSqlBinary(SqlBinaryExpression sqlBinaryExpression)
   at Microsoft.EntityFrameworkCore.Query.SqlExpressionVisitor.VisitExtension(Expression extensionExpression)
   at System.Linq.Expressions.Expression.Accept(ExpressionVisitor visitor)
   at System.Linq.Expressions.ExpressionVisitor.Visit(Expression node)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.VisitSelect(SelectExpression selectExpression)
   at Microsoft.EntityFrameworkCore.Query.QuerySqlGenerator.GetCommand(SelectExpression selectExpression)
   at Microsoft.EntityFrameworkCore.Query.Internal.RelationalCommandCache.GetRelationalCommand(IReadOnlyDictionary`2 parameters)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.InitializeReader(DbContext _, Boolean result)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at EfCore3Test.Program.Main(String[] args) in /home/ajcvickers/AllTogetherNow/ThreeOne/ThreeOne.cs:line 83
  1. This is a duplicate of #11794

  2. Can you explain in more detail what you mean by, "Changing the expression type from DATETIME to DATE?"

smitpatel commented 4 years ago

First one should be blocked during translation. It is doing DateTime + TimeSpan, we have code to block that translation since SqlServer does not support it. I believe issue is DateTime.Today does not have TypeMapping assigned so we let it pass. And eventually tried to print a TimeSpan literal using DateTimeTypeMapping.

NetMage commented 4 years ago

Why couldn't you convert DateTime + TimeSpan to DATEADD with the components from the TimeSpan?

smitpatel commented 4 years ago

@NetMage - https://github.com/dotnet/efcore/issues/11039

dbrownems commented 4 years ago

Can you explain in more detail what you mean by, "Changing the expression type from DATETIME to DATE?"

Sure. It's a common practice in TSQL to convert to DATE to truncate the time portion of an expression. But it's really a shortcut with some unwanted side-effects. In most cases SQL Server's permissive implicit conversion rules will convert the DATE back to DATETIME or DATETIME2 as needed, but not always.

This query (and the ones in the issue):

select dateadd(second,1,cast(getdate() as date))

fail with


Msg 9810, Level 16, State 1, Line 1
The datepart second is not supported by date function dateadd for data type date.

As adding seconds, minutes, or hours to a DATE is simply not allowed. Dateadd (like many TSQL functions) is type-preserving, ie the return type matches the data type of one of its arguments. The docs call this "dynamic" but it's really not.

Conceptually it's like a family of C# overloads

public date dateadd(datepart,number,date)
public datetime dateadd(datepart,number,datetime)
public datetime2 dateadd(datepart,number,datetime2)

So you can't allow adding seconds to a DATE because the expression dateadd(second,1,someDate) is of type DATE.

The correct translation of DateTime.Today would be a datetime or datetime2 expression like:

cast(cast(sysdatetime() as date) as datetime2)

or the old-school

convert(datetime, convert( char(8), getdate(), 112))

round-tripping from datetime to datetime through either the DATE data type or an culture-invariant string format, YYYYmmDD, that omits the time portion..

Or the arguably-correcter

cast('20200513' as datetime2)

Which passes the client's Today to the server for evaluation.