npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.54k stars 226 forks source link

DateTimeOffset does not mapped correctly to query parameter #303

Closed grayver closed 6 years ago

grayver commented 6 years ago

I have an issue with DateTimeOffset parameter mapping. Here is my model:

    public class ShopPack
    {
        public int Id { get; set; }
        public int ShopItemId { get; set; }
        public decimal Price { get; set; }
        public decimal? StrikethroughPrice { get; set; }
        public bool IsActive { get; set; }
        public DateTimeOffset? ExpiresAt { get; set; }
        public int? Position { get; set; }

        [ForeignKey("ShopItemId")]
        public ShopItem Item { get; set; }
    }

Here is my EF.Core query for this model:

DbContext.ShopPacks.Include(sp => sp.Item)
                .Where(sp => sp.IsActive && (sp.ExpiresAt == null || DateTimeOffset.Now < sp.ExpiresAt))
                .OrderBy(sp => sp.Position)
                .ToListAsync();

Machine, running this .NET code has following timezone: 'Europe/Moscow' PostgreSQL server has following timezone: 'UTC'

According to this mapping table Npgsql should convert DateTimeOffset to UTC locally and then send it to PostgreSQL as timestamptz value. But it doesn't. My models, which have expire time in less than 3 hours, are not selected by the query.

I tried to investigate and collect some logs and here what I found in PostgreSQL logs:

2018-02-14 17:17:08 MSK LOG: execute : SELECT "sp"."Id", "sp"."Count", "sp"."CreatedAt", "sp"."ExpiresAt", "sp"."IsActive", "sp"."LastModifiedAt", "sp"."Position", "sp"."Price", "sp"."ShopItemId", "sp"."StrikethroughPrice", "sp"."SubscriptionDuration", "sp.Item"."Id", "sp.Item"."BasePrice", "sp.Item"."BaseSubscriptionDuration", "sp.Item"."CreatedAt", "sp.Item"."IsSubscription", "sp.Item"."LastModifiedAt", "sp.Item"."Name", "sp.Item"."Title"

FROM "ShopPacks" AS "sp"

INNER JOIN "ShopItems" AS "sp.Item" ON "sp"."ShopItemId" = "sp.Item"."Id"

WHERE ("sp"."IsActive" = TRUE) AND ("sp"."ExpiresAt" IS NULL OR ($1 < "sp"."ExpiresAt"))

ORDER BY "sp"."Position", "sp"."Id" 2018-02-14 17:17:08 MSK DETAIL: parameters: $1 = '2018-02-14 17:17:08.635765'

So, as you can see, DateTimeOffset value is not converted to UTC before sending and that is cause of the problem. I also tried to change DateTimeOffset.Now in query to DateTimeOffset.UtcNow - it helped in that timezone configuration, but what if PostgreSQL server will have another timezone? We have different PostgreSQL servers for different environments and some of them doesn't have 'UTC' timezone.

Is there any solution, which would not depend on timezone configuration of ASP.NET Core server and PostgreSQL?

I'm wondering why this problem exists at all. .NET has special type (DateTimeOffset) to solve timezone confusions. PostgreSQL has special type (timestamptz) to solve timezone confusions. But there are still timezone confusions. How to solve them?

roji commented 6 years ago

Sorry it took me so long to get to this. timestamptz is, frankly, a very problematic type in general that indeed causes a great deal of confusion. Always keep in mind that it does not actually store a timezone - it merely causes PostgreSQL to perform conversions when sending/receiving the types in text mode, a capability which really has little value for O/RM scenarios. I'd recommend it be avoided with EF in general.

