Range function resolution fails for column (tstzrange) and parameter (tsrange)

[dpsenner]

With the following database table:

create table public.some_record (
  during tstzrange
);

The following model:

public class SomeRecord
{
    public NpgsqlRange<DateTime> During { get; set; }
}

causes the following exception on insert:

2018-08-02 15:36:52.732 +02:00 [Error] [8504472] [14] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("17"ms) 
[Parameters=["@p0='[2018-08-02 15:36:51,)' (DbType = Object)"], CommandType='Text', CommandTimeout='600']"
""INSERT INTO public.some_record (during)
VALUES (@p0);"
Npgsql.PostgresException (0x80004005): 42804: column "during" is of type tstzrange but expression is of type tsrange
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 444
   at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1219
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1130

[austindrenski]

@dpsenner This is by design. NpgsqlRange<DateTime> is default mapped to tsrange, though other mappings also exist.

You should be able to configure SomeRecord.During to use tstzrange explicitly (e.g. annotations or the fluent API).

Could you give that a try and let us know how it goes?

[austindrenski]

@dpsenner We are also strongly encouraging users to take a look at our plugin for NodaTime which provides a significant improvement in date and time handling.

[dpsenner]

Thanks for having a look. We attempted to change our database models to use nodatetime Instant's but that works for tsrange while it shows the same symptoms for tstzrange. The query parameter is passed in as follows:

[2018-08-02T15:49:28Z,2018-08-02T15:54:28Z]' (DbType = Object)

and causes the same exception:

Npgsql.PostgresException (0x80004005): 42804: column "during" is of type tstzrange but expression is of type tsrange
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 444
   at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1219
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1130
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)

I'm going to give your suggestion a shot tomorrow.

[dpsenner]

I just gave the column type a shot nonetheless and it improved the situation somewhat:

builder.Property(t => t.During)
       .IsRequired()
       .HasColumnName("during")
       .HasColumnType("tstzrange");

The original was:

builder.Property(t => t.During)
       .IsRequired()
       .HasColumnName("during");

Now values can be inserted and fetched from the database, but querying with a where clause does not work:

Instant dateTimeOffsetInRange = Instant.FromDateTimeOffset(DateTimeOffset.Now);

List<SomeRecord> userTransponderRecords = 
    DbContext.SomeRecords
             .Where(t => t.During.Contains(dateTimeOffsetInRange))
             .ToList();

and produces the following stacktrace:

2018-08-02 17:57:58.799 +02:00 [Error] [6418856] [13] [] [] [:] [Microsoft.EntityFrameworkCore.Database.Command] Failed executing DbCommand ("8"ms) [Parameters=["@__dateTimeOffsetInRange_0='2018-08-02T15:57:56Z' (DbType = DateTime)"], CommandType='Text', CommandTimeout='600']"
""SELECT t.during
FROM public.some_record AS t
WHERE (t.during @> @__dateTimeOffsetInRange_0) = TRUE"
Npgsql.PostgresException (0x80004005): 42883: operator does not exist: tstzrange @> timestamp without time zone
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlConnector.<>c__DisplayClass161_0.<<ReadMessage>g__ReadMessageLong|0>d.MoveNext() in C:\projects\npgsql\src\Npgsql\NpgsqlConnector.cs:line 1032
--- End of stack trace from previous location where exception was thrown ---
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming) in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 444
   at Npgsql.NpgsqlDataReader.NextResult() in C:\projects\npgsql\src\Npgsql\NpgsqlDataReader.cs:line 332
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1219
   at Npgsql.NpgsqlCommand.ExecuteDbDataReader(CommandBehavior behavior) in C:\projects\npgsql\src\Npgsql\NpgsqlCommand.cs:line 1130
   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)

[austindrenski]

@dpsenner Yes... we've seen this issue elsewhere when passing parameters that need non-default mappings.

I'll see what we can do to detect/add type casts to the SQL generation phase.

In the meantime, if this is a showstopper, my preferred workaround is to define a custom overloaded operator to patch the type resolution:

CREATE TYPE example_enum AS ENUM ('1', '2', '3', '4');

--
-- overloading the equality operator
--

CREATE FUNCTION example_enum_as_text(example_enum, TEXT) RETURNS BOOLEAN AS
$$ SELECT $1 :: TEXT = $2 :: TEXT; $$
LANGUAGE SQL IMMUTABLE RETURNS NULL ON NULL INPUT;

CREATE OPERATOR = (
  procedure = example_enum_as_text,
  leftarg = example_enum,
  rightarg = TEXT,
  HASHES,
  MERGES);

--
-- overloading the equality operator (flipped)
--

CREATE FUNCTION example_enum_as_text(TEXT, example_enum) RETURNS BOOLEAN AS
$$ SELECT $1 :: TEXT = $2 :: TEXT; $$
LANGUAGE SQL IMMUTABLE RETURNS NULL ON NULL INPUT;

CREATE OPERATOR = (
  procedure = example_enum_as_text,
  leftarg = TEXT,
  rightarg = example_enum,
  HASHES,
  MERGES);

Related

#428
dotnet/efcore#4978,

[roji]

@austindrenski corrected analyzed this, EF Core is indeed missing a way to explicitly specify the store type for cases of non-default mappings.

However, with NodaTime you may be able to work around this... Instead of Instants you may try using OffsetDateTime (or ZonedDateTime), which we map to timestamptz by default. This is a pretty hacky workaround until dotnet/efcore#4978 gets resolved.

[roji]

Finally, as @austindrenski wrote above, make sure you know exactly why you want to use timestamp with time zone - it's a really problematic database type, and most usages I've seen don't really make sense (timestamp without time zone should be used instead).

[roji]

Am going to close this as there's nothing more to do in Npgsql (although the discussion can definitely continue).

[dpsenner]

Thanks to both of you again for looking into this. Unfortunately, @roji, neither OffsetDateTime, ZonedDateTime or LocalDateTime can be used in the model because Instant is the only class that implements IComparable<T> and therefore works in combination with NpgsqlRange<T> when writing a simple query like:

var instant = ...
var = DbContext
	.SomeRecords
	.Where(t => t.During.Contains(instant))
	.ToList();

@austindrenski @roji Would you please share with us your insights on these datatypes? Under what circumstances have you learned that the timezoned data types are problematic and in what respect are they problematic to you? Have you worked out guidelines when to use either datatypes with timezone or without?

To me it is clear that either datatypes store information as utc in the database and timezones affect only the behavior of how data is read from and written to the database store. We delibaretly chose to use the timezoned datatypes because it provides autoconversion from and into the database clients timezone. In the first version we explicitly implemented all database model properties by using DateTimeOffset. This way, whoever works with the models knows that the timestamp given is offset from utc into the timezone of the machine where the application runs on. At the same time the npgsql provider (or the database) transforms both utc timestamps and timestamps that have a timezone offset to utc when writing them into the database. On top of that, when such timestamps are logged they can easily be correlated with the time on the machine where the application runs on without manual timezone conversions all over the place.

I hope to gain some insights from your valuable input that either shows that our findings were valid but cannot be implemented currently because dependencies are not ready for this yet. Or it makes us reconsider this decision and we exclusively use timestamps without timezone and implement conversions in the backend whereever a functionality forces us to do so.

[YohDeadfall]

@dpsenner You can find the answer and examples in npgsql/npgsql#1803. The long story short, DateTime couldn't be unambiguously mapped to PostgreSQL date time types because of the Kind property.

[roji]

@dpsenner as I wrote in #63, if you want to know what I think about using timezone-aware types (and PostgreSQL timestamp with time zone), take a look at #303 and #473, but prepare yourself for a long and exhausting rant :) Below is an attempt at a compacted version.

PostgreSQL timestamp with time zone is quirky in that timezone conversions are introduced (at least potentially) when your application reads/writes timestamps from/to the database. Sit down and think if you really want this in a sane application (especially if you believe in the UTC-everywhere pattern).

Regarding DateTimeOffset... If you want to be UTC everywhere (and in most cases it's a very good idea), then I can't see any sense in having an offset alongside your timestamps in your entire application, which is what DateTimeOffset does. Once there's an offset, it's possible for that offset to be non-zero, and then any code that looks at a property of your DateTimeOffset (e.g. .Hour) will get skewed. In that sense DateTimeOffset doesn't really offer any protection against programmer error compared to DateTime - if you happen to have a non-UTC timestamp (offset != 0 for DateTimeOffset, kind != UTC for DateTime), your code wlil stop working. If you want to be UTC everywhere, you do not want any offset.

The best practice (IMHO) is to make sure any and all timezone conversions happen at the very edge of your application, e.g. when rendering a timestamp for user display based on that user's browser's configured timezone. Any incoming timestamps should be normalized to UTC timestamps as soon as possible, and the rest of your code should simply work with the assumption that they are UTC.

This isn't to say that DateTime is a great type - both DateTime and DateTimeOffset are quite bad. I'd recommend you take a look at NodaTime which clearly distinguishes between Instant - which is a simple point in time without a timezone - and LocalDateTime/ZonedDateTime which deal with human-intelligible concepts such as calendars, timezones and all the rest.

[austindrenski]

@roji For clarity to future readers:

If we were in the business of dictating schema redesigns, would it be your recommendation that @dpsenner change during from tstzrange to tsrange, and refactor away from both DateTime and DateTimeOffset?

[roji]

@austindrenski it's hard to make sweeping recommendations without taking account any specific needs etc.

But yes, my personal opinion is that in the general case, unless your application has special needs and cannot be UTC-everywhere:

• Avoid using DateTimeOffset.
• If at all possible, use NodaTime, which makes most of these issues disappear.
• If NodaTime isn't an option, use DateTime with Kind=UTC and make sure all timestamps are properly normalized/converted to UTC at the very edge.
• Assume timestamps in your application are UTC, and add assertions in critical places to ensure this is so.
• Consider using an analyzer to further enforce UTC DateTime (e.g. disallow DateTime.Now).

Note that this is my personal take on things and I'm sure many would disagree.

[dpsenner]

Incidentally I came to the same conclusions. If team lead agrees we are going to set sails with nodatime instants, tsrange and timestamp without timezones. Where required we will introduce additional columns for client local times and/or the timezone id of the client that created or updated that specific entity.

[roji]

@dpsenner good to hear it, not everyone agrees with my views on date/time handling :)

Note that if you want to represent the client's timezone inside your database, you need a separate column in the database no matter what, since timestamp with time zone doesn't actually store a timezone. This is true whether you go with NodaTime or not.

Another important point is that DateTimeOffset does not contain a timezone - only an offset. Many people confuse the two, and end up storing a DateTimeOffset in SQL Server thinking that they've resolved the question. However, a timezone also has daylight saving rules, which can be very significant and are completely ignored when all you have is an offset. That's another disadvantage of DateTimeOffset - it gives you the illusion of having taken care of the problem, whereas in fact you haven't. In that sense I prefer PostgreSQL which doesn't have an actual "timestamp with offset" type, but rather forces you to think about things and represent the timezone in a separate column if that's what you really want.

@austindrenski have we actually done any change in this issue? If not can you please remove the label and milestone?

[dpsenner]

@roji, that's what we went through when we discussed the proposal. :-) It took a significant amount of arguments until the underlying problems finally were understood by everyone and only then we were able to come to an agreement. Unfortunately we do also have the functional requirement to track the original time in the timezone when something was made or done. Currently we have three possible solutions for this and I would like to know what you think about these:

1. store a utc timestamp into a timestamp (without timezone) and the timezone id (i.e. Europe/Rome) where that instant came from
2. store a zoned datetime (not utc) into a timestamp (without timezone) column along with the timezone id (i.e. Europe/Rome) where that instant came from
3. store both utc into a timestamp, a zoned datetime into a timestamp and the timezone id (i.e. Europe/Rome) where that instant came from

We have also discussed to store the offset (in hours and minutes), but we are yet unsure about the implications and whether we actually should do so. The reasoning behind the previously mentioned options is that we surely need a system time that every participant can agree with. But we do also need the information of a local timestamp that easily allows to correlate "4am in america" with "4 am in europe" and "4 am in china" which is not possible with utc and the timezone id. Daylight savings would for instance shift the timestamps during summer and "4 am in europe" would suddenly become "3 am in europe".

[Brar]

store a utc timestamp into a timestamp (without timezone) and the timezone id (i.e. Europe/Rome) where that instant came from

IMO this is the safest and most useful option.

An UTC timestamp is a unique moment in time that is the same in every place of the world and completely independent of the time zone or whether daylight saving time is in effect.
If you know and save the time zone where your user is located you can always calculate the time that was displayed on his clock in that moment (even if the government decides to dismiss daylight saving time or change the time zone).

timestamp without time zone doesn't do any magic to your timestamp so it's the best representation for UTC timestamps.

[roji]

Thanks for the interesting discussion. I agree with what @Brar wrote above, here's some more detail from my side.

Your first two options (if I understand them correctly) seem very similar - in both cases you have one column of type timestamp without time zone and one column of type text, for the timezone (e.g. Europe/Rome). The only difference is whether what you put in the timestamp column is a UTC timestamp or a timestamp local to the timezone specified in the other column. As @Brar said above, I'd go with option 1, first because it allows you to unambiguously read the UTC timestamp without looking at the timezone, assuming you have some places in the code where that's relevant. In other words, for cases where a UTC timestamp is enough, you have that in a simple column, and for cases where you need the original timezone, you have that as well.

But there's one more reason to prefer storing UTC. Timezones actually change from time to time: a country in GMT+2 can decide to become GMT+1 tomorrow (this is rare but it happens). So if you store a local timestamp in the database and the timezone changes, well, you're kinda screwed :) If you store UTC and only convert to local timestamps in your program when needed, then as long as your timezone database is up to date you'll always be OK.

I frankly don't see any added value to the 3rd option - it's needless duplication (and room for error). Once you have a (UTC) timestamp and a timezone, you can always easily convert back and forth, from UTC to local (according to the stored timezone) and back.

We have also discussed to store the offset (in hours and minutes), but we are yet unsure about the implications and whether we actually should do so. The reasoning behind the previously mentioned options is that we surely need a system time that every participant can agree with. But we do also need the information of a local timestamp that easily allows to correlate "4am in america" with "4 am in europe" and "4 am in china" which is not possible with utc and the timezone id. Daylight savings would for instance shift the timestamps during summer and "4 am in europe" would suddenly become "3 am in europe".

Here I'm a bit confused. First, "4AM in America" is a time with timezone, not a timestamp (there is no date). Once you put a date in there, "4AM on 1/1/2018 in EST", daylight savings no longer shifts things around, because a given date was always either in daylight savings or not... I also don't see the point of storing the offset (hours, minutes) - you're already storing the timezone (as an ID), which is much better.

To sum it up, just use UTC timestamps everywhere, all the time. If you have to know of a user's preferred timezone (for display purposes or anything else), store that separately and use that to translate the UTC timestamp as required, only where required. In NodaTime terms (and I definitely recommend you use NodaTime here), you're storing an Instant (globally agreed-upon point in time, regardless of calendar/timezone/human representation), plus a timezone. Whenever relevant, you can put the two together to create a ZonedDateTime.

[dpsenner]

The reasoning behind two timestamp columns was, that when a user reads that timestamp today it says "4 am" but if daylight savings change overnight, tomorrow he will read the same timestamp as "3 am". Having a local time stored explicitly will make that record "4 am" forever, which is also what the user would expect. Please correct me if I'm wrong.

[roji]

@dpsenner unless I'm totally mistaken about daylight savings (DST), every time zone has a DST definition: DST starts at a certain date, and stops at another date. When you read a UTC timestamp from the database (let's call it Instant) and convert it to a ZonedDateTime, NodaTime (or whoever) will consult their timezone database to see if DST is on for the date of the timestamp.

Long story short, date/time conversions aren't affected by the current timestamp - if DST changes between today and tomorrow, you will see the same thing. The only thing that matters is the timestamp itself, and the timezone.

[roji]

I suggest you do a quick test to confirm the above... Write a trivial console program that uses NodaTime, and see what happens as you play around with your machine's date (in and out of DST). You shouldn't see any change in behavior in the program.

[Brar]

@dpsenner converting an UTC timestamp to a local timestamp is more than just adding an offset in hours.
There are changes in the assignment of a country to a specific time zone, introduction or dismissal of daylight saving time in a country and other weird things that might happen and be set in effect at a specific timestamp due to government decisions.

It's best to rely on tools that are backed by time zone databases that have all the knowledge of those details and do the heavy lifting for you.

TimeZoneInfo Methods are probably the most natural way for .Net but there is also https://github.com/LZorglub/TimeZone which uses the IANA time zone database (I haven't tried it).
The idea is to convert the timestamp back to its original value before displaying it to the user

[austindrenski]

Not to distract from the wider discussion, but while every time zone has a definition of DST, not every locale in the time zone necessarily observes it:

https://www.timeanddate.com/time/us/arizona-no-dst.html

[roji]

To supplement what @Brar said above, NodaTime also uses the IANA timezone database, which is pretty much the standard outside of the Windows world. The advantage of NodaTime is a sane date/time type hierarchy along with the timezone database - basically everything is taken care of for you.

Not to distract from the wider discussion, but while every time zone has a definition of DST, not every locale in the time zone necessarily observes it:

Bah, date/time insanity... That's why it should all be delegated to a library which actually knows about all that complexity...

[dpsenner]

Does anyone happen to know how the timezone database is updated in the postgres database? The documentation for Nodatime makes this clear, but I can't find any resources that document where postgres takes its timezone database from. On unix it'll use tzdata, but what does it do on windows? Is there an api to know which timezone database version is currently in effect? I ask because I'd like to evaluate how we could synchronize the timezone database such that we have a plan in case the day comes where the timezone databases used by postgres and the application disagree about how a specific timezone id works.

[roji]

At least if I'm understanding you're scenario properly, you're not supposed to care about PostgreSQL at all, since you'll never be asking or to do any sort of conversions for you. This is precisely why you should avoid timestamp with time zone... just use UTC timestamps in the database, and if your really need to, a string code representing a timezone. Then, in your application you do any conversions if you need to.

Don't involve the database in any timezone-aware logic unless you need to - I'm not saying it should never happen, just that you want to avoid it if at all possible.

[austindrenski]

Does anyone happen to know how the timezone database is updated in the postgres database?

Not sure if there's anything more authoritative, but it's discussed here:

https://github.com/postgres/postgres/blob/master/src/timezone/README

[dpsenner]

You've got a point. There are however several database functions that return timestamptz datatypes and there are therefore scenarios where the query generator causes bad timestamps to be read from the database. NOW() is one of them. As a minimal bad example, a linq query containing DateTime.Now translates to a database query that contains NOW(). That causes a timestamptz column to be generated at runtime. This would result in timestamptz values to be materialized into Nodatime Instants with a timestamp that is not utc at all. To defend against this scenario I just hard-coded the connection string to contain Timezone=UTC which should ultimately override the timezone behavior. I'm not sure if that's enough as a solution. Do you know of any other pitfalls?

[Brar]

NOW() is one of them. As a minimal bad example, a linq query containing DateTime.Now translates to a database query that contains NOW()

Are you aware of DateTime.UtcNow in .Net and the other PostgreSQL date/time functions (https://www.postgresql.org/docs/current/static/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT)?
It should be possible to generate an UTC timestamp at every layer and combine it with the users' time zone id if you want to store or display it.

Edit: It is actually a good practice to set your whole PostgreSQL server to UTC (https://www.postgresql.org/docs/current/static/runtime-config-client.html#GUC-TIMEZONE) to avoid any timestamp magic.
Just use UTC everywhere and convert it only when you display it to the user.

[roji]

To supplement @Brar's comment (which is very correct), PostgreSQL NOW() indeed returns a timestamp with time zone, and the EF Core provider does translate DateTime.Now to NOW().

However, as @Brar suggests, if you use DateTime.UtcNow, the EF Core provider will translate that to NOW() AT TIME ZONE 'UTC', which returns a TIMESTAMP WITHOUT TIME ZONE. This is what you really want to use.

DateTime.Now should simply be banned from most code (at least from most server-side code), as it introduces a dependency on the local machine's configured timezone.

[dpsenner]

As a last comment I wanted to share with you an insight that I gained while attempting to find a valid way to convert a utc timestamp into a zoned timestamp that works fine without any magic conversions of the resulting timestamp happening. This is what I came up with, something that works regardless of the session timezone:

set timezone to 'UTC';
select
	'2018-06-06T12:00:00'::timestamp at time zone 'UTC' at time zone 'Europe/Rome', -- works
	'2018-06-06T12:00:00Z'::timestamp at time zone 'UTC' at time zone 'Europe/Rome' -- works

while the following is broken even though it appears to be the right syntax at first sight:

set timezone to 'UTC';
select
	'2018-06-06T12:00:00Z'::timestamp at time zone 'Europe/Rome', -- broken
	'2018-06-06T12:00:00'::timestamp at time zone 'Europe/Rome' -- broken

Happy coding and thanks for the productive conversation we had!