DateTimeOffset does not convert in EF queries

[patrickklaeren]

In all areas where we are storing datetimes, we're currently utilising .NET's DateTimeOffset type. While we try and retain the timezone/offset information, it turns out, the EF Core driver for Postgres does not support conversions for the DateTimeOffset type and errors with client side evaluation required, an example would be:

context.Messages.GroupBy(x => x.Timestamp.Date).ToList()

This is impossible without client side evaluation.

Furthermore it seems people recommend not to use DateTimeOffset for postgres related work, and instead to utilise DateTime.

Therefore proposing to migrate to DateTime. I have added a migration in a local dev branch and the migration seems painless, with EF able to do everything automagically.

npgsql/efcore.pg#473

[JakenVeina]

I'm going to guess we can fix this with an EF value converter, similar to how we handle ulong, despite that not being a natively-supported data type. Going to experiment...

[JakenVeina]

Yup.

public static class ValueConverters
{
    public static readonly ValueConverter<DateTimeOffset, DateTime> DateTimeOffsetToDateTime
        = new ValueConverter<DateTimeOffset, DateTime>(
            x => x.UtcDateTime,
            x => DateTime.SpecifyKind(x, DateTimeKind.Utc);
}

builder
    .Property(x => x.Timestamp)
    .HasConversion(ValueConverters.DateTimeOffsetToDateTime);

Now, need to investigate the impact of this, including what DateTimeOffset values we're currently storing that might have non-UTC values, and how difficult this will be to migrate.

[JakenVeina]

the EF Core driver for Postgres does not support conversions for the DateTimeOffset

Sorry to break it to you, but this is totally wrong. The driver has no problem whatsoever with converting DateTimeOffset values.

I noticed this while investigating what this would affect in MODiX and saw this in our model snapshot:

b.Property<DateTimeOffset>("Created")
    .HasColumnType("timestamp with time zone");

I went and dropped the HasConversion() annotation in my test project from above, and re-ran everything. DateTimeOffset values are stored and retrieved without data loss, even when different offsets are included in the dataset. Values of ANY offset going into the database always come back as the local machine offset, with the actual timestamp value adjusted accordingly.

I can also run a .GroupBy(x => x.Timestamp) query just fine.

I took a look at the issue linked above, and it looks like the ACTUAL problem is that they don't translate the static members of DateTimeOffset, such as the one I'm gonna be you're trying to use, DateTimeOffset.UtcNow. That should translate into the SQL expression NOW for comparison against the database server's current clock.

If this is the case, the easy workaround is to just put that in a variable first, instead of within the expression, so it gets parameterized. That means the query will be comparing against the application server's current clock, rather than the database, but so long as DateTimeOffset is used, timezone differences will be accounted for automatically. The only issue is then the presence of skew between the two server clocks, but since there's only ever one app server generating timestamp values, this shouldn't matter.

If this isn't the issue with your query, post it here, so we can have a look.

[JakenVeina]

Additional clarification: It's not just static members that Npgsql doesn't translate, it's any DateTimeOffset member calls. The use of .Date is specifically what's been causing Inzanit's queries to fail to translate.

[patrickklaeren]

Sorry to break it to you, but this is totally wrong

Yeah, as you've clarified it looks like it affects any member calls to DateTimeOffset.

[JakenVeina]

Reopening, even though it's resolved for our purposes by #685, we should remove this workaround stuff when https://github.com/npgsql/efcore.pg/issues/473 is resolved.

[patrickklaeren]

As part of a new effort to refocus on priorities, I will close this. If you feel this is imperative to the bot, a new issue can be opened to supersede this.