Default DateTime value on smalldatetime column throws exception when saving new object

[jirikanda]

It is not possible to use DateTime default value for columns of type smalldatetime.

Exception message: Conversion failed when converting character string to smalldatetime data type.
Stack trace:
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(DbContext _, ValueTuple`2 parameters)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at ConsoleApp.Program.Main(String[] args) in D:\Dev\ConsoleApp1\ConsoleApp3\Program.cs:line 46

Steps to reproduce

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

namespace ConsoleApp
{
	public class Note
	{
		public int Id { get; set; }
		public string Name { get; set; } = String.Empty;
		public DateTime Created { get; set; } // lets create a database default for this property
	}

	public class NoteDbContext : DbContext
	{
		protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
		{
			base.OnConfiguring(optionsBuilder);
			optionsBuilder.UseSqlServer("Data Source=(localdb)\\mssqllocaldb;Initial Catalog=ConsoleApp;ConnectRetryCount=0");
		}

		protected override void OnModelCreating(ModelBuilder modelBuilder)
		{
			base.OnModelCreating(modelBuilder);
			modelBuilder.Entity<Note>(entity =>
			{
				// [Created] smalldatetime NOT NULL DEFAULT '2010-01-01T00:00:00.0000000'
				entity.Property(note => note.Created).HasColumnType("smalldatetime");
				entity.Property(note => note.Created).HasDefaultValue(new DateTime(2010, 1, 1));
			});
		}
	}

	public static class Program
	{
		public static void Main(string[] args)
		{		
			using (var dbContext = new NoteDbContext())
			{
				dbContext.Database.EnsureDeleted();
				dbContext.Database.Migrate();

				dbContext.Set<Note>().Add(new Note());
				dbContext.SaveChanges(); // Conversion failed when converting character string to smalldatetime data type.
			}
		}
	}
}

Install nuget packages:

• Microsoft.EntityFrameworkCore.Design (version 2.2.1)
• Microsoft.EntityFrameworkCore.SqlServer (version 2.2.1)
• Microsoft.EntityFrameworkCore.Tools (version 2.2.1)

Add a migration:

• open Package Manager Console
• add-migration Initial

Workarounds

• Don't use smalldatetime, use datetime (better for us)
• Don't use HasDefaultValue, use HasDefaultValueSql.

Further technical details

EF Core version: 2.2.1
Database Provider: Microsoft.EntityFrameworkCore.SqlServer

[ajcvickers]

Note for triage: smalldatetime range is "January 1, 1900, through June 6, 2079"

[mirol-h]

This bug affects me too, so I'd be willing to work on this. Is it still possible get a fix for this bug integrated into 3.0?

Browsing through the code of MigrationsSqlGenerator it looks like it is necessary to change signature of DefaultValue method:

https://github.com/aspnet/EntityFrameworkCore/blob/b8317e800841aaa3cb5de35fb90c78df3c617b1c/src/EFCore.Relational/Migrations/MigrationsSqlGenerator.cs#L1227

protected virtual void DefaultValue(
    [CanBeNull] object defaultValue,
    [CanBeNull] string defaultValueSql,
    [NotNull] MigrationCommandListBuilder builder)

This method for generating default value has no idea, what the type of the column is going to be. Because of this, it cannot get correct RelationalTypeMapping for datetime (it gets default for DateTime CLR type, which is datetime2). One solution could be to add string parameter type with store/database type:

protected virtual void DefaultValue(
    [CanBeNull] object defaultValue,
    [CanBeNull] string defaultValueSql,
    [CanBeNull] string type,
    [NotNull] MigrationCommandListBuilder builder)

and the implementation will prefer to use this type for resolving RelationalTypeMapping, instead of defaultValue object:

else if (defaultValue != null)
{
    var typeMapping = type != null ?
        Dependencies.TypeMappingSource.GetMapping(type) :
        Dependencies.TypeMappingSource.GetMappingForValue(defaultValue);

    builder
        .Append(" DEFAULT ")
        .Append(typeMapping.GenerateSqlLiteral(defaultValue));
}

Store/database type should be available at both calling sites (ColumnDefinition method) and in SqlServerMigrationsSqlGenerator.

I understand this is breaking change, because other database providers depend on this method (such as SqlServer provider), but that should not be a problem, since 3.0 is already introducing some breaking changes, right?

Any thoughts on this approach?

[bricelam]

@mirol-h That sounds like the right fix to me. Feel free to send a PR so we can further iterate on the implementation.

[ajcvickers]

@mirol-h Agree with @bricelam, but it may not get in to 3.0 because we limiting risk at this point.

[mirol-h]

OK, so I created PR including tests, but I would like to get some feedback, if they are OK (as I'm not familiar with tests for EF Core yet).

After looking at existing tests for MigrationSqlGenerator, I added new test in SqlServerMigrationSqlGeneratorTest:

[ConditionalFact]
public virtual void AddColumnOperation_smalldatetime_with_defaultValue()
{
    Generate(
        new AddColumnOperation
        {
            Table = "People",
            Schema = "dbo",
            Name = "Birthday",
            ClrType = typeof(DateTime),
            ColumnType = "smalldatetime",
            IsNullable = false,
            DefaultValue = new DateTime(2019, 1, 1)
        });

    AssertSql(@"ALTER TABLE [dbo].[People] ADD [Birthday] smalldatetime NOT NULL DEFAULT '2019-01-01T00:00:00';
");
}

This issue also affects datetime columns, so I went ahead and added a test for this column type too.