LocalDateTime has invalid string serialization

[kdubb]

The following shows that a LocalDateTime is always always tethered to the current time zone during insertion.

pgjdbc/pgjdbc/src/main/java/org/postgresql/jdbc/TimestampUtils.java

Line 833 in ebada4a

// LocalDateTime is always passed with time zone so backend can decide between timestamp and timestamptz

This is incorrect. A timestamp column has no timezone and therefore should never be altered during insertion/retrieval to a timestamp column. Tethering it to a time zone on insertion can (and does!) cause changes to the inserted time which are then apparent during retrieval.

The specific example is SetObject310Text.testSetLocalDateTime which tests a number of time zones including Europe/Moscow.

In that time zone it tests a time that exists in a one hour time void at 2000-03-26 2:00 (i.e 2:00 to 2:59 doesn't exist in Moscow on this date). During insertion the LocalDateTime value of 2000-03-26 2:00 is tethered to Moscow time and then formatted as a string which results in 2000-03-26 3:00 being inserted into the database due to the void.

The test itself even has to tether the LocalDateTime to a time zone just to test the value correctly.

pgjdbc/pgjdbc/src/test/java/org/postgresql/test/jdbc42/SetObject310Test.java

Line 202 in ebada4a

String expected = localDateTime.atZone(zone)

Two issues

1. The time is still inserted without a time zone so the tethering to Moscow time only affected the string conversion and then disappears.
2. This is a local date/time and the column it's inserting into is a timestamp (also local). If 2000-03-26 2:00 is inserted as 2000-03-26 3:00 in Moscow and read back in a different time zone it will have the wrong value for no explicable reason.

[vlsi]

Tethering it to a time zone on insertion can (and does!) cause changes to the inserted time which are then apparent during retrieval.

@kdubb , would you please provide a test case with expected/actual output?

[kdubb]

The reference test SetObject310Text.testSetLocalDateTime is already in place and does it, as explained above. The issue is that it tests the incorrect value. #1391 contains the proposed fix and alteration to the test to make it correct.

[vlsi]

It is not clear which JDBC call sequence produces wrong result.
In #1391 you alter both implementation and the test code at once, so it is not clear which bug do you fix.

I appreciate you dig into timezone issues (which is not wildly understood), however I would like to have a trivial test-case before we introduce yet another breaking change.

Note: pgjdbc always sends timstamps as UNSPECIFIED type, and it appends timezone just in case.
Have you seen this:

pgjdbc/pgjdbc/src/main/java/org/postgresql/jdbc/PgPreparedStatement.java

Lines 1247 to 1264 in 178eecc

	// We must use UNSPECIFIED here, or inserting a Date-with-timezone into a
	// timestamptz field does an unexpected rotation by the server's TimeZone:
	//
	// We want to interpret 2005/01/01 with calendar +0100 as
	// "local midnight in +0100", but if we go via date it interprets it
	// as local midnight in the server's timezone:
	// template1=# select '2005-01-01+0100'::timestamptz;
	// timestamptz
	// ------------------------
	// 2005-01-01 02:00:00+03
	// (1 row)
	// template1=# select '2005-01-01+0100'::date::timestamptz;
	// timestamptz
	// ------------------------
	// 2005-01-01 00:00:00+03
	// (1 row)

?

[kdubb]

Use #1391, but discard the changes in TimestampUtils.java and you'll see the error.

It attempts to insert a LocalDateTime value of 2000-03-26 2:00 but the database receives 2000-03-26 3:00 (i.e. time jumped by 1 hour).

I believe that's the invalid "call sequence" you're looking for.

[mpilone]

I added a comment to #1108 (comment) about this same issue. My comment would probably be more correct on this ticket but it applied to TZ handling in the driver in general.

I provided a test case on #1108 that shows this exact issue with LocalDateTime. If you're in a TZ that honor DST, you end up with an hour of the year (depending on your TZ rules) where timestamp insertions are incorrectly shifted.

[alexbishop1]

Here’s a test case:

public static void main(String[] args) throws SQLException {
    String url = "jdbc:postgresql://localhost/test?user=me";

    String createTableSql = "CREATE TABLE IF NOT EXISTS dates_and_times (" +
            "id SERIAL PRIMARY KEY, " +
            "date_time TIMESTAMP WITHOUT TIME ZONE" +
            ")";

    String insertSql = "INSERT INTO dates_and_times(date_time) VALUES(?)";

    try (Connection conn = getConnection(url);
         Statement createTableStmt = conn.createStatement();
         PreparedStatement insertTimestampPs = conn.prepareStatement(insertSql)
    ) {
        createTableStmt.execute(createTableSql);

        TimeZone.setDefault(TimeZone.getTimeZone("Europe/Paris"));

        // This time did not exist in the Europe/Paris time zone because it
        // fell into the hour skipped when Paris transitioned from Central
        // European Time to Central European Summer Time
        LocalDateTime localDateTime = LocalDateTime.parse("2020-03-29T02:30");

        insertTimestampPs.setObject(1, localDateTime);
        insertTimestampPs.executeUpdate();

        // If we go and look in the database now, the timestamp will be
        // 2020-03-29 03:30:00 rather than the expected 2020-03-29 02:30:00
    }

}

We can see how this happens in the code:

pgjdbc/pgjdbc/src/main/java/org/postgresql/jdbc/TimestampUtils.java

Lines 904 to 906 in 93d8b68

	// LocalDateTime is always passed with time zone so backend can decide between timestamp and timestamptz
	java.time.ZonedDateTime zonedDateTime = localDateTime.atZone(getDefaultTz().toZoneId());
	return toString(zonedDateTime.toOffsetDateTime());

We start off with a LocalDateTime of 2020-03-29T02:30. This then gets combined with the default time zone (Europe/Paris, in this case) to create a ZonedDateTime. Since there was no such time as 2020-03-29T02:30 in the Europe/Paris time zone, it gets adjusted and we end up with a ZonedDateTime of 2020-03-29T03:30+02:00[Europe/Paris]. This then gets converted to an OffsetDateTime of 2020-03-29T03:30+02:00. After a bit more finessing, it gets passed to the backend, where I understand the offset gets dropped because it’s a TIMESTAMP WITHOUT TIME ZONE column, leaving just 2020-03-29 03:30:00 to be stored.

This only happens during that hour. If our LocalDateTime was an hour earlier, then 2020-03-29T01:30 would become 2020-03-29T01:30+01:00[Europe/Paris], then 2020-03-29T01:30+01:00 and eventually 2020-03-29 01:30:00.

I know JDBC requires that the default time zone be used whenever one is required but I don’t think that applies in this case. Both LocalDateTime and TIMESTAMP WITHOUT TIME ZONE represent year-month-day-hour-minute-second values so a time zone should not be required to convert between one and the other. The fact that an offset has to be passed to the backend is really just an implementation detail of Postgres.

If UTC was always used for the offset, this would solve the problem because UTC always has a consistent offset, so every LocalDateTime can be converted to an ZonedDateTime without the local time changing. However, the comments on #1391 indicate there’s a bit more to it than that. I think this would also change how a LocalDateTime gets stored in a TIMESTAMP WITH TIME ZONE column (not part of the JDBC spec as far as I’m aware but it does seem to be supported).

The Java time API can always provide an offset for a LocalDateTime in a given time zone, even if that time did not exist (so for 2020-03-29T02:30 combined with Europe/Paris, it gives +01:00). This means it might be possible to fix this while still using the default time zone by skipping the intermediate ZonedDateTime and going straight to the OffsetDateTime. Something like this:

java.time.ZoneOffset zoneOffset = getDefaultTz().toZoneId().getRules().getOffset(localDateTime);
return toString(localDateTime.atOffset(zoneOffset));

I think this would work for putting a LocalDateTime in a TIMESTAMP WITHOUT TIME ZONE column (because the offset is always thrown away) but I’m not certain about the other cases that use this code.

[findepi]

@alexbishop1 FWIW you can workaround this by instantiating PGobject and doing the serialization yourself (which requires nothing more than calling toString())
see https://github.com/prestosql/presto/blob/be2b97a8480f657f311e673fa38867c8faf4e2df/presto-postgresql/src/main/java/io/prestosql/plugin/postgresql/TypeUtils.java#L231-L238

[davecramer]

Well as you aptly pointed out the JDBC spec doesn't contemplate both a timestamp with and without a TZ so ultimately we end up getting it right most of the time and wrong sometimes. If I changed it to the behaviour you wanted I would undoubtedly get another issue from someone wanting the behaviour the way it is right now. Timezones are an abomination...