Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nullable DateTime in projection class - System.Data.SqlClient.SqlException: 'Conversion failed when converting date and/or time from character string.' #12797

Closed
brunolau opened this issue Jul 25, 2018 · 24 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Milestone

Comments

@brunolau
Copy link

brunolau commented Jul 25, 2018

Having a one to many relationship, the main entity can have zero to n child entities. When I want to select the main entity's Id and most recent DateTime from the child entity collection, SQL execution crashes with a System.Data.SqlClient.SqlException

Exception message: 
Conversion failed when converting date and/or time from character string

Stack trace: 
   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)
   at System.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)
   at System.Data.SqlClient.SqlDataReader.Read()
   at Microsoft.EntityFrameworkCore.Storage.RelationalDataReader.Read()
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryingEnumerable`1.Enumerator.BufferlessMoveNext(DbContext _, Boolean buffer)
   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.Linq.Enumerable.FirstOrDefault[TSource](IEnumerable`1 source)
   at lambda_method(Closure )
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ResultEnumerable`1.GetEnumerator()
   at Microsoft.EntityFrameworkCore.Query.Internal.LinqOperatorProvider.ExceptionInterceptor`1.EnumeratorExceptionInterceptor.MoveNext()
   at System.Linq.Enumerable.First[TSource](IEnumerable`1 source)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.<>c__DisplayClass15_1`1.<CompileQueryCore>b__0(QueryContext qc)
   at Microsoft.EntityFrameworkCore.Query.Internal.QueryCompiler.Execute[TResult](Expression query)
   at Microsoft.EntityFrameworkCore.Query.Internal.EntityQueryProvider.Execute[TResult](Expression expression)
   at System.Linq.Queryable.FirstOrDefault[TSource](IQueryable`1 source)
   at EfCoreErrReproduction.Program.Main() 


SQL produced:
Executed DbCommand (57ms) [Parameters=[@__id_0='3422'], CommandType='Text', CommandTimeout='30']
SELECT TOP(1) [p].[Id], (
    SELECT COUNT(*)
    FROM [TicketNumberDelta] AS [t]
    WHERE [p].[Id] = [t].[TicketNumberId]
) AS [DeltaCount], COALESCE((
    SELECT TOP(1) [lx].[RecordTimeStamp]
    FROM [TicketNumberDelta] AS [lx]
    WHERE [p].[Id] = [lx].[TicketNumberId]
    ORDER BY [lx].[RecordTimeStamp] DESC
), '0001-01-01T00:00:00.0000000') AS [LastEntry]
FROM [TicketNumber] AS [p]
WHERE [p].[Id] = @__id_0

System.Data.SqlClient.SqlException: 'Conversion failed when converting date and/or time from character string.'

Steps to reproduce

EfCoreErrReproduction.zip

using Microsoft.EntityFrameworkCore;
using System;
using System.Collections.Generic;
using System.Linq;

namespace EfCoreErrReproduction
{
    class Program
    {
        static void Main(string[] args)
        {
            var db = new StrippedDownContext();
            var crash = db.TicketNumbers.Where(p => p.Id == 3422).Select(p => new ProjectionClass
            {
                Id = p.Id,
                DeltaCount = p.TicketNumberDeltas.Count(),
                LastEntry = p.TicketNumberDeltas.OrderByDescending(lx => lx.RecordTimeStamp).Select(le => le.RecordTimeStamp).FirstOrDefault()
            }).FirstOrDefault();
        }
    }

    class ProjectionClass
    {
        public int Id { get; set; }
        public int DeltaCount { get; set; }
        public DateTime? LastEntry { get; set; }
    }
}

namespace EfCoreErrReproduction
{
    public class StrippedDownContext : DbContext
    {
        public StrippedDownContext()
        {
        }

        public StrippedDownContext(DbContextOptions<StrippedDownContext> options)
            : base(options)
        {
        }


        public virtual DbSet<TicketNumber> TicketNumbers { get; set; }


        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            if (!optionsBuilder.IsConfigured)
            {
                optionsBuilder.UseSqlServer("db conn string");
            }
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<TicketNumber>(entity =>
            {
                entity.ToTable("TicketNumber");
            });

            modelBuilder.Entity<TicketNumberDelta>(entity =>
            {
                entity.ToTable("TicketNumberDelta");
               
                entity.HasOne(p => p.TicketNumber)
                    .WithMany(p => p.TicketNumberDeltas)
                    .HasForeignKey(p => p.TicketNumberId);

                entity.Property(e => e.RecordTimeStamp).HasColumnType("datetime");
            });
        }
    }

    public class TicketNumber
    {
        public TicketNumber()
        {
            TicketNumberDeltas = new List<TicketNumberDelta>();           
        }

        public int Id { get; set; }
        public virtual ICollection<TicketNumberDelta> TicketNumberDeltas { get; set; }
    }

    public partial class TicketNumberDelta
    {
        public int Id { get; set; }
        public int? TicketNumberId { get; set; }
        public DateTime RecordTimeStamp { get; set; }
        public virtual TicketNumber TicketNumber { get; set; }
    }
}

Further technical details

EF Core version: 2.1.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer
Operating system: Win 10 Enterprise x64
IDE: Visual Studio 2017 15.7.5

@maumar
Copy link
Contributor

maumar commented Jul 26, 2018

I was able to reproduce this on current bits - model needs to contain TicketNumber entity without any TicketNumberDeltas, so that COALESCE tries to return the second argument)

@maumar
Copy link
Contributor

maumar commented Jul 26, 2018

Bug was introduced when fixing: #10294. Before this scenario would throw a different exception - nullable object must have a value

@maumar
Copy link
Contributor

maumar commented Jul 26, 2018

@brunolau workaround is to cast the RecordTimeStamp to nullable DateTime before its projected, like so:

            var query = db.TicketNumbers.Where(p => p.Id == 3422).Select(p => new ProjectionClass
            {
                Id = p.Id,
                DeltaCount = p.TicketNumberDeltas.Count(),
                LastEntry = p.TicketNumberDeltas.OrderByDescending(lx => lx.RecordTimeStamp).Select(le => (DateTime?)le.RecordTimeStamp).FirstOrDefault()
            }).FirstOrDefault();

@brunolau
Copy link
Author

Workaround works, thank you!

@maumar
Copy link
Contributor

maumar commented Jul 26, 2018

Actually, fix for #10294 only exposed this issue for this particular scenario - I was able to reproduce it without the aggregate. It could still be a regression but need to investigate the underlying cause to determine this.

@ajcvickers ajcvickers added this to the 2.1.4 milestone Jul 30, 2018
@ajcvickers
Copy link
Member

@maumar Let's investigate for patch.

@ajcvickers
Copy link
Member

See also #12819 when working on this.

@ajcvickers
Copy link
Member

@maumar Any update on this?

@ksmithRenweb
Copy link

A potentially simpler reproduction is this.

var ret = from t in context.NullableDate
select new NotNullableDateDto
{
NotNullable = t.NullableDate ?? new DateTime(1900, 1, 1)
}

Code worked fine in Ef Core 2.0

@maumar maumar removed this from the 2.1.4 milestone Aug 8, 2018
@ajcvickers ajcvickers added this to the 3.0.0 milestone Aug 8, 2018
@maumar maumar removed this from the 3.0.0 milestone Aug 13, 2018
@maumar
Copy link
Contributor

maumar commented Aug 13, 2018

confirmed that scenario provided by @ksmithRenweb is a regression from 2.0

@maumar
Copy link
Contributor

maumar commented Aug 13, 2018

in 2.0 we used to generate:

SELECT COALESCE([t].[NullableDate], '1900-01-01T00:00:00.000') AS [NotNullable]
FROM [Nullables] AS [t]

and now we generate:

SELECT COALESCE([t].[NullableDate], '1900-01-01T00:00:00.0000000') AS [NotNullable]
FROM [Nullables] AS [t]

@smitpatel
Copy link
Contributor

@ksmithRenweb - What is the data type of Column NullableDate in database?

@maumar
Copy link
Contributor

maumar commented Aug 13, 2018

@smitpatel

            modelBuilder.Entity<Nullable>().Property(e => e.NullableDate).HasColumnType("datetime");

@maumar
Copy link
Contributor

maumar commented Aug 13, 2018

we should be using the type from the other side of coalesce to determine how to render a datetime literal for the second term. We have a general issue to be better about it, but perhaps there is something we can easily do for this specific case. @smitpatel thoughts?

@maumar maumar assigned smitpatel and unassigned maumar Aug 13, 2018
@smitpatel
Copy link
Contributor

https://github.com/aspnet/EntityFrameworkCore/blob/939f6d82e17c71f885419ff15e77b7ad8a0cda06/src/EFCore.Relational/Query/Sql/DefaultQuerySqlGenerator.cs#L1317-L1338

Looks like, our type mapping inference for Binary is missing out Coalesce by accident. Should be easy fix.

@ksmithRenweb
Copy link

@smitpatel Data Type of "datetime" and of course set to Allow Nulls.

@ajcvickers ajcvickers added this to the 3.0.0 milestone Aug 28, 2018
@smitpatel smitpatel removed this from the 3.0.0 milestone Jun 5, 2019
@smitpatel
Copy link
Contributor

@ajcvickers to give this a read before next triage

@ajcvickers
Copy link
Member

Notes from triage: The literal generator for the provider/type/value combination should throw if asked to generate a literal for values that cannot be represented in the store. Se also #12998.

@ajcvickers ajcvickers added this to the 3.0.0 milestone Jun 6, 2019
@ajcvickers ajcvickers assigned ajcvickers and unassigned smitpatel Jun 6, 2019
@ajcvickers ajcvickers modified the milestones: 3.0.0, Backlog Jun 28, 2019
@ajcvickers ajcvickers modified the milestones: Backlog, 5.0.0 Nov 13, 2019
@ajcvickers
Copy link
Member

Removing from milestone to discuss in triage. Not sure what is left to do here.

@ajcvickers ajcvickers removed this from the 5.0.0 milestone Dec 22, 2019
@ajcvickers
Copy link
Member

@smitpatel to create a repro.

@smitpatel
Copy link
Contributor

In 3.0 query pipeline we reverted the fix we made for #10294 which introduced Coalesce function in sql tree. Now we let server side be null and when reading back valuetype from it, get default value. Hence we don't generate problematic SQL anymore.

@smitpatel smitpatel added this to the 3.1.0 milestone Dec 23, 2019
@smitpatel smitpatel added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Dec 23, 2019
@ajcvickers ajcvickers removed their assignment Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported punted-for-3.0 type-bug
Projects
None yet
Development

No branches or pull requests

6 participants