Now regarding your specific issue. It's not exactly clear from your logs why you think the conversion isn't happening (especially since you don't post the time at the client side). What I see is that when PostgreSQL logs the timestamptz value ($1 = '2018-02-14 17:17:08.635765'), the absence of a trailing offset probably means that it's displaying it in UTC, since that's what your PostgreSQL server timezone is set to be. Keep in mind that whatever you're seeing in the logs is a textual representation of what PostgreSQL receives from Npgsql, and has therefore already undergone a conversion based on the server's timezone.

The wire representation of a timestamptz (what Npgsql actually sends) is simply a UTC timestamp with no timezone component, and as written above, when asked to send a DateTimeOffset, Npgsql first converts to to UTC and then sends that.

Again, I'd actually recommend dropping timestamptz if possible, switching to timestamp and using UTC everywhere. But if you think you need timestamptz for some reason (it would be interesting to know why, aside from legacy reasons), please try to post a full, clear code sample that illustrates the issue, preferably with fixed values and not DateTimeOffset.Now.

grayver commented 6 years ago

Thank you for detailed answer!

First of all, let me explain why I use DateTimeOffset and timestamptz. Initially, I had DateTime for .NET and timestamp for PostgreSQL. It worked fine on developer machine, where browser, app server and database had same timezone settings. But when we started to use our application in test and beta environment, we met a lot of timezone-related problems. Default Ubuntu and Docker installation of PostgreSQL has UTC timezone. User browsers have different timezones. So conversions from JS to .NET and from .NET to PostgreSQL (and in reverse order) brought many discrepancies.

I examined related issues in Npgsql repository (for example this, this and this) and made a conclusion that I need to move to DateTimeOffset and timestamptz. According to documentation, in this case .NET, PostgreSQL and Npgsql should take care of all timezone conversions. According to this table DateTimeOffset and timestamptz are closest types and conversion between them is most natural.

image

image

So I was sure that this is best type combination and I will never meet any timezone problems again. Now I discouraged to meet again timezone problem and surprised with your advice to get rid of timestamptz.

Now regarding to the specific issue:

It's not exactly clear from your logs why you think the conversion isn't happening (especially since you don't post the time at the client side).

This log was collected on developer machine with 'Europe/Moscow' timezone and as you can see datetime value in parameter matches datetime value in log entry (2018-02-14 17:17:08 MSK DETAIL), so conversion was not performed.

I prepared separate project with this error reproduced here: https://github.com/grayver/pgefcore2-datetime-bug

I get this issue stably with this project on my local developer machine: OS is Windows 7/10 and it has 'Europe/Moscow' timezone Local installed PostgreSQL 9.6 with 'UTC' timezone settings

Please pay attention to create sample data migration - there are 4 records with following ExpiresAt values:

  1. '2018-03-02 16:00:00+03'
  2. '2018-03-02 18:00:00+03'
  3. '2018-03-02 20:00:00+03'
  4. '2018-03-02 22:00:00+03'

Here is query and according to it's logic and sample data it should return 2 records:

            var borderDate = new DateTimeOffset(2018, 03, 02, 18, 00, 00, TimeSpan.FromHours(3));

            return _context.ShopOffers.Include(so => so.Item)
                .Where(so => so.IsActive && (so.ExpiresAt == null || borderDate < so.ExpiresAt))
                .OrderBy(so => so.Position).ThenBy(so => so.Id)
                .ToListAsync();

But it returns only one record (the last one). And here what I see int PostgreSQL log:

2018-03-02 14:36:32 MSK LOG: execute : SELECT "so"."Id", "so"."CreatedAt", "so"."ExpiresAt", "so"."IsActive", "so"."Name", "so"."Position", "so"."ShopItemId", "so"."SubscriptionDuration", "so.Item"."Id", "so.Item"."BasePrice", "so.Item"."BaseSubscriptionDuration", "so.Item"."CreatedAt", "so.Item"."IsSubscription", "so.Item"."Name"

FROM "ShopOffers" AS "so"

LEFT JOIN "ShopItem" AS "so.Item" ON "so"."ShopItemId" = "so.Item"."Id"

WHERE ("so"."IsActive" = TRUE) AND ("so"."ExpiresAt" IS NULL OR ($1 < "so"."ExpiresAt"))

ORDER BY "so"."Position", "so"."Id" 2018-03-02 14:36:32 MSK DETAIL: parameters: $1 = '2018-03-02 18:00:00'

So, datetime value with +03 timezone offset was not converted to UTC.

P.S. Very interesting feature in this issue - it is not reproducable in console application, when I call repository method in sync manner. In that case datetime is converted to UTC and I see '+00' in parameter in PostgreSQL log. But in async ASP.NET Core MVC action it is stably reproducable.

roji commented 6 years ago

First of all, let me explain why I use DateTimeOffset and timestamptz. Initially, I had DateTime for .NET and timestamp for PostgreSQL. It worked fine on developer machine, where browser, app server and database had same timezone settings. But when we started to use our application in test and beta environment, we met a lot of timezone-related problems. Default Ubuntu and Docker installation of PostgreSQL has UTC timezone. User browsers have different timezones. So conversions from JS to .NET and from .NET to PostgreSQL (and in reverse order) brought many discrepancies.

Thanks for providing more detail. I have no idea what kind of troubles you previously had with DateTime and timestamp, but I will say one thing - unless you have a very good reason to, it's a very good idea to have UTC timestamps everywhere, and only convert to a non-UTC timezone when displaying timestamps to users. In other words, keep all your timezone conversions at the very edges of your application, as part of the user display, much like you'd render an integer value to a culture-sensitive representation only at the display layer (e.g. 1,000 vs. 1.000).

It sounds from the above that this was not the case. I'm really not sure what in the docs or the issues you cited convinced you to move to timestamptz and DateTimeOffset - it would be good to know. But unless you have some very special circumstances it is definitely not a good way forward - it means you're inserting timezone conversions and awareness in various places in your stack - your database now starts to be timezone-aware (based on its own timezone and your client's PG session's timezone, all of which is already non-trivial to manage), your database driver (Npgsql) has to deal with it, your .NET application does it, etc. The machine timezone in various different machines also starts to matter (db server, app server...). I strongly recommend that if you want to maintain your sanity, switch back to DateTime (Kind=UTC) and timestamp, make sure that you have UTC absolutely everywhere, and do timezone conversions only at the very edges, e.g. right before displaying to the user.

Regarding the actual issue, I'll be taking a look soon to understand exactly what's going on. One quick note though... When you say the following:

Please pay attention to create sample data migration - there are 4 records with following ExpiresAt values:

'2018-03-02 16:00:00+03' '2018-03-02 18:00:00+03' '2018-03-02 20:00:00+03' '2018-03-02 22:00:00+03'

You should understand what's really going on - the above aren't raw values. In your database, timestamptz doesn't hold an offset, it only holds a timestamp (see warning in http://www.npgsql.org/doc/types/datetime.html). PostgreSQL assumes that the timestamp is UTC, and performs timezone conversion when converting to text. So if you connect to your PostgreSQL with psql (or pgadmin) and select those records, PostgreSQL converts to offset +03 based on your client session setting (which can be changed via set TimeZone). The values themselves, inside the database, have no offset (that's exactly the type of confusion I recommend you avoid by switching to timestamp).

P.S. Very interesting feature in this issue - it is not reproducable in console application, when I call repository method in sync manner. In that case datetime is converted to UTC and I see '+00' in parameter in PostgreSQL log. But in async ASP.NET Core MVC action it is stably reproducable.

I can't think of anyway where the application type (console vs. ASP.NET Core) could have any impact on this. Are you running your console application on the same server as where you're running the ASP.NET Core MVC? If not it's very likely that the different machines have a different timezone or something similar, and that explains the difference.

grayver commented 6 years ago

You should understand what's really going on - the above aren't raw values. In your database, timestamptz doesn't hold an offset, it only holds a timestamp

I understand. I insert those values in text query, so PostgreSQL automatically converts them to UTC.

Are you running your console application on the same server as where you're running the ASP.NET Core MVC? If not it's very likely that the different machines have a different timezone or something similar, and that explains the difference.

Yes, I'm running console and ASP.NET Core MVC on the same developer machine.

roji commented 6 years ago

OK, I'll try to take a closer look in the coming days, but please consider switching to a simple UTC-everywhere architecture with timestamp and DateTime, unless you have a really good reason not to (I haven't seen one yet).

grayver commented 6 years ago

Ok, I'll try to describe my thoughts and reasons about DateTime\timestamp switching:

  1. I decided to use DateTimeOffset to get rid of any ambiguity of time points on application level. After reading mentioned issues I decided to examine Microsoft recommendations. Microsoft advices here to use DateTimeOffset to "Uniquely and unambiguously identify a single point in time" and "Perform general date and time arithmetic". Of course, we could use DateTime with UTC kind, but then every developer developing our application should follow this convention. This way increases probability of human mistakes.

  2. You write "make sure that you have UTC absolutely everywhere". What does it mean? Should PostgreSQL have timezone setting set to "UTC"? Should application server which hosts ASP.NET Core application have timezone "UTC"? If yes - then it is impossible. We have many environments (dev, test, beta, prod) which use different machines in different places. Every machine has many applications and we can't change it's settings for the sake of one application. And I suppose it is not a good practice - changing global environment settings for some local needs. Good application should adapt for existing environment settings.

  3. Managing data in database. In case "UTC everywhere" database administrator should always keep in mind, that all datetime values in database are stored in UTC. So every query he writes to manage data should take this into account. Again, this increases probability of human mistakes.

  4. I reviewed again datetime conversion table between PG and .NET. I'm confused with this row: image Does it mean, that I need to manually set Kind=UTC for each DateTime which I read from database in case I switch to DateTime\timestamp?

roji commented 6 years ago

Here are some answers, not in the same order as the questions.

  1. You write "make sure that you have UTC absolutely everywhere". What does it mean? Should PostgreSQL have timezone setting set to "UTC"? Should application server which hosts ASP.NET Core application have timezone "UTC"? If yes - then it is impossible. We have many environments (dev, test, beta, prod) which use different machines in different places. Every machine has many applications and we can't change it's settings for the sake of one application. And I suppose it is not a good practice - changing global environment settings for some local needs. Good application should adapt for existing environment settings.

My answer to this has two parts...

First, and that's a very important point: you don't necessarily have to configure PostgreSQL or your servers to UTC. What I'm suggesting is that your PostgreSQL and machine timezones have absolutely no effect on your application. At no point should your application look at the local machine's timezone, nor should that timezone be a part in any part of an application's timestamp operation. Anything else exposes your application to changes by a completely arbitrary parameter outside its control - the location where some server happens to be located. If you move some component from a data center in Europe to the US, your application starts behaving differently - not an amazing situation.

The second part of my answer is that yes, I'd strongly recommend configuring your servers in UTC (although that's beyond the scope of this issue). I'd suggest that your read this rant, which forces this point a little :) I indeed configure all my servers around the world to be UTC; this prevents time jumps because of daylight saving changes (oh the agony), allows them to talk to each other transparently without having to worry about conversions, and allows me to copy log files without having to start thinking where they came from when examining timestamps. And in any case, if you're displaying timestamps to users, it's those users' timezones which matter, and not the geographical location of the servers they're talking to.

At the end of the day, timezones are something that matters to humans, not to computers - so it's generally best to keep them out of application altogether, except for display/input purposes (of course this is a generalization, and specific applications may have different needs).

  1. I decided to use DateTimeOffset to get rid of any ambiguity of time points on application level. After reading mentioned issues I decided to examine Microsoft recommendations. Microsoft advices here to use DateTimeOffset to "Uniquely and unambiguously identify a single point in time" and "Perform general date and time arithmetic".

Ahh, that's quite a debate. First I recommend you take a long look at the Nodatime library and read on why it exists; the next version of Npgsql will optionally support Nodatime types instead of the built-in .NET types (#546).The basic .NET types (DateTime, DateTimeOffset) are actually flawed in many ways and promote some bad programming by their ambiguities - this is what Nodatime attempts to fix.

I'd have to know more about what your application does, but you have to ask yourself one important question... When you're storing your timestamp in the database, is that simply a moment that occurred at some point, and which, when displayed, needs to be shown in the user's timezone? In other words, is the offset or timezone (more on that later) part of what it is that you're storing, or does it just represent an instant in the "global timeline", regardless of which timezone a particular observer is in? Another way to look at it, is to ask yourself whether it's conceivable that the same timestamp would have to be displayed to different users, and therefore in different timezones?

