jackc / pgx

PostgreSQL driver and toolkit for Go
MIT License
10.34k stars 818 forks source link

Normalize UTC dates in pgxtypes v5 #1945

Closed jalandis closed 4 months ago

jalandis commented 6 months ago

Is your feature request related to a problem? Please describe.

When decoding timestamptz fields, UTC values have an unexpected location set instead of the GO standard nil value.

Test code comparing structs with reflect.DeepEqual fails on time.Time fields.

Describe the solution you'd like This appears to have been solved already in the separate pgtypes project but didn't make it to v5.

https://github.com/jackc/pgtype/commit/75446032b914bb0be5e07da29c976034c0a666cf

Changing the logic to match how GO std time works with UTC seems like a safe choice. I would be happy to create a PR with the code already in pgtypes v4.

Describe alternatives you've considered

Converting all assertions in the project is a large lift. It would not be feasible for our code base.

Additional context I am using the sql/database interfaces and not the pgx interfaces.

From GO time

        // loc specifies the Location that should be used to
    // determine the minute, hour, month, day, and year
    // that correspond to this Time.
    // The nil location means UTC.
    // All UTC times are represented with loc==nil, never loc==&utcLoc.
    loc *Location
jalandis commented 6 months ago

I found that there was a PR in testify.Equal to use time.Equal in comparisons. This was determined to be a bug as 2 times with different representations were not considered equal.

The PR's fix/bug was reverted by 1.9.0 :-(

https://github.com/stretchr/testify/issues/1536

https://github.com/stretchr/testify/pull/1537

jackc commented 6 months ago

I think that https://github.com/jackc/pgtype/commit/75446032b914bb0be5e07da29c976034c0a666cf only affects parsing a text formatted timestamptz because the time zone will be part of the string. Binary formatted values do not contain any time zone information. The time zone is somewhat arbitrary. This led to https://github.com/jackc/pgx/issues/1195.

Hmm... thinking about this issue and reviewing https://github.com/jackc/pgx/issues/1195 gives me an idea...

... ... ...

Try PR https://github.com/jackc/pgx/pull/1948. I think that this issue could be resolved by the following code in an after connect hook.

        conn.TypeMap().RegisterType(&pgtype.Type{
            Name:  "timestamptz",
            OID:   pgtype.TimestamptzOID,
            Codec: &pgtype.TimestamptzCodec{ScanLocation: time.UTC},
        })
jalandis commented 6 months ago

That worked beautifully! All my tests are now passing.

jackc commented 4 months ago

Merged #1948.