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

UTC date is not sent as-is #1708

Closed
MichaelBuen opened this Issue May 27, 2018 · 13 comments

Comments

Projects
None yet
3 participants
@MichaelBuen

MichaelBuen commented May 27, 2018

schedule field is of timestamptz type.

backing .NET property for schedule field is DateTime type.

User selected May 5, 2018 1:37 AM. My computer's local is Asia/Manila, which is +8.

So UTC date sent is "2018-05-04T17:37:00.000Z"

image

What's shown from NHibernate log, 2018-05-04T17:37:00.0000000Z

NHibernate: 
    select
        nextval ('job.job_id_seq')
NHibernate: 
    INSERT 
    INTO
        job.job
        (created_by_fk, created_date, position, description,
         schedule, duration, duration_type_fk, pay, is_pay_net, 
         needed, is_needed_open, 
         location, address_line_1, address_line_2, city_fk, zip_code, 
         location_type_fk, id) 
    VALUES
        (:p0, :p1, :p2, :p3, :p4, :p5, :p6, :p7, :p8, :p9, :p10, :p11, :p12, :p13, :p14, :p15, :p16, :p17);

:p0 = 1 [Type: Int32 (0:0:0)], 
:p1 = 2018-05-27T03:36:57.2189470Z [Type: DateTime (0:0:0)], 
:p2 = 'xxx' [Type: String (0:0:0)], 
:p3 = 'e' [Type: String (0:0:0)], 

:p4 = 2018-05-04T17:37:00.0000000Z [Type: DateTime (0:0:0)], 
:p5 = 1 [Type: Int32 (0:0:0)], 
:p6 = 'D' [Type: String (0:0:0)], 
:p7 = 123 [Type: Decimal (0:0:0)], 
:p8 = True [Type: Boolean (0:0:0)], 

:p9 = 324 [Type: Int32 (0:0:0)], 
:p10 = True [Type: Boolean (0:0:0)], 

:p11 = 'l' [Type: String (0:0:0)], 
:p12 = 'a1' [Type: String (0:0:0)], 
:p13 = 'a2' [Type: String (0:0:0)], 
:p14 = 991 [Type: Int32 (0:0:0)], 
:p15 = '1230' [Type: String (0:0:0)], 

:p16 = 'SC' [Type: String (0:0:0)], 
:p17 = 12 [Type: Int32 (0:0:0)]

What gets sent to Postgres (from Postgres log), the 'Z' is scrubbed, '2018-05-04 17:37:00':

INSERT INTO job.job (
    created_by_fk, created_date, position, description, 
    schedule, duration, duration_type_fk, pay, is_pay_net, 
    needed, is_needed_open, 
    location, address_line_1, address_line_2, city_fk, zip_code, 
    location_type_fk, id) 
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18)

2018-05-27 11:36:57.470 +08 [3705] DETAIL:  parameters: 
$1 = '1', 
$2 = '2018-05-27 03:36:57.218947', 
$3 = 'xxx', 
$4 = 'e', 

$5 = '2018-05-04 17:37:00', 
$6 = '1', 
$7 = 'D',
$8 = '123', 
$9 = 't', 

$10 = '324', 
$11 = 't', 

$12 = 'l', 
$13 = 'a1', 
$14 = 'a2', 
$15 = '991', 
$16 = '1230', 

$17 = 'SC', 
$18 = '12'

It's confusing that NHibernate sends wrong data to Postgres even though NHibernate's log indicate that it's going to send the exact and correct data.

schedule field from NHibernate log: 2018-05-04T17:37:00.0000000Z

schedule field from Postgres log: '2018-05-04 17:37:00'

That causes an incorrect value saved to Postgres database:

image

Is it possible to configure NHibernate to make it send fields exactly as it is? I tried in pgAdmin, it's possible to insert UTC date directly:

image

@oskarb

This comment has been minimized.

Member

oskarb commented May 27, 2018

It looks like NHibernate does aim to handle this properly, from a quick glance in the code, but what version are you using, and how have you mapped the field?

Also, have you tried setting the value using direct calls to Npgsql, just to rule out bugs there?

@fredericDelaporte

This comment has been minimized.

Member

fredericDelaporte commented May 27, 2018

There are many points to consider about date & time handling.

The DateTime .Net type is ill-suited for handling timezone, as it only distinguishes between UTC times and "local" times, without conveying any information on what is the "local" timezone. It is handled as a DbType.DateTime type, which is not meant to handle any timezone information.

Anyway, NHibernate does not alter the local/utc kind of a .Net DateTime with the default NHibernate type DateTimeType. It transmits it "as is" to the data provider, which is very likely Npgsql in your case. So it is very likely Npgsql that drops the "local or utc" information. (But I would not consider this as a bug, since it receives a DbType.DateTime typed parameter.)

If you want this behavior to be changed, you should in my opinion see that on Npgsql side. (So I am closing your issue here.) And indeed you have already done it. (npgsql/npgsql#1955)

Now for a better support of the timezone, you should consider using the .Net type DateTimeOffset instead. Unfortunately, NHibernate PostgreSQL dialect currently does not handles it: you will need to use a custom dialect deriving from it and adding the mapping for it.

Overriding RegisterDateTimeTypeMappings should be enough.

protected override void RegisterDateTimeTypeMappings()
{
	base.RegisterDateTimeTypeMappings();
	RegisterColumnType(DbType.DateTimeOffset, 6, "timestamp($s) with time zone");
	// Not specifying default scale: Postgres doc writes it means "no explicit limit",
	// so max of what it can support, which suits our needs.
	RegisterColumnType(DbType.DateTimeOffset, "timestamp with time zone");
}

Update: timestamp with time zone does not have a time zone! See there, and npgsql/npgsql#11. This type does only store an UTC timestamp, converted back on read to the database local time. There is no suitable PostgreSQL types for handling DateTimeOffset.

@fredericDelaporte

This comment has been minimized.

Member

fredericDelaporte commented May 27, 2018

Well, I have read a bit more on npgsql/npgsql#1955 side, and it seems to me npgsql/npgsql#1940 will not be enough alone if you want to keep using DateTime instead of DateTimeOffset.

It states:

Datetime is always sent as timestamp by default, regardless of its kind. You can still specify NpgsqlDbType.TimestampTz, in which case local DateTime gets converted to UTC before sending.

NHibernate will not specify NpgsqlDbType.TimestampTz. You will need to use a custom driver deriving from NHibernate.Driver.NpgsqlDriver and overriding InitializeParameter, casting the DbParameter to the NpgsqlParameter specific class then specifying its NpgsqlDbType, of course only if that is suitable for the parameter.

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 27, 2018

@oskarb I tried on NHibernate 5.1.2 using Npgsql 3.2.7, then tried its latest Npgsql 4.0.0-rc1

Have tried setting all the combinations of timestamp and timestamptz with local date and utc date to check NHibernate/Npgsql behavior. See npgsql/npgsql#1955 (comment)

I tried using Npgsql only, looks like it has the correct behavior. See: npgsql/npgsql#1955 (comment)

Looks like there might be some problems how NHibernate uses Npgsql?

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 27, 2018

@fredericDelaporte Will try the RegisterDateTimeTypeMappings suggestion

@fredericDelaporte

This comment has been minimized.

Member

fredericDelaporte commented May 27, 2018

Registering DateTimeOffset will not change anything in a test using only DateTime. (Moreover it appears PostgreSQL cannot actually handle DateTimeOffset: it has no type actually storing the offset, see this comment.) If you want to keep using DateTime, you have to override InitializeParameter and tweak DateTime parameters instead.

By the way a test case supplied as images are far less than ideal. Better provide formatted code, or a gist, or even PR a test case.

Also please note that your Npgsql test case does specify neither the parameters DbType nor their NpgsqlDbType, letting Npgsql inferring it, which may result in something else than DbType.DateTime. While NHibernate does specify the DbType. The difference in behavior is likely to come from that.

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 27, 2018

@fredericDelaporte Awesome! the InitializeParameter approach works. The utc 2018-05-04T17:37:00.000Z is now correctly persisting to timestamptz field as 2018-05-05 01:37:00+08, not as 2018-05-04 17:37:00+08. I'll just keep using DateTime for timestamptz until there is a compelling reason to use DateTimeOffset for timestamptz.

Thanks!

Posting the code here in case other users encountered same problem:

namespace AspNetCoreExample.Infrastructure.NHibernateNpgsqlInfra
{
    using System.Data.Common;

    using NHibernate.SqlTypes;
    using Npgsql;

    public class NpgsqlDriverExtended : NHibernate.Driver.NpgsqlDriver
    {        
        // this gets called when an SQL is executed
        protected override void InitializeParameter(DbParameter dbParam, string name, SqlType sqlType)
        {            
            if (sqlType is NpgsqlExtendedSqlType && dbParam is NpgsqlParameter)
            {
                this.InitializeParameter(dbParam as NpgsqlParameter, name, sqlType as NpgsqlExtendedSqlType);
            }
            else
            {
                base.InitializeParameter(dbParam, name, sqlType);
                
                if (sqlType.DbType == System.Data.DbType.DateTime)
                {
                    dbParam.DbType = System.Data.DbType.DateTimeOffset;
                }
            }
        }

        // NpgsqlExtendedSqlType is used for Jsonb
        protected virtual void InitializeParameter(NpgsqlParameter dbParam, string name, NpgsqlExtendedSqlType sqlType)
        {
            if (sqlType == null)
            {
                throw new NHibernate.QueryException(string.Format("No type assigned to parameter '{0}'", name));
            }

            dbParam.ParameterName = FormatNameForParameter(name);
            dbParam.DbType        = sqlType.DbType;
            dbParam.NpgsqlDbType  = sqlType.NpgDbType;
        }               
    }
}

Sample NpgsqlDriver wiring: https://github.com/MichaelBuen/AspNetCoreExample/blob/b78b97f085730cfb9494c3ec15085cccf7f761be/AspNetCoreExample.Ddd.Mapper/_TheMapper.cs#L32

@fredericDelaporte

This comment has been minimized.

Member

fredericDelaporte commented May 27, 2018

I have to amend a point: switching to DateTimeOffset cannot work with PostgreSQL, because it has no suitable types for handling a DateTimeOffset.

timestamp with time zone (short timestamptz) does not have a time zone! See there, and npgsql/npgsql#11. This type does only store an UTC timestamp, converted back on read to the database local time. There is no suitable PostgreSQL types for handling DateTimeOffset.

So the only viable solution when using NHibernate with PostgreSQL and Npgsql is currently to override InitializeParameter for "patching" parameters typed DbType.DateTime in order to tweak them and work around the semantic of timestamptz and how you try to use it (sending an UTC DateTime to it). I do not think NHibernate will ever handle it natively, because that is a type that stores things in UTC but yields them back in the database local time. There is no NHibernate types handling this behavior.

In fact in your case maybe you would have better to map that field with the NHibernate LocalDateTime type, and write it in your entities already in local time.

Or map it with the NHibernate UtcDateTime type but change your database local zone for it to be UTC, thus ensuring it will yield timestamptz back as UTC.

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 29, 2018

Yes timestamp with time zone does not have a time zone, and its unfortunate that it's for presentation purposes only.

I think I'll opt to latter, just set the database local zone to UTC and use NHibernate's UtcDateTime

if (type == typeof(System.DateTime) || type == typeof(System.DateTime?))            
{               
        propertyMapper.Type<NHibernate.Type.UtcDateTimeType>();
}             

It's confusing to debug the app that it is sending UTC in Zulu format 2018-05-04T17:37:00.000Z, yet it is receiving same UTC value in local time format 2018-05-05T01:37:00+08:00, I'd rather see the same format 2018-05-04T17:37:00.000Z all throughout.

It would be really nice though if Postgres truly supports persisting UTC's timezone too, so the app can send 2018-05-05T01:37:00+08:00 and2018-05-05T01:37:00+09:00, and receive them as-is, not as 2018-05-05T01:37:00+08:00 and 2018-05-05 00:37:00+08 respectively.

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 29, 2018

I tried setting the database to UTC in postgresql.conf:

# - Locale and Formatting -

datestyle = 'iso, mdy'
#intervalstyle = 'postgres'
# timezone = 'Asia/Manila'
timezone = 'UTC'

And then on auto-mapper:

static void Mapper_BeforeMapProperty(
    NHibernate.Mapping.ByCode.IModelInspector modelInspector,
    NHibernate.Mapping.ByCode.PropertyPath propertyPath,
    NHibernate.Mapping.ByCode.IPropertyMapper propertyMapper
)
{
    string postgresFriendlyName = propertyPath.ToColumnName().ToLowercaseNamingConvention();

    propertyMapper.Column(postgresFriendlyName);

    System.Type type = propertyPath.LocalMember.GetPropertyOrFieldType();     

    if (type == typeof(System.DateTime) || type == typeof(System.DateTime?))            
    {               
            propertyMapper.Type<NHibernate.Type.UtcDateTimeType>();
    }            
}

Though the UtcDateTimeType auto-mapper above made the date return in Zulu format, the existing field that is persisted from 2018-05-04T17:37:00.000Z is now return incorrectly as 2018-05-05T01:37:00Z. Though It returned the desired JSON date format (Zulu), it returns incorrect correct value (2018-05-05T01:37:00Z), it should return 2018-05-04T17:37:00.000Z instead.

If the auto-mapper to UtcDateTimeType is removed, though the UTC 2018-05-04T17:37:00.000Z is not returned in Zulu format and returns 2018-05-05T01:37:00+08:00 format instead, that returned value is correct.

Using NHibernate's UtcDateTimeType, the existing field that is persisted from 2018-05-04T17:37:00.000Z, is returned as 2018-05-05T01:37:00Z, it's wrong.

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 29, 2018

I removed the NHibernate's UtcDateTimeType auto-mapper, though it's returning the desired format (Zulu), it's returning incorrect value.

Just solved the problem at service side instead, ASP.NET Core can be configured to return date in Zulu format, that is instead of the value 2018-05-04T17:37:00.000Z serializes as 2018-05-05T01:37:00+08:00, it now serializes as Zulu format 2018-05-04T09:38:00Z.

Found the solution here: https://stackoverflow.com/questions/41728737/iso-utc-datetime-format-as-default-json-output-format-in-mvc-6-api-response/41728953.

I keep this, since ASP.NET just concerns itself with serialization/deserialization, thanks for this 👍

namespace AspNetCoreExample.Infrastructure.NHibernateNpgsqlInfra
{
    using System.Data.Common;

    using NHibernate.SqlTypes;
    using Npgsql;

    public class NpgsqlDriverExtended : NHibernate.Driver.NpgsqlDriver
    {        
        // this gets called when an SQL is executed
        protected override void InitializeParameter(DbParameter dbParam, string name, SqlType sqlType)
        {            
            if (sqlType is NpgsqlExtendedSqlType && dbParam is NpgsqlParameter)
            {
                this.InitializeParameter(dbParam as NpgsqlParameter, name, sqlType as NpgsqlExtendedSqlType);
            }
            else
            {
                base.InitializeParameter(dbParam, name, sqlType);
             
                //// Solved this problem: https://github.com/nhibernate/nhibernate-core/issues/1708#issuecomment-392338162
                if (sqlType.DbType == System.Data.DbType.DateTime)
                {
                    dbParam.DbType = System.Data.DbType.DateTimeOffset;
                }
            }
        }

        // NpgsqlExtendedSqlType is used for Jsonb
        protected virtual void InitializeParameter(NpgsqlParameter dbParam, string name, NpgsqlExtendedSqlType sqlType)
        {
            if (sqlType == null)
            {
                throw new NHibernate.QueryException(string.Format("No type assigned to parameter '{0}'", name));
            }

            dbParam.ParameterName = FormatNameForParameter(name);
            dbParam.DbType        = sqlType.DbType;
            dbParam.NpgsqlDbType  = sqlType.NpgDbType;
        }               
    }
}
@fredericDelaporte

This comment has been minimized.

Member

fredericDelaporte commented May 29, 2018

In my tests, using an UtcDateTimeType on a timestamptz fails due to the datareader reading the date converted back to the local timezone, although the database timezone has been set to UTC. I do not know if this is a Npgsql bug (since the documentation just states it returns the date with a local Kind, without mentioning any conversion process, contrary to the writing case), or a PostgreSQL behavior. (Does it receives the client timezone and does it takes precedence other the database one?)

But that does not look to me as an issue on NHibernate side. (Otherwise a new issue should be opened.)

Overall for using UtcDateTimeType, I now think the timestamp type must be used instead of timestamptz. That would remove any auto-conversion on the Npgsql/PostgreSQL side, conversions which are the root cause of the trouble.

@MichaelBuen

This comment has been minimized.

MichaelBuen commented May 30, 2018

YesUtcDateTimeType + timestamp works, before I posted this issue I almost settle on that combo. I just went back to timestamptz as I feel it's easier to query than timestamp. timestamp need extra hoops.

x=# create table z(id int primary key, _timestamp timestamp, _timestamptz timestamptz);
CREATE TABLE
x=# 
x=# insert into z(id, _timestamp, _timestamptz) values (1, '2018-05-04T17:37:00Z', '2018-05-04T17:37:00Z');
INSERT 0 1
x=# 
x=# set timezone to 'Asia/Manila';
SET
x=# select 
x-#   _timestamp, 
x-#  _timestamptz, 
x-#  _timestamp at time zone 'UTC' at time zone 'Japan' as "timestamp in Japan", 
x-#  _timestamptz at time zone 'Asia/Tokyo' "timestamp in Japan",
x-#  _timestamp at time zone 'UTC' at time zone 'Asia/Manila' "timestamptz in Philippines", 
x-#  _timestamptz at time zone 'Asia/Manila' as "timestamptz in Philippines"
x-# from z;
-[ RECORD 1 ]--------------+-----------------------
_timestamp                 | 2018-05-04 17:37:00
_timestamptz               | 2018-05-05 01:37:00+08
timestamp in Japan         | 2018-05-05 02:37:00
timestamp in Japan         | 2018-05-05 02:37:00
timestamptz in Philippines | 2018-05-05 01:37:00
timestamptz in Philippines | 2018-05-05 01:37:00

x=# 
x=# set timezone to 'Asia/Tokyo';
SET
x=# select 
x-#   _timestamp, 
x-#  _timestamptz, 
x-#  _timestamp at time zone 'UTC' at time zone 'Japan' as "timestamp in Japan", 
x-#  _timestamptz at time zone 'Asia/Tokyo' "timestamp in Japan",
x-#  _timestamp at time zone 'UTC' at time zone 'Asia/Manila' "timestamptz in Philippines", 
x-#  _timestamptz at time zone 'Asia/Manila' as "timestamptz in Philippines"
x-# from z;
-[ RECORD 1 ]--------------+-----------------------
_timestamp                 | 2018-05-04 17:37:00
_timestamptz               | 2018-05-05 02:37:00+09
timestamp in Japan         | 2018-05-05 02:37:00
timestamp in Japan         | 2018-05-05 02:37:00
timestamptz in Philippines | 2018-05-05 01:37:00
timestamptz in Philippines | 2018-05-05 01:37:00

Does it receives the client timezone and does it takes precedence other the database one?

Yes that might be the reason, the client/system's timezone is overriding/ignoring whatever timezone is set in postgresql.conf. roji confirmed that session's timezone is ignored and Npgsql just uses the system's timezone. Same thing applies to Postgres-wide timezone (timezone set in postgresql.conf), Postgres just sends back the raw value of timestamptz regardless of timezone in postgresql.conf.

When Npgsql reads values from PostgreSQL, those values are in binary encoding. PostgreSQL's binary encoding of timestamptz is simply the UTC timestamp value, as stored in the column - there's no timezone to transfer. To (partially) mimic the PostgreSQL conversion behavior, when reading a timestamptz value, Npgsql will convert it to the local timezone. However, this means the client machine's timezone (where Npgsql is executing), and not PostgreSQL's timezone - this is where the discrepancy is coming from.

It looks like for all intents and purposes, the session's timezone, and even postgresql.conf's timezone are just a pgAdmin / psql thing, at least for now. roji said that it's possible to make Npgsql use the timezone from Postgres, he said it's not a trivial thing to do due to differences in IANA timezone id and Windows timezone id

I wish though Npgsql author could at least facilitate UTC timezone conversion since it's constant anyway. That is if the postgresql.conf/session's timezone is set to UTC, Npgsql would apply the UTC's offset(+00:00) to 2018-05-04T17:37:00.000Z and return that as 2018-05-04T17:37:00+00:00, then NHibernate's UtcDateTimeType would work on timestamptz

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment