Decoding java.time.Instant is not correct when temporal is LocalDateTime

[dstepanov]

r2dbc-postgresql/src/main/java/io/r2dbc/postgresql/codec/InstantCodec.java

Lines 44 to 54 in 9c773c8

	return decodeTemporal(buffer, dataType, format, Instant.class, temporal -> {
	if (temporal instanceof LocalDateTime) {
	return ((LocalDateTime) temporal).toInstant(ZoneOffset.UTC);
	}
	if (temporal instanceof LocalDate) {
	return ((LocalDate) temporal).atStartOfDay(ZoneId.systemDefault()).toInstant();
	}
	return Instant.from(temporal);

I'm not a Java time expert but I think the correct way to convert LocalDateTime to Instant is to do this:

((LocalDateTime) temporal).atZone(ZoneId.systemDefault()).toInstant()

In my test case binding and reading an instant produces different values shifted by my local timezone.

[mp911de]

Thanks for the report. We tried to align with the JDBC driver, need to check what's going on.

[davecramer]

Have I mentioned how much I dislike timezones ?

[dstepanov]

[davecramer]

Thanks for the report. We tried to align with the JDBC driver, need to check what's going on.

When you find out, let me know :)

[mp911de]

Here's the result of my analysis based on the TIMESTAMPTZ type (that's the only one that can be represented properly as Instant):

1. Calling Row.get(0) returns OffsetDateTime
2. Calling Row.get(0, OffsetDateTime.class) returns OffsetDateTime containing the timezone offset. Converting it to Instant yields what 3. returns.
3. Calling Row.get(0, Instant.class) returns Instant at the UTC timezone.
4. Calling Row.get(0, LocalDateTime.class) returns LocalDateTime that has the value of the Instant pretending to be UTC. (Local to UTC, not local to the client time zone).

The LocalDateTime conversion at UTC was a deliberate choice as the database stores values at UTC (See https://www.postgresql.org/docs/current/datatype-datetime.html#id-1.5.7.13.20.2, 8.5.3. Time Zones).

In contrast, the JDBC driver works as follows:

1. Calling ResultSet.getObject(1) returns Timestamp
2. Calling ResultSet.getObject(1, OffsetDateTime.class) returns OffsetDateTime
3. Calling ResultSet.getObject(1, LocalDateTime.class) fails with Cannot convert the column of type TIMESTAMPTZ to requested type timestamp.

Coming back to the initial report, a zoned temporal value requires local zone information for conversion into a Local… value. In our case, we stick to UTC instead of the client system zone. Likely, we should align the LocalDate conversion which uses atStartOfDay(ZoneId.systemDefault()) as well.

See https://gist.github.com/mp911de/301bcdb05ca16b6f9d8eda51f76653bb for a reproducer.

[davecramer]

2. Calling Row.get(0, OffsetDateTime.class) returns OffsetDateTime containing the timezone offset. Converting it to Instant yields what 3. returns.

At which timezone ? Since the database does not store timezone information. What it does is use the session timezone. Do we have a way to specify the session timezone ?

[mp911de]

The Postgres response string was 2022-06-13 11:21:41.322+02 (text). Currently, we do not support configuring the TimeZone at startup (it is set to TimeZone.getDefault().getID()), but it would be a good idea anyway to have timezone information when decoding binary TIMESTAMPTZ.

So going forward, the TIMESTAMPTZ conversion to LocalDateTime works as designed, we should set the timezone within the session and align LocalDate conversion to UTC as well for a consistent behavior.

[davecramer]

What the JDBC driver does is set the PostgreSQL session timezone to the clients timezone. So regardless of the servers timezone the client sees the date in their timezone. Further it's my understanding that if you don't set the session timezone you will get the timezone that is set in the server. Imagine querying data which is spread across multiple timezones. Unless the server timezone is set to UTC, you would get a different answer from each timezone. Further if somehow the timezone of the server were to be changed the times would also change. (BTW, did I mention I dislike timezones )

[mp911de]

I plan to replicate the behavior with #521. Additionally, I want to introduce the possibility of configuring the time zone via #521

[mp911de]

done.