In the vast majority of the applications I've seen, timestamps represent simple instants in time, and therefore should be represented in something like milliseconds since the Unix epoch, or a DateTime (which is basically the same). No timezone or offset is necessary or advisable, as it can only cause confusion wherever the value needs to be interpreted.

Regarding the Microsoft docs... How is DateTimeOffset better at uniquely an unambiguously identifying a single point of time, more than a simple Unix-epoch timestamp or a UTC DateTime? In fact, two different DateTimeOffset can represent the same instant, simply because they have two offsets (14:00 at offset +1 or 15:00 at offset +2). When the Microsoft docs talk about a "single point of time", they're referring to a user's local time, i.e. 14:00 in England - that's very rarely something that should be stored somewhere (as opposed to displayed). Note also that the docs do state that DateTime is fine as long you're representing UTC, and that arithmetic works too - why wouldn't it.

Once again I recommend reading this Nodatime document, which explains the concepts above. Note also that DateTimeOffset has an offset, but an offset is not a timezone. Timezones also have daylight saving, so a simple offset is simply insufficient to fully display timestamps to users - it will sometimes work, and sometimes not. That's just one flaw in the BCL types, and that alone makes it unusable for many (most?) purposes (this is why the Microsoft docs state that "it is not a fully time zone-aware data structure") .

Of course, we could use DateTime with UTC kind, but then every developer developing our application should follow this convention. This way increases probability of human mistakes.

Of course all developers on your application have to agree and work together... Would you want one developer to encode strings in UTF8 and another in UTF16, or one to work in C# and another in Java?

The reality is actually the opposite - UTC everywhere reduces the probability of mistakes and general confusion. If you're storing your timestamps as PostgreSQL timestamptz, then it's your database's configuration that determines timestamp conversion, inside your database. This depends on the settings inside your PostgreSQL config file, your session's TimeZone parameter... You're effectively distributing timezone logic all around your application and infrastructure.

  1. Managing data in database. In case "UTC everywhere" database administrator should always keep in mind, that all datetime values in database are stored in UTC. So every query he writes to manage data should take this into account. Again, this increases probability of human mistakes.

That's the same objection as the one made above (1) for developers. Yes, projects have rules that need to be followed, otherwise things turn into a catastrophic mess, and telling everyone they can use whatever timezone they want will not reduce mistakes. But more importantly, you may be forgetting that PostgreSQL's timestamptz really does store UTC - there is no timezone or offset in the database column. timestamptz just relies on PostgreSQL to perform incoming and outgoing conversions based on its configuration and session, which frankly makes absolutely no sense in the majority of cases...

  1. I reviewed again datetime conversion table between PG and .NET. I'm confused with this row: Does it mean, that I need to manually set Kind=UTC for each DateTime which I read from database in case I switch to DateTime\timestamp?

That's a good question. When you use PostgreSQL timestamp, there's nothing in the database saying that the value is in UTC, and so Npgsql returns that as a DateTime with kind Unspecified. This doesn't really matter to anything we've discussed above. As long as your application treats timestamps coming from PostgreSQL as UTC, it doesn't really matter if the DateTime kind is Utc or Unspecified - simply treat them as UTC, always, everywhere - no need to convert between servers or applications, no need to reason about daylight savings time.

To continue this discussion I'd need a bit more detail on how your application interacts with timestamps. Where do timestamps come from, what is done with them?

grayver commented 6 years ago

Wow, thanks for detailed and impressive answer. I've read mentioned articles about NodaTime and "Use UTC everywhere" manifest. Here are my thoughts and answers:

Of course all developers on your application have to agree and work together... Would you want one developer to encode strings in UTF8 and another in UTF16, or one to work in C# and another in Java?

Sorry, I haven't meant not to have any convention at all. I meant - if we have a chance to choose from convention and restriction, then last one should be preferred. If a human has a chance to make a mistake, then it happens one day. One day developer will use DateTime.Now insted of DateTime.UtcNow and this would cause a bug in application. I would prefer automatic DateTime handling (with DateTimeOffset either with NodaTime) over manual. Of course, only if it works correctly and covers all corner cases. According to provided articles, DateTimeOffset doesn't cover all corner cases.

The second part of my answer is that yes, I'd strongly recommend configuring your servers in UTC (although that's beyond the scope of this issue).

Unfortunately, it's not always possible. Due to administrative policy and other applications working on the same server (or database).

In the vast majority of the applications I've seen, timestamps represent simple instants in time, and therefore should be represented in something like milliseconds since the Unix epoch, or a DateTime (which is basically the same). No timezone or offset is necessary or advisable, as it can only cause confusion wherever the value needs to be interpreted.

Yes, my application also uses simple instants. There is no need to store timezone.

Where do timestamps come from, what is done with them?

In most cases application server generates them with trivial datetime arithmetics. Also there are functions in PostgreSQL which contain timestamp comparision logic (usually comparing with CURRENT_TIMESTAMP) and INTERVAL type arithmetics. In very rare cases administrative users can input some datetime data via web forms (Angular 4 + ASP.NET Core).

roji commented 6 years ago

Of course all developers on your application have to agree and work together... Would you want one developer to encode strings in UTF8 and another in UTF16, or one to work in C# and another in Java?

Sorry, I haven't meant not to have any convention at all. I meant - if we have a chance to choose from convention and restriction, then last one should be preferred. If a human has a chance to make a mistake, then it happens one day. One day developer will use DateTime.Now insted of DateTime.UtcNow and this would cause a bug in application.

That's a valid point, you're right that accidental usage of DateTime.Now can be dangerous. This kind of problem can be addressed with analyzers - take a look at this article, there's also a suggestion here, reiterating the idea that any sort of timezone awareness in a server application is a bad idea.

This kind of mistake is also prevented when using Nodatime, since you don't have a single kind that can mean both a timezone-independent instant in time and a zoned timestamp.

I'll just repeat myself by saying that using DateTimeOffset everywhere introduces its own set of risks of programmer mistakes - the conversions that have to happen at the edge of every system (database to application, between different servers...) increase the risk that at some point somebody will badly convert (e.g. forget the offset or similar). With UTC everywhere you're just passing around the same value, having the same meaning, absolutely everywhere.

I would prefer automatic DateTime handling (with DateTimeOffset either with NodaTime) over manual. Of course, only if it works correctly and covers all corner cases. According to provided articles, DateTimeOffset doesn't cover all corner cases.

I'm still not sure what "automatic DateTime handling" means, or what you expect DateTimeOffset or Nodatime to do for you. Either an offset/timezone is relevant for your server application (which it isn't in 99% of cases) or it isn't - if it's the latter you shouldn't be reasoning about it. Nodatime fixes things by having a much more sane type hierarchy which separates Instants (points in a global timeline regardless of observer timezones) from types like ZonedDateTime; the .NET types mix things up, create a lot of ambiguiity and thus encourage bad programming practices.

Aside from the legitimate risk discussed above of a programmer calling DateTime.Now instead of DateTime.UtcNow, I really can't see the advantage of moving an offset/timezone around in your application and having to convert values everywhere.

The second part of my answer is that yes, I'd strongly recommend configuring your servers in UTC (although that's beyond the scope of this issue).

Unfortunately, it's not always possible. Due to administrative policy and other applications working on the same server (or database).

I understand, that's true in many cases. I would still try to push moving to UTC on your servers - for your administrators' own good - but the least you can do is isolate your application from whatever timezone your server happens to be configured to.

In the vast majority of the applications I've seen, timestamps represent simple instants in time, and therefore should be represented in something like milliseconds since the Unix epoch, or a DateTime (which is basically the same). No timezone or offset is necessary or advisable, as it can only cause confusion wherever the value needs to be interpreted.

Yes, my application also uses simple instants. There is no need to store timezone.

So I'd really recommend you use a type that corresponds to that. I'd also recommend considering using Nodatime for your timestamp processing. It's true that currently you'd have to convert DateTime values coming from Npgsql to Nodatime Instants, but in the next version of Npgsql you'll have the option of reading Nodatime types directly.

Where do timestamps come from, what is done with them?

In most cases application server generates them with trivial datetime arithmetics. Also there are functions in PostgreSQL which contain timestamp comparision logic (usually comparing with CURRENT_TIMESTAMP) and INTERVAL type arithmetics. In very rare cases administrative users can input some datetime data via web forms (Angular 4 + ASP.NET Core).

OK, so that sounds like a quite standard application. As I wrote above I'd consider using Nodatime (this is pretty much my recommendation for almost any app doing date/time logic in .NET), but at the very least I'd switch to timestamp and DateTime.

There's still an issue about the actual behavior of DateTimeOffset and timestamptz (proper conversion when writing), so I'll keep this open and try to look soon.

grayver commented 6 years ago

Is there any update?

roji commented 6 years ago

Sorry, no, I didn't have time to look at this at all with the big 4.0 release... Note that we did do some changes to date/time handling for 4.0: DateTime is now always sent as timestamp and DateTimeOffset is always sent as timestamptz (#1940), and in addition the NodaTime plugin has been released.

Can you please confirm that you're still encountering the problematic timestamptz behavior with the new version 4.0?

grayver commented 6 years ago

Wow, I just upgraded my reproduce project to .NET Core 2.1 + Npgsql 4.0 and problem has gone. Query now returns 2 records and log entry contains correct parameter value with timezone:

2018-06-04 00:44:22 MSK DETAIL:  parameters: $1 = '2018-03-02 15:00:00+00'

It seems to be this issue was somehow fixed with your refactoring in 4.0, so you can close it.

roji commented 6 years ago

Ok, great!

sguryev commented 6 years ago

Hi guys, thank you for such long and detailed conversation.

I have another question which I was not able to resolve. I have enabled the warning (RelationalEventId.QueryClientEvaluationWarning) in the EF to raise the exception in case of local evaluation. You can read more about it here: https://docs.microsoft.com/en-us/ef/core/querying/client-eval. I'm running latest .netcore 2.1 with Npgsql.EntityFrameworkCore.PostgreSQL 2.1.1 referencing Npgsql 4.0.1

And I'm receiving InvalidOperationException on any DateTimeOffset filter queries. With EF.Functions or without them.

'Error generated for warning 'Microsoft.EntityFrameworkCore.Query.QueryClientEvaluationWarning: The LINQ expression 'where ([u].DateTimeOffsetNull > Convert(DateTimeOffset.UtcNow, Nullable`1))' could not be translated and will be evaluated locally.'. This exception can be suppressed or logged by passing event ID 'RelationalEventId.QueryClientEvaluationWarning' to the 'ConfigureWarnings' method in 'DbContext.OnConfiguring' or 'AddDbContext

image

I would like to know if there any way to execute such filtering queries on the server side? Please note that we have been working with the MSSQL for several months. And now we would like to move to the PostgreSQL for one of the clients. We expected pretty easy migration process with leaving same code base and just migration recreation process.

I saw you answer saying:

Always keep in mind that it does not actually store a timezone - it merely causes PostgreSQL to perform conversions when sending/receiving the types in text mode, a capability which really has little value for O/RM scenarios

I'm concerned that such value-text-value conversion doesn't support any filtering at all.

@grayver what do you do with the client-side evaluation? Kindly mentioning you @roji

Looking forward for your response.

roji commented 6 years ago

@sguryev right now DateTimeOffset's members ~are~ aren't translated, so evaluation happens on the client. This is unlike DateTime which does translate. #473 tracks translating DateTimeOffset's members.

timbze commented 2 years ago

Not trying to resurrect an old issue, but I was linked here from another place, and this seems like good information for someone seeking info like me... until I find that it seems the timestamp vs timestamptz info on this thread is no longer valid, according to the release notes for pgsql 6.0: https://www.npgsql.org/doc/release-notes/6.0.html#major-changes-to-timestamp-mapping

roji commented 2 years ago

@timbze that's correct - this issue is very old and Npgsql's timestamp handling has changed in a significant way in 6.0, as you wrote.