PG types without timezone being parsed into time.Times with.

[e-dard]

Hi,

I briefly discussed this with @johto in #postgresql.

Currently, when a time-based PG type that doesn't have timezone information is parsed into a Go type, it's parsed into a time.Time that does contain timezone information. Specifically, the resulting time.Time contains a single zone in the Location, while the zero-value for a time.Location would contain an empty slice of zones, i.e., the absence of any timezones.

Given the PG value '2015-01-13'::date it should be equivalent to the following time.Time:

time.Time{
        sec: 63556704000, 
        nsec: 0, 
        // zero value for a *time.Location
        loc: &time.Location{
                name: "", 
                zone: []time.zone(nil), 
                tx: []time.zoneTrans(nil), 
                cacheStart: 0, 
                cacheEnd: 0, 
                cacheZone: (*time.zone)(nil),
         },
}

Currently, lib/pq would return the following time.Time:

time.Time{
    sec: 63556704000, 
    nsec: 0, 
    loc: &time.Location{
        name: "", 
        zone: []time.zone{
            time.zone{
                name: "", 
                offset: 0, 
                isDST: false,
            },
        }, 
        tx: []time.zoneTrans{
            time.zoneTrans{
                when: -9223372036854775808, 
                index: 0x0, 
                isstd: false, 
                isutc: false},
            },
        },
        cacheStart: -9223372036854775808, 
        cacheEnd: 9223372036854775807, 
        cacheZone:(*time.zone)(0xc20801eaa0),
    },
}

I'm don't think this is correct because the time.Location produced by lib/pq contains zone information, even if the zone is itself the zero-value.

I would be happy to put a fix in for this, but wanted to see if there was support for that idea or not? I've added a PR with a test case highlighting the issue.

[johto]

I think what we'd need to consider here are:

1. How many applications would changing this realistically break?
2. What are the practical gains (meaning "outside of test cases") of changing this? Are there, for example, any calculations which give the wrong (or at least a dubious) answer with the time.Time value returned now?

I can't claim to know the time package well enough to answer the second question without any research.

[johto]

I've given this some more thought, and if this doesn't break any calculations, I think we should fix this. It might break someone's test cases, but I think doing the correct thing going forward is more important than that.

[e-dard]

@johto sorry for the late reply to your original comment.

I agree (obviously) 😄

Shall I try and put together a fix for this issue and then go from there?

[johto]

I think the first thing would be to research to see if there are any cases where the user would see a difference in behavior.

[cbandy]

Maybe time.Time isn't the appropriate Go type for these Postgres types.

http://golang.org/pkg/time/#Time:

A Time represents an instant in time with nanosecond precision.
Each Time has associated with it a Location...

http://golang.org/pkg/time/#Date:

Date panics if loc is nil.

I don't have access to check right now, but I wonder how Postgres handles comparing date to timestamp (with and without time zone) values?

[johto]

Maybe time.Time isn't the appropriate Go type for these Postgres types.

Yeah, it's not really designed for this, but it's very very close.

http://golang.org/pkg/time/#Time:

A Time represents an instant in time with nanosecond precision.
Each Time has associated with it a Location...
http://golang.org/pkg/time/#Date:

Date panics if loc is nil.

Yeah, but the idea is to use the "zero" location instead of a nil location.

I don't have access to check right now, but I wonder how Postgres handles comparing date to timestamp (with and without time zone) values?

The date is converted to the appropriate type* first.

[cbandy]

with time zone is probably session timezone then?

I also forgot that users get to scan into whatever they want. If they want a time.Time, then I think it should either have the [backend] session timezone or the [go runtime] local timezone.

[e-dard]

http://golang.org/pkg/time/#Time:

A Time represents an instant in time with nanosecond precision.
Each Time has associated with it a Location...
http://golang.org/pkg/time/#Date:

Date panics if loc is nil.

Yeah, but the idea is to use the "zero" location instead of a nil location.

Exactly, so in a test if you were say asserting you got a certain DATE type back from PG you would use the following to create the appropriate time.Time:

expected := time.Date(2015, 2, 13, 0, 0, 0, 0, &time.Location{})

while currently, you have to do:

expected := time.Date(2015, 2, 13, 0, 0, 0, 0, time.FixedZone("", 0))

According to the time package documentation FixedZone:

// FixedZone returns a Location that always uses
// the given zone name and offset (seconds east of UTC).

I must admit, at this point in time I'm not entirely sure what the use-case for FixedZone actually is, but the semantics don't match up with the absence of a time zone, which I'm arguing &time.Location (and thus the zero-value for any embedded zone information, does.

[cbandy]

I'm not entirely sure what the use-case for FixedZone actually is

http://golang.org/pkg/time/#Parse uses FixedZone when the input offset is not local.

[johto]

with time zone is probably session timezone then?

I also forgot that users get to scan into whatever they want. If they want a time.Time, then I think it should either have the [backend] session timezone or the [go runtime] local timezone.

I think our behavior when scanning a timestamp with time zone into a time.Time is correct already. The problem is that we claim that the time.Time when scanned from a timestamp without time zone has a UTC offset of 0. But we don't know what the times represent, so it's a lie.

[johto]

Hmm.. But this doesn't look correct either:

    t := time.Date(2015, 2, 13, 0, 0, 0, 0, &time.Location{})
    fmt.Println(t)

% go run test.go
2015-02-13 00:00:00 +0000 UTC

.. UTC? That seems to be even more wrong than the "+0000 +0000" we currently return.

[e-dard]

I think that's because of the fmt.Stringer implementation for a time.Location.

[johto]

Yeah. Thing is, they both behave as if they're UTC (or at least something with 0 offset from UTC). The difference is that what we currently return doesn't claim to be UTC, and I think that's less wrong in this case. I can't really see any other differences here unless I use reflection to look into the time.Time struct itself, and that's just implementation details in my mind.

I think both behaviors are wrong, but the one we currently implement is less so.

[cbandy]

On the backend, values without zone information act like they are in the zone of the current session:

pqgotest=> SHOW timezone;
    TimeZone     
-----------------
 America/Chicago
(1 row)

pqgotest=> SELECT a, b, a = b, a < b, a > b FROM (VALUES ('2015-02-13'::date, '2015-02-13T00:00:00+00:00'::timestamptz)) AS x(a,b);
     a      |           b            | ?column? | ?column? | ?column? 
------------+------------------------+----------+----------+----------
 2015-02-13 | 2015-02-12 18:00:00-06 | f        | f        | t
(1 row)

pqgotest=> SELECT a, b, a = b, a < b, a > b FROM (VALUES ('2015-02-13T04:05:06'::timestamp, '2015-02-13T04:05:06+00:00'::timestamptz)) AS x(a,b);
          a          |           b            | ?column? | ?column? | ?column? 
---------------------+------------------------+----------+----------+----------
 2015-02-13 04:05:06 | 2015-02-12 22:05:06-06 | f        | f        | t
(1 row)

However, it is possible to convert from one zone to another on the backend and discard the zone information:

pqgotest=> SHOW timezone;
    TimeZone     
-----------------
 America/Chicago
(1 row)

pqgotest=> SELECT a, b, a AT TIME ZONE 'America/New_York', b AT TIME ZONE 'America/New_York' FROM (VALUES ('2015-02-13T04:05:06'::timestamp, '2015-02-13T04:05:06+00:00'::timestamptz)) AS x(a,b);
          a          |           b            |        timezone        |      timezone       
---------------------+------------------------+------------------------+---------------------
 2015-02-13 04:05:06 | 2015-02-12 22:05:06-06 | 2015-02-13 03:05:06-06 | 2015-02-12 23:05:06
(1 row)

The short answer is, we are entirely incapable of discerning the correct offset to apply when the backend does not supply it.

Maybe time.Time isn't the appropriate Go type for these Postgres types.

Yeah, it's not really designed for this, but it's very very close.

I think it is not close. Given 2015-02-13 04:05:06, how should the Time object respond to Unix() or Local()?

The interface on Time is very wide and I don't think we can populate the struct correctly for every use case.