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

How to read TIMESTAMPTZ as DateTimeOffset #2641

Open
shadeglare opened this issue Sep 26, 2019 · 19 comments

Comments

@shadeglare
Copy link

commented Sep 26, 2019

I can't understand how to read a postgres timestamptz as .NET DateTimeOffset value. According to the documentation it seems to be possible. Could someone describe how to handle this issue.

Postgres: 11.x
Npgsql: 4.1.0

The code snippet is extremely simple:

using (var connection = new NpgsqlConnection(connectionString))
{
    connection.Open();
    using (var command = new NpgsqlCommand())
    {
        command.Connection = connection;
        command.CommandType = CommandType.Text;
        command.CommandText = "SELECT '2004-10-19 10:23:54+02'::timestamptz";
        var o = (DateTimeOffset)command.ExecuteScalar();
        Console.WriteLine(o);
    }
}

It throws an invalid cast exception from DateTime to DateTimeOffset:

Unhandled Exception: System.InvalidCastException: Unable to cast object of type 'System.DateTime' to type 'System.DateTimeOffset'
@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

You're right, it's a bug. The timestamptz handler reads values as DateTime by default. To workaround this use the GetFieldValue<>(int) method to specify the return type.

using var reader = command.ExecuteReader();
var value = reader.GetFieldValue<DateTimeOffset>(0);
@austindrenski

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

Standard plug to recommend taking a look at Npgsql's NodaTime plugin which supports reading timestamptz as either ZonedDateTime or OffsetDateTime.

@shadeglare

This comment has been minimized.

Copy link
Author

commented Sep 26, 2019

You're right, it's a bug. The timestamptz handler reads values as DateTime by default. To workaround this use the GetFieldValue<>(int) method to specify the return type.

using var reader = command.ExecuteReader();
var value = reader.GetFieldValue<DateTimeOffset>(0);

It works but not suitable for me as I have only a Type object so no way to use the GetFieldValue method. reader.GetValue is also broken.

@shadeglare

This comment has been minimized.

Copy link
Author

commented Sep 26, 2019

Standard plug to recommend taking a look at Npgsql's NodaTime plugin which supports reading timestamptz as either ZonedDateTime or OffsetDateTime.

Have no idea how it works with other libs, serializers, etc. I prefer not to use external dependencies.

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 26, 2019

In that case you can make your own type handler which will wrap a handler produced by TimestampTzHandlerFactory. Just inherit from NpgsqlSimpleTypeHandlerWithPsv<DateTime, NpgsqlDateTime> and proxy all calls.

@YohDeadfall YohDeadfall self-assigned this Sep 26, 2019
@YohDeadfall YohDeadfall added the bug label Sep 26, 2019
@YohDeadfall YohDeadfall added this to the 5.0 milestone Sep 26, 2019
@shadeglare

This comment has been minimized.

Copy link
Author

commented Sep 27, 2019

Guys. One more thing.

If I pass a DateTime value into a NpgsqlCommand parameter:

        command.CommandText = "SELECT @p";
        command.Parameters.AddWithValue("@p", NpgsqlDbType.TimestampTz, DateTime.Now);
        var result = (DateTime)command.ExecuteScalar();

or

        command.CommandText = "SELECT @p";
        command.Parameters.AddWithValue("@p", DateTime.Now);
        var result = (DateTime)command.ExecuteScalar();

