-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
DateTime.Kind comes back as Unspecified #4711
Comments
|
A slight variation of this is that it would be nice if one could configure the default |
+1 on whar @rmja suggested, it's a really nice addition. |
One thing worth noting is that I think the |
Note for triage: Investigated mapping to DateTime CLR type to SQL Server datetimeoffset by default, but even when switching to the typed value buffer factory this still won't work without type conversions. |
Conclusions from triage:
|
@rowanmiller how about my suggestion? It would be really useful in systems where the kind does not need to be stored, because it is agreed upon by convention. |
@rmja I should have mentioned that too... at the moment we rely on ADO.NET to construct the value and then we set it on the property. We do want to introduce a feature where you can control how that value is get/set from/to the property. This would allow things like setting DateTime.Kind, type conversions, using Get/Set methods rather than properties, etc. #240 is the issue tracking this feature |
@rowanmiller would you mind giving an example (like my second comment) that uses |
Never mind my last comment, I didn't realize there was a corresponding sql type called |
@gzak glad it's working for you now |
I also would want such a convention @rmja mentioned (#4711 (comment)). Because not all database providers have datetimeoffset type. |
I found a solution for that and shared as a blog post: http://volosoft.com/datetime-specifykind-in-entity-framework-core-while-querying/ |
I had this same issue today. Why is Datetime/Timezones still an issue? I don't really care how DateTime is stored in the database as long as the returned value is the same as when I stored it. It could even be stored as a standardized(ISO 8601) string for all I care. |
Switching to DateTimeOffset doesn't solve the problem. It isn't time zone aware; it only stores the UTC offset for a specific instant in time. There is no simple way to derive the time zone where the value was created. Picking the underlying UTC value out of the DateTimeOffset value would be the best you could do and DateTime is just as good at assuming that. |
@EEaglehouse and others. I'm working on a change that will allow the kind to be be preserved by storing the DateTime as a long in the database and automatically converting it on the way in and out. Should be in 2.1 as part of #242. |
@ajcvickers Thank you. It would be nice to have that as an option. For me, the better solution remains to have an option to force the Kind to a particular value (Utc in my case) for DateTime values. Reading it as a DateTimeOffset is useless to me because I would still need to convert to DateTime with Kind as Utc in my applications' data layer; all local time zone conversions are handled in the presentation layer. Converting DateTime to a long for storage may be useful, but would effectively obfuscate the column type in the database. But please don't take this as discouraging you from implementing the custom type mapping, because then I could do for myself what I'm asking. |
@EEaglehouse Yes, after #242 you will be able to set things up to still store as a datetime in the database, but force a specific kind whenever it is read from the database. |
We are using this updated workaround from: http://volosoft.com/datetime-specifykind-in-entity-framework-core-while-querying/ public class DateTimeKindMapper
{
public static DateTime Normalize(DateTime value)
=> DateTime.SpecifyKind(value, DateTimeKind.Utc);
public static DateTime? NormalizeNullable(DateTime? value)
=> value.HasValue ? DateTime.SpecifyKind(value.Value, DateTimeKind.Utc) : (DateTime?)null;
public static object NormalizeObject(object value)
=> value is DateTime dateTime ? Normalize(dateTime) : value;
}
public class DateTimeKindEntityMaterializerSource : EntityMaterializerSource
{
private static readonly MethodInfo _normalizeMethod =
typeof(DateTimeKindMapper).GetTypeInfo().GetMethod(nameof(DateTimeKindMapper.Normalize));
private static readonly MethodInfo _normalizeNullableMethod =
typeof(DateTimeKindMapper).GetTypeInfo().GetMethod(nameof(DateTimeKindMapper.NormalizeNullable));
private static readonly MethodInfo _normalizeObjectMethod =
typeof(DateTimeKindMapper).GetTypeInfo().GetMethod(nameof(DateTimeKindMapper.NormalizeObject));
public override Expression CreateReadValueExpression(Expression valueBuffer, Type type, int index, IProperty property = null)
{
if (type == typeof(DateTime))
{
return Expression.Call(
_normalizeMethod,
base.CreateReadValueExpression(valueBuffer, type, index, property));
}
if (type == typeof(DateTime?))
{
return Expression.Call(
_normalizeNullableMethod,
base.CreateReadValueExpression(valueBuffer, type, index, property));
}
return base.CreateReadValueExpression(valueBuffer, type, index, property);
}
public override Expression CreateReadValueCallExpression(Expression valueBuffer, int index)
{
var readValueCallExpression = base.CreateReadValueCallExpression(valueBuffer, index);
if (readValueCallExpression.Type == typeof(DateTime))
{
return Expression.Call(
_normalizeMethod,
readValueCallExpression);
}
if (readValueCallExpression.Type == typeof(DateTime?))
{
return Expression.Call(
_normalizeNullableMethod,
readValueCallExpression);
}
if (readValueCallExpression.Type == typeof(object))
{
return Expression.Call(
_normalizeObjectMethod,
readValueCallExpression);
}
return readValueCallExpression;
}
} Workaround from @hikalkan works, except cases in subquery, like: context.Entity.Select(e => new
{
DateTimeField = e.NavigationProperty.OrderBy(n => n.OrderField).FirstOrDefault().DateTimeField
}); So extended And it has additional fix for nullable |
Hi, Alexander,
That looks like a nice solution and more advanced than the solution we have been using, which was to use T4 to generate a custom DBContext. The code you presented is pretty easy to understand and not very complicated. When we migrate to .NET Core, this will definitely come in handy.
Thank you very much for taking the time to share it with me!
…--Ed
From: Alexander Puzynia [mailto:notifications@github.com]
Sent: Thursday, January 18, 2018 1:32 AM
To: aspnet/EntityFrameworkCore <EntityFrameworkCore@noreply.github.com>
Cc: Ed Eaglehouse <eeaglehouse@buckeyemountain.com>; Mention <mention@noreply.github.com>
Subject: Re: [aspnet/EntityFrameworkCore] DateTime.Kind comes back as Unspecified (#4711)
We are using this updated workaround from: http://volosoft.com/datetime-specifykind-in-entity-framework-core-while-querying/
public class DateTimeKindMapper
{
public static DateTime Normalize(DateTime value)
=> DateTime.SpecifyKind(value, DateTimeKind.Utc);
public static DateTime? NormalizeNullable(DateTime? value)
=> value.HasValue ? DateTime.SpecifyKind(value.Value, DateTimeKind.Utc) : (DateTime?)null;
public static object NormalizeObject(object value)
=> value is DateTime dateTime ? Normalize(dateTime) : value;
}
public class DateTimeKindEntityMaterializerSource : EntityMaterializerSource
{
private static readonly MethodInfo _normalizeMethod =
typeof(DateTimeKindMapper).GetTypeInfo().GetMethod(nameof(DateTimeKindMapper.Normalize));
private static readonly MethodInfo _normalizeNullableMethod =
typeof(DateTimeKindMapper).GetTypeInfo().GetMethod(nameof(DateTimeKindMapper.NormalizeNullable));
private static readonly MethodInfo _normalizeObjectMethod =
typeof(DateTimeKindMapper).GetTypeInfo().GetMethod(nameof(DateTimeKindMapper.NormalizeObject));
public override Expression CreateReadValueExpression(Expression valueBuffer, Type type, int index, IProperty property = null)
{
if (type == typeof(DateTime))
{
return Expression.Call(
_normalizeMethod,
base.CreateReadValueExpression(valueBuffer, type, index, property));
}
if (type == typeof(DateTime?))
{
return Expression.Call(
_normalizeNullableMethod,
base.CreateReadValueExpression(valueBuffer, type, index, property));
}
return base.CreateReadValueExpression(valueBuffer, type, index, property);
}
public override Expression CreateReadValueCallExpression(Expression valueBuffer, int index)
{
var readValueCallExpression = base.CreateReadValueCallExpression(valueBuffer, index);
if (readValueCallExpression.Type == typeof(DateTime))
{
return Expression.Call(
_normalizeMethod,
readValueCallExpression);
}
if (readValueCallExpression.Type == typeof(DateTime?))
{
return Expression.Call(
_normalizeNullableMethod,
readValueCallExpression);
}
if (readValueCallExpression.Type == typeof(object))
{
return Expression.Call(
_normalizeObjectMethod,
readValueCallExpression);
}
return readValueCallExpression;
}
}
Workaround from volosoft works, except cases in subquery, like:
context.Entity.Select(e => new
{
DateTimeField = e.NavigationProperty.OrderBy(n => n.OrderField).FirstOrDefault().DateTimeField
});
So extended EntityMaterializerSource can handle this as well, but because it wraps every object response as well, it can cause additional performance cost because of object to DateTime cast.
And it has additional fix for nullable DateTime as well. Before 2.1 we will use this workaround.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#4711 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ASrwvT9VrPRI9gyXit0b6RQOJd0K7-zyks5tLuVMgaJpZM4HrCRe>.
|
Please note that EntityMaterializerSource is internal code and any solution like that shown above may get broken with a new release of EF. That doesn't mean I'm saying not to use it--it's publicly available internal code for a reason--but please be aware of the risk associated with it. Starting in EF Core 2.1, this would be one way to deal with DateTime.Kind: modelBuilder
.Entity<Foo>()
.Property(e => e.SomeDate)
.HasConversion(v => v, v => DateTime.SpecifyKind(v, DateTimeKind.Local)); This will ensure every time the date is read from the database it is specified as Local automatically. |
@fartwhif EF Core already allows you to specify the DateTime.Kind coming back from the databases - via value converters - and 6.0 even allows you to configure that globally for all DateTime properties in your model. Beyond that it's hard to see what EF Core could do - SQL Server's datetime/datetime2 types simply do not store DateTimeKind, which is a .NET-only concept. Are you looking for anything in particular? |
@roji IMO, the only real solution is to store the value in two columns (datetime and timezone name). Unfortunately DateTimeOffset is not a real solution for this problem since many timezone's exist within a single offset (for example Arizona and Colorado are both in Mountain Time however Arizona does not have daylight savings while Colorado does) and those timezones could have different rules about daylight savings time. Since this isn't a feature of EF Core, I always store all my datetime's in UTC and manually store the timezone name when necessary. If only daylight savings didn't exist :P |
Note that there's a very big difference between storing the DateTime's timestamp and kind, and storing a timestamp and timezone - this issue really is about fully persisting a DateTime, which does not have a timezone name (or even an offset). Aside from that, it's not just the database that doesn't have a single type for timestamp+timezone, it's also .NET (I recommend looking at NodaTime, which does have ZonedDateTIme). Other than that, if your goal is to save a timestamp and timezone name, then you're right that you need two columns (see this blog post which I just recently wrote about the subject). To summarize, on the .NET side you need two properties (DateTime and string, for the time zone), and on the SQL Server side you need two columns (datetime2 and nvarchar). If you want to use NodaTime (highly recommended), then on the .NET side you can have a single property of time ZonedDateTime. At the moment EF Core cannot map a single .NET property to multiple columns, but that feature is planned for EF7 (#13947). |
@roji could you elaborate on the global configuration? I cannot see to find a simple way of doing this in our new EF Core setup, that uses an existing schema (and therefore the two column option is not viable for us). We don't care about storing the datetime kind in the database, just that all datetime properties are returned as UTC instead of unspecified. At present our mapping code from ef model -> domain model custom converts every single property which means it's down to a developer to remember that... |
@SebFerraro above I just referred to the ability to configure value converters to modify the Kind on DateTimes being read, as here. That can be combined with the new EF Core 6.0 bulk configuration to apply this globally to your model. But I want to stress again that this isn't the ideal way to do it. If what you want is to simply store UTC timestamps and to retrieve them as UTC DateTimes, use version 6.0 and map your EF DateTime properties to |
@roji Thanks - I was more meaning that I can't see to find an example of the new convention specifically converting to datetime kind anywhere. The individual method being:
The format must have changed as I cannot figure out how to implement that at configbuilder level. Eg. This does not work
I also appreciate this is not the best way of doing it. But it's better than our current implementation working on a schema that's 14 years old with 160+ entities, all of which have 1-3 datetimes on them. |
@SebFerraro the bulk configuration using using System;
using System.Reflection;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Storage.ValueConversion;
namespace Whatever;
public class UtcValueConverter : ValueConverter<DateTime, DateTime>
{
public UtcValueConverter()
: base(v => v, v => DateTime.SpecifyKind(v, DateTimeKind.Utc))
{
}
}
public static class UtcDateAnnotation
{
private const string IsUtcAnnotation = "IsUtc";
public static PropertyBuilder<TProperty> IsUtc<TProperty>(this PropertyBuilder<TProperty> builder, bool isUtc = true)
{
if (builder is null)
{
throw new ArgumentNullException(nameof(builder));
}
return builder.HasAnnotation(IsUtcAnnotation, isUtc);
}
public static bool IsUtc(this IMutableProperty property)
{
if (property is null)
{
throw new ArgumentNullException(nameof(property));
}
var attribute = property.PropertyInfo?.GetCustomAttribute<IsUtcAttribute>();
if (attribute is not null && attribute.IsUtc)
{
return true;
}
return ((bool?)property.FindAnnotation(IsUtcAnnotation)?.Value) ?? true;
}
/// <summary>
/// Make sure this is called after configuring all your entities.
/// </summary>
public static void ApplyUtcDateTimeConverter(this ModelBuilder builder)
{
if (builder is null)
{
throw new ArgumentNullException(nameof(builder));
}
foreach (var entityType in builder.Model.GetEntityTypes())
{
foreach (var property in entityType.GetProperties())
{
if (!property.IsUtc())
{
continue;
}
if (property.ClrType == typeof(DateTime) ||
property.ClrType == typeof(DateTime?))
{
property.SetValueConverter(typeof(UtcValueConverter));
}
}
}
}
}
[AttributeUsage(AttributeTargets.Property)]
public class IsUtcAttribute : Attribute
{
public IsUtcAttribute(bool isUtc = true) => this.IsUtc = isUtc;
public bool IsUtc { get; }
} then in THE END OF (important!)
so your developers only need to remember never put any configuration below that line, and all properties will be automatically converted for @roji slightly off topic: the links in the first paragraph of the docs you sent a link for are broken (see "xref:") https://docs.microsoft.com/en-us/ef/core/modeling/bulk-configuration#pre-convention-configuration EDIT: @SebFerraro did not see that you used existing schema, sorry. For others reading this issue with code-first and wanting to use precompiled model, I'll keep this post nevertheless... |
@joakimriedel it was an existing database that I ported into a brand new ef core project, and your solution has actually worked, which is great! Thanks! |
@joakimriedel @SebFerraro if what you want is for DateTime to always come back as UTC, there's no need for an attribute; @SebFerraro your code didn't work since HaveConversion in ConfigureConventions only accepts a value converter type, rather than lambdas (in order to better support compiled models). See below for a minimal code sample which shows everything working without an attribute. Code sampleawait using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();
var blog = await ctx.Blogs.SingleAsync();
Console.WriteLine(blog.DateTime.Kind);
public class BlogContext : DbContext
{
public DbSet<Blog> Blogs { get; set; }
protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
=> optionsBuilder
.UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0")
.LogTo(Console.WriteLine, LogLevel.Information)
.EnableSensitiveDataLogging();
protected override void OnModelCreating(ModelBuilder modelBuilder)
=> modelBuilder.Entity<Blog>().HasData(new Blog { Id = 1, DateTime = DateTime.Now });
protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
{
configurationBuilder
.Properties<DateTime>()
.HaveConversion(typeof(UtcValueConverter));
}
class UtcValueConverter : ValueConverter<DateTime, DateTime>
{
public UtcValueConverter()
: base(v => v, v => DateTime.SpecifyKind(v, DateTimeKind.Utc))
{
}
}
}
public class Blog
{
public int Id { get; set; }
public DateTime DateTime { get; set; }
}
Thanks. The 6.0 API docs (which this links point to) have only recently been pushed online in dotnet/EntityFramework.Docs#3630, and I don't think we've pushed our conceptual docs since then - this should go away the next time we do. |
@roji the attribute is a negative option, so the code will actually set the value converter for all DateTime properties without using any attributes. But regarding the
Since I explicitly set a model via But you're saying it should work with compiled models? I will try right away! EDIT: It actually works. 🤯 I'd definitely suggest rewording this remark, it had me fooled ever since 6.0 released. Perhaps
.. or something similar, if this is actually how it works? |
/cc @AndriySvyryd for the above remark |
In order to save it as Utc I use:
This way if someone uses unspecified or local time at least it is converted to utc time. |
@qsdfplkj Be carful with this approach. If the DateTimeKind is Unspecified and you call ToUniversalTime, it will assume that the DateTime is in local time which can cause some hard to find bugs. This can easily happen when crossing over a serialization boundary such as a WebAPI. |
Thanks I will check this serialization scenario. |
Here's my take on this for a nullable DateTime, inspired by @qsdfplkj's and @ChristopherHaws' answers: public DateTime? TimestampUtc { get; set; } builder.Property(x => x.TimestampUtc)
.HasConversion(new UtcDateTimeConverter()); public class UtcDateTimeConverter : ValueConverter<DateTime?, DateTime?>
{
public UtcDateTimeConverter()
: base(
d => !d.HasValue ? null : ConvertToUtc(d.Value),
d => !d.HasValue ? null : SpecifyUtc(d.Value))
{
}
private static DateTime ConvertToUtc(DateTime date)
{
switch (date.Kind)
{
case DateTimeKind.Utc:
return date;
case DateTimeKind.Local:
return date.ToUniversalTime();
default:
throw new InvalidTimeZoneException($"Unsupported DateTimeKind: {date.Kind}");
}
}
private static DateTime SpecifyUtc(DateTime date)
{
return DateTime.SpecifyKind(date, DateTimeKind.Utc);
}
} |
I didn't use this approach anymore because usually data comes in from an API and it is unspecified. So I decided that unspecified is the actually the better way. (Maybe use DateTimeOffset?) |
Thanks for the suggestion. I actually use both types. My model looks like this: public DateTimeOffset Timestamp { get; set; }
public DateTime InsertedAtUtc { get; set; } Timestamp is when an event actually occured. It's useful for our users to know the offset. (Yes, the API requires an offset to be included.) On the other hand, the insertion date is automatically set in the backend, and only UTC makes sense here. Maybe I'm overthinking, but I think that having the possibility to store an offset creates ambiguity. |
BTW: npgsql (postgres) provider just disallows (per default) any non-UTC DateTime, both for storage and query operations. And always return UTC, too - of course. |
Carefully consider what exactly this means: are you really saying that you have no idea what time zone the timestamps are, which your API provides? If that's true, you can't really do anything with that data, since it could refer to arbitrary moments in time, etc. It's more common for the time zone to be simply implicit, e.g. for the timestamps to be in UTC or some very specific local timestamp - even if it's not specified. If that's the case, I highly recommend changing the DateTime's Kind to Utc (via DateTime.SpecifyKind), right where you're reading the timestamps, and not when writing them to the database (if you're deserializing JSON from the API, you can probably configure the JSON deserializer to do this for you as well). Note that DateTimeOffset also implies that you know which time zone the timestamps are in. |
I guess I've to revisit this but timestamp is always Utc. But what is a local times on a server in azure anyway? |
@qsdfplkj if the timestamp is UTC, then it should be represented in .NET as a DateTime with Kind=Utc (or possibly as DateTimeOffset with Offset=0). Do this as early as possible (i.e. as soon as you read the value from an external source), and persist it to PostgreSQL as a As to how Azure servers are configured, I have no idea - that likely depends how you set things up. But the server time zone should not matter: your application should be be using e.g. DateTime.UtcNow rather than DateTime.Now to get current timestamps in UTC, and so be oblivious of the server timestamp. The exception to using UTC should be when you really do care about the time zone (e.g. since some event needs to happen at some point in time in some user's time zone), in which case you need to store the time zone (not offset!) in your database (e.g. see this blog post). |
after reading all this discussion and the related npgsql doc, I am still a bit confused about what I should do in our very classic use case:
dotnet swagger generate the openapi.json with properties with JsonSerializer also defined UTC per default AKAIK . Everything seem to work fine in our first tests. So apparently, there is NOTHING else to add, no need for DateTimeOffset, special converter and so one if UTC is the default all the way. |
@ericbl the details vary somewhat across databases. Generally speaking, there's no need at all to switch to DateTimeOffset for using UTC timestamps; a DateTime with Kind=Utc will do just as well. However, since e.g. SQL Server doesn't store the Kind, DateTime instances read from it will have Kind=Unspecified; this could be a reason to use DateTimeOffset instead, since in that case an offset of 0 can be stored in the database. Or you can use an EF value converter to ensure that DateTimes read from the database have Kind=Utc. In PostgreSQL that particular problem doesn't exist. Beyond that, if you have a specific question or difficulty, please let us know and we'll try to help. |
So being able to round-trip public class UtcDateTimeConverter : ValueConverter<DateTime, DateTime>
{
public UtcDateTimeConverter()
: base(
static datetime => datetime.ToUniversalTime(),
static datetime => DateTime.SpecifyKind(datetime, DateTimeKind.Utc)
)
{
}
} And then manually applying this to every EF really needs this as a built-in feature, with the ability to specify this behaviour as a global convention. The ability to actually store a zone offset is often unnecessary, rather we just want the Even if EF picks up the ability to set a global value converter without the need for all that reflection over entity properties, I think UTC DateTime conversion should still be a built-in feature, since it's a super common pitfall and all this boilerplate should ultimately be unnecessary. |
Note that this isn't what "round-tripping" means - your converter makes it so that all DateTimes are returned with Kind=Utc; in other words, the Kind property of the DateTime is lost.
Not all users are actually interested in saving UTC timestamps in the database; many scenarios work only with local timestamps of a specific (implicit) timezone, and for those, Unspecified can be a good option. We would in any case not change the default way that DateTime is written and read, as that would be a big breaking change; in addition, Unspecified DateTimes are returned by the database driver itself (SqlClient), and it's not EF's role to override that decision. Given that we wouldn't change the default, including a type converter such as what you propose above still wouldn't remove the need to actually configure that converter; because of that, and because the actual converter code is very short/trivial, that seems to be quite low value... |
Agree with everything @roji said, but if you're interested, the available built-in conversions, are documented. |
@crozone just a side note on using var unspecified = new DateTime(2024, 05, 02, 12, 0, 0);
Console.WriteLine(unspecified.ToLocalTime());
Console.WriteLine(unspecified.ToUniversalTime());
I'm quite happy with npgsqls decision to just throw on any non-utc value. |
If you store a
DateTime
object to the DB with aDateTimeKind
of eitherUtc
orLocal
, when you read that record back from the DB you'll get aDateTime
object whose kind isUnspecified
. Basically, it appears theKind
property isn't persisted. This means that a test to compare equality of the before/after value of a round trip to the DB will fail.Is there something that needs to be configured for the
Kind
to be preserved? Or what's the recommended way to handle this?The text was updated successfully, but these errors were encountered: