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

Support the new BCL DateOnly and TimeOnly structs for SQL Server #24507

Closed
roji opened this issue Mar 25, 2021 · 39 comments · Fixed by #30109
Closed

Support the new BCL DateOnly and TimeOnly structs for SQL Server #24507

roji opened this issue Mar 25, 2021 · 39 comments · Fixed by #30109
Assignees
Labels
area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-enhancement
Milestone

Comments

@roji
Copy link
Member

roji commented Mar 25, 2021

New DateOnly and TimeOnly structs are being introduced to .NET 6 as alternatives to DateTime (dotnet/runtime#49036). This issue tracks supporting them in SQL Server.

I've opened a separate SqlClient issue for support (dotnet/SqlClient#1009).

@mattjohnsonpint
Copy link

Please note that the final names for these types are DateOnly and TimeOnly and that they are now merged into the main branch for the next version of .NET 6 (preview 4 likely). Please rename the title of this issue accordingly. Thanks.

@roji roji changed the title Support the new BCL DateOnly and TimeOfDay structs for SQL Server Support the new BCL DateOnly and TimeOnly structs for SQL Server Apr 15, 2021
@roji roji removed this from the Backlog milestone May 16, 2021
@roji
Copy link
Member Author

roji commented May 16, 2021

Clearing milestone to discuss doing this in 6.0.0.

@mattjohnsonpint
Copy link

DateOnly and TimeOnly should map cleanly to SQL Server's date and time types. The tricky part will be to decide what to do with existing behavior that maps them to DateTime and TimeSpan. They should continue to function, but the new types should be preferred for things like scaffolding.

@CleanCodeX
Copy link

CleanCodeX commented Aug 2, 2021

As of NET 6 Preview 6 I get an error using EF Core mapping DateOnly properties stating that the DB Provider (SQL Server) does not support this type. Is it supported or not? If not, can HaveConversion be a workaround? so far nothing worked (except not using DateOnly).

@roji
Copy link
Member Author

roji commented Aug 2, 2021

@CleanCodeX EF Core support for DateOnly/TimeOnly depends on lower-level support in SqlClient; that's tracked in dotnet/SqlClient#1009.

@CleanCodeX
Copy link

As already discussed there, it's not supported. What's the common workaround?
builder.Properties<DateOnly>().HaveConversion(typeof(DateTime));
does not do the trick.
If possible at all, do I need to write a custom Converter/Comparer to make it work or is anything built-in available that works as a workaround to keep using DateOnly?

@roji
Copy link
Member Author

roji commented Aug 2, 2021

@CleanCodeX there isn't anything built-in for SQL Server, you'll have to implement your own value converter.

@CleanCodeX
Copy link

CleanCodeX commented Aug 2, 2021

@CleanCodeX there isn't anything built-in for SQL Server, you'll have to implement your own value converter.

So I did.
In case anyone has the same problem:
This seems to do the trick. (at least I get no errors anymore...) If you find any error within, feel free to correct it.

Edit: missing call to HaveColumnType added
Edit 2: as of RC2 the Converter for nullable DateOnly isn't recognized anymore.

      public class YourDbContext : DbContext
      {
            Ctors() {...}

        protected override void ConfigureConventions(ModelConfigurationBuilder builder)
        {
            builder.Properties<DateOnly>()
                .HaveConversion<DateOnlyConverter>()
                .HaveColumnType("date");

            builder.Properties<DateOnly?>()
                .HaveConversion<NullableDateOnlyConverter>()
                .HaveColumnType("date");
        }
      }
              
      /// <summary>
      /// Converts <see cref="DateOnly" /> to <see cref="DateTime"/> and vice versa.
      /// </summary>
      public class DateOnlyConverter : ValueConverter<DateOnly, DateTime>
      {
          /// <summary>
          /// Creates a new instance of this converter.
          /// </summary>
          public DateOnlyConverter() : base(
                  d => d.ToDateTime(TimeOnly.MinValue),
                  d => DateOnly.FromDateTime(d))
          { }
      }
   
      /// <summary>
      /// Converts <see cref="DateOnly?" /> to <see cref="DateTime?"/> and vice versa.
      /// </summary>
      public class NullableDateOnlyConverter : ValueConverter<DateOnly?, DateTime?>
      {
        /// <summary>
        /// Creates a new instance of this converter.
        /// </summary>
        public NullableDateOnlyConverter() : base(
            d => d == null 
                ? null 
                : new DateTime?(d.Value.ToDateTime(TimeOnly.MinValue)),
            d => d == null 
                ? null 
                : new DateOnly?(DateOnly.FromDateTime(d.Value)))
        { }
    }

I hope the dev community must not wait until NET 7, that a basic converter like this is added internally so users can use the short version of it:

    protected override void ConfigureConventions(ModelConfigurationBuilder builder)
    {
        builder.Properties<DateOnly>().HaveConversion<DateTime>();
        // OR
        builder.Properties<DateOnly>().HaveConversion<DateOnlyToDateTimeConverter>();
    }

Thanks for your time. Keep up the good work.

@akamud
Copy link

akamud commented Aug 2, 2021

I hope the dev community must not wait until NET 7, that a basic converter like this is added internally so users can use the short version of it:

It isn't just "a basic converter" because you are still using datetime in the database. It is a great workaround for anyone who want to just map to DateOnly/TimeOnly types, but I believe your solution won't easily understand querying on DateOnly/TimeOnly types, mapping from/to migrations, do scaffolding, etc. The official support for SQL Server should probably use date and time on the database when appropriate.

Just being fair with the team working on this ;)

@roji
Copy link
Member Author

roji commented Aug 2, 2021

It isn't just "a basic converter" because you are still using datetime in the database.

That's true, though it's possible to map DateTime to SQL Server date or time columns (using the Fluent API or Data Annotations). This would be a bit convoluted - use EF Core value converters to go from DateOnly to DateTime, and then tell SqlClient to write it to SQL Server date - but it would work.

Another thing missing from the above is translations from properties/methods on DateOnly/TimeOnly; so this would only take care of reading and writing the values.

@CleanCodeX
Copy link

CleanCodeX commented Aug 2, 2021

Yeah of course the the properties needed to be marked as date / time sql type (or with fluent api). I thought that would be clear then. My mistake if it wasn't.

Edit:

protected override void ConfigureConventions(ModelConfigurationBuilder builder)
        {
            builder.Properties<DateOnly>()
                .HaveConversion<DateOnlyConverter, DateOnlyComparer>()
                .HaveColumnType("date");
        }

@davidhenley
Copy link

What is the status on this with RC1? Do we still need to write custom converters?

@ErikEJ
Copy link
Contributor

ErikEJ commented Sep 15, 2021

DateOnly and TimeOnly support for SQL Server is not ready yet, earliest EF Core 7 is my guess.

@CleanCodeX

This comment has been minimized.

@davidbuckleyni
Copy link

Have these been removed in .net 7 and 2019 I get

[]( at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_0.<.ctor>b__0()
at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.b__0()
at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
The 'DateOnly?' property 'Supplier.LastModifiedDate' could not be mapped to the database type 'LastModifiedDate' because the database provider does not support mapping 'DateOnly?' properties to 'LastModifiedDate' columns. Consider mapping to a different database type or converting the property value to a type supported by the database using a value converter. See https://aka.ms/efcore-docs-value-converters for more information. Alternately, exclude the property from the model using the '[NotMapped]' attribute or by using 'EntityTypeBuilder.Ignore' in 'OnModelCreating'.)

@roji
Copy link
Member Author

roji commented Dec 5, 2022

@davidbuckleyni DateOnly and TimeOnly aren't yet supported when using the SQL Server provider, that's what this issue tracks.

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 6, 2022

I have just published a preview package that adds support for DateOnly and TimeOnly with EF Core 6 and 7.

Feel free to give it a try and provide feedback in the github repo.

https://www.nuget.org/packages/ErikEJ.EntityFrameworkCore.SqlServer.DateOnlyTimeOnly/6.0.0-preview.1#readme-body-tab

@WellspringCS
Copy link

@ErikEJ your timing couldn't have been more perfect for me. Giving it a whirl now.

@SimonCropp
Copy link
Contributor

doesnt the existence of ErikEJ.EntityFrameworkCore.SqlServer.DateOnlyTimeOnly imply that it should be possible for DateOnly and TimeOnly support to be added into the main EF codebase without waiting for a SqlClient update? @roji

@ErikEJ
Copy link
Contributor

ErikEJ commented Dec 8, 2022

@SimonCropp no. My extension depends on SqlClient 5.1 preview. If a LTS version is released before EF Core 8 it will be possible.

@roji
Copy link
Member Author

roji commented Dec 8, 2022

@SimonCropp yeah, as @ErikEJ wrote, an LTS version of SqlClient with DateOnly/TimeOnly support is expected to land before EF Core 8.0 is released (in fact, it may quite soon). Once that happens, we can add native support in the EF SQL Server provider.

@SimonCropp
Copy link
Contributor

@roji @ErikEJ thanks for the clarification

@elachlan
Copy link

We ran into this and we've had to use a work around. Many thanks Erik for your work unblocking this.

@Lonli-Lokli
Copy link

Is there any way to add this support to scaffold as well? Just to not override types manually after generation with db-first

@ErikEJ
Copy link
Contributor

ErikEJ commented Jan 3, 2023

@Lonli-Lokli My package should support Scaffolding as well.

But I will remind myself to add to EF Core Power Tools actually! ErikEJ/EFCorePowerTools#1642

@davidbuckleyni
Copy link

@davidbuckleyni DateOnly and TimeOnly aren't yet supported when using the SQL Server provider, that's what this issue tracks.

I new this ages ago dont no why only replying now

@roji roji removed the blocked label Jan 20, 2023
roji added a commit to roji/efcore that referenced this issue Jan 21, 2023
roji added a commit to roji/efcore that referenced this issue Jan 22, 2023
roji added a commit to roji/efcore that referenced this issue Jan 22, 2023
roji added a commit to roji/efcore that referenced this issue Jan 22, 2023
@roji roji added closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. and removed consider-for-current-release labels Jan 30, 2023
@ajcvickers ajcvickers modified the milestones: Backlog, 8.0.0-preview1 Feb 2, 2023
@ajcvickers ajcvickers modified the milestones: 8.0.0-preview1, 8.0.0 Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-sqlserver closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. punted-for-7.0 Originally planned for the EF Core 7.0 (EF7) release, but moved out due to resource constraints. type-enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.