In the first case I get a DateTime with Kind = Local (and that's right) and in the second one Kind = Unspecified (ok, the driver treats it as just a timestamp without time zone).

Same goes for passing DateTime arrays. Have to specify NpgsqlDbType.TimestampTz | NpgsqlDbType.Array to get an array of DateTime with kind Local objects.

But if I have a composite type with a timestamp field and pass it via the same technique as above the DateTime field will be correctly returned as DateTime Local. Just for example:

CREATE TYPE obj_with_date AS ( 
    date timestamp with timezone
);

CREATE OR REPLACE FUNCTION get_object_with_date(_value object_with_date)
RETURNS object_with_date AS $$
BEGIN
    RETURN _value;
END;
$$ LANGUAGE plpgsql;
public class ObjectWithDate 
{
    public DateTime Date { get; set; }
}
NpgsqlConnection.GlobalTypeMapper.MapComposite<ObjectWithDate>("object_with_date");
...
command.CommandText = "get_object_with_date";
command.Parameters.AddWithValue("_value", new ObjectWithDate { Date = DateTime.Now });
using var reader = command.ExecuteReader();
reader.Read();
var date = (DateTime)reader.GetValue(0);

So I have no clue how the driver understands that. It looks like inconsistent behavior.

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

PostgreSQL uses OIDs to specify value types when sends or receives data. So...

When Npgsql executes a command it sends a few messages to the server. One of them is Parse which contains parameter types.

When PostgreSQL returns a non-empty result set it sends RowDescription to the client which contains all required information about columns in the set including their names, types, etc.

In the first case you manually specifies that the value type is timestamptz. The backend returns the same value with the same OID which is used by Npgsql to determine which handler should be used to read the value.

In the second case Npgsql detects the right type based on the value type and the default mapping (DateTime maps to timestamp), and the same thing happens as before. As you guess it uses TimestampHandler and the corresponding OID.

When you are working with composites, field types are determined by metadata returned from the server during the first connection to it. This is how Npgsql knows what handlers should be used to write and read composite fields. In your case the type of the date field is timestamp with timezone.

For reference:

@roji

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

@YohDeadfall,

You're right, it's a bug. The timestamptz handler reads values as DateTime by default. To workaround this use the GetFieldValue<>(int) method to specify the return type.

Where do you see the bug exactly? In our read mappings docs, we clearly say that timestamptz is read as DateTime by default, and as DateTimeOffset optionally. As far as I can see the above is the expected behavior?

It works but not suitable for me as I have only a Type object so no way to use the GetFieldValue method.

If you only have a Type instance in runtime, another possible option is to generate a delegate that would call GetFieldValue with that type, and cache that delegate. It's somewhat advanced but it could solve your problem. However, this wouldn't help for fields within composites.

@shadeglare

This comment has been minimized.

Copy link
Author

commented Sep 27, 2019

@roji seems you're right. is there a simple way to override the default behavior? I was supposed to write my own mapper but too much inner classes have internal props, methods or so. Just wanted to get DateTimeOffset and yes there can be composite types a row not only date columns. It makes creating an expression only for one type quite annoying. Thanks anyway.

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

As far as I can see the above is the expected behavior?

Ah, yes... Are you sure that it's the right mapping and doesn't introduce confusion?

@YohDeadfall YohDeadfall removed this from the 5.0 milestone Sep 27, 2019
@YohDeadfall YohDeadfall added discussion and removed bug labels Sep 27, 2019
@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Sep 27, 2019

Just wanted to get DateTimeOffset and yes there can be composite types a row not only date columns.

If there is a DateTimeOffset field in some composite it will be read as DateTimeOffset. The last version uses generic methods in the composite handler and won't throw an InvalidCastException.

@roji

This comment has been minimized.

Copy link
Member

commented Sep 30, 2019

@YohDeadfall

Ah, yes... Are you sure that it's the right mapping and doesn't introduce confusion?

We can reopen the conversation - to be honest the date/time mapping logic changed enough that I no longer remember the history anymore. We return DateTime(Unspecified) for timestamp and DateTime(Local) for timestamptz. I'm not sure why DateTimeOffset would be more appropriate...

@Kharos

This comment has been minimized.

Copy link

commented Oct 14, 2019

@YohDeadfall

Ah, yes... Are you sure that it's the right mapping and doesn't introduce confusion?

We can reopen the conversation - to be honest the date/time mapping logic changed enough that I no longer remember the history anymore. We return DateTime(Unspecified) for timestamp and DateTime(Local) for timestamptz. I'm not sure why DateTimeOffset would be more appropriate...

The .net DateTime(local) is a timestamp in the "local" timezone (local being the computer that runs the calculation, which does not always make sense in a client-server environment by the way). DateTimeOffset is a timestamp including timezone.

If I understand correctly, timestamptz is a timestamp with timezone, exactly like DateTimeOffset. When reading it as a DateTime you convert it to a different timezone. While that conversion is hopefully done such that it represents the same UTC timestamp, DateTimeOffset is still a more faithful representation of the original data.

@roji

This comment has been minimized.

Copy link
Member

commented Oct 14, 2019

I understand correctly, timestamptz is a timestamp with timezone, exactly like DateTimeOffset

That is incorrect and a common misconception (see this warning). If timestamptz contained an offset or timezone, we'd have mapped it to .NET DateTimeOffset, but it doesn't...

@Kharos

This comment has been minimized.

Copy link

commented Oct 14, 2019

Wow... thanks for clearing my confusion! I should have read the documentation more attentively. While the examples in the documentation (e.g. TIMESTAMP WITH TIME ZONE '2004-10-19 10:23:54+02') make it look like it is stored as a timestamp with timezone, the documentation then goes on to say:

For timestamp with time zone, the internally stored value is always in UTC (Universal Coordinated Time, traditionally known as Greenwich Mean Time, GMT).
When a timestamp with time zone value is output, it is always converted from UTC to the current timezone zone, and displayed as local time in that zone.

Reading that it does look that DateTime(local) is the correct mapping, as much as I prefer using DateTimeOffset in my .net code.

@YohDeadfall

This comment has been minimized.

Copy link
Member

commented Oct 14, 2019

One reason to have DateTimeOffset as the default mapping is that psql (and probably others) prints values with an offset when a set is returned.

@roji

This comment has been minimized.

Copy link
Member

commented Oct 14, 2019

Any change in the default mapping would be a pretty major breaking change, so it's a good idea to avoid it until there's a good reason.

The original logic for avoiding DateTimeOffset is that it gives the impression that there's an offset coming from the database - that simply isn't the case. I think this would cause even more confusion (and there's already a lot).

Unless anyone has strong feelings on this, I propose we close and keep the current behavior.

@Kharos

This comment has been minimized.

Copy link

commented Oct 21, 2019

Related to my initial confusion: https://www.npgsql.org/doc/types/datetime.html

The ".NET types and PostgreSQL types" paragraph says in the prominent warning:

A common mistake is for users to think that the PostgreSQL timestamp with timezone type stores the timezone in the database. This is not the case: only the timestamp is stored. There is no single PostgreSQL type that stores both a date/time and a timezone, similar to .NET DateTimeOffset.

A bit further down, in the Timezone paragraph, the documentation says:

PostgreSQL time with time zone is the only date/time type which actually stores a timezone in the database.

Umm... is that a documentation bug? If so, do we need to create a doc issue to get it fixed?

@roji

This comment has been minimized.

Copy link
Member

commented Oct 21, 2019

@Kharos nope, note that timestamp with timezone is very different from time with timezone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.