Skip to content
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

Always use UTC for SQL TIMESTAMP to java.sql.Timestamp conversion #1108

Closed
Davio opened this issue Feb 12, 2018 · 9 comments
Closed

Always use UTC for SQL TIMESTAMP to java.sql.Timestamp conversion #1108

Davio opened this issue Feb 12, 2018 · 9 comments

Comments

@Davio
Copy link

Davio commented Feb 12, 2018

I find it odd that the system calendar plays a role in converting an SQL TIMESTAMP to java.sql.Timestamp.

When I do SELECT EXTRACT(EPOCH FROM mytimestampcolumn) * 1000 FROM mytable it returns the milliseconds since the Unix epoch, say 1507075200000.

If I manually feed this to java.sql.Timestamp with new Timestamp(long millis), it returns the Timestamp of these millis. So far so good.

However, from reading through the source code it seems that when there is no TimeZone obtainable it defaults back to the system timezone for this conversion. This leads to wildly inconsistent behavior.

The reason we used TIMESTAMP in our column type is because we always want to store UTC time and don't have datetimes using different timezones in our column. This conversion messes up our intentions.

For a more consistent behavior and tests, I would suggest that when there is no explicit timezone, for the sake of converting an SQL TIMESTAMP to java.sql.Timestamp (or LocalDateTime?), UTC is always used.
Imaging having a system observing DST and continuously changing its returned values.

You might argue: That's what you get when you don't use SQL TIMESTAMP WITH TZ, but there should be a way to just have a TIMESTAMP column treated as UTC without forcing either the system to be in UTC or forcing the Java TimeZone to UTC.

Relevant code from PgResultSet.java:

  @Override
  public Timestamp getTimestamp(int i, java.util.Calendar cal) throws SQLException {
    checkResultSet(i);
    if (wasNullFlag) {
      return null;
    }

    if (cal == null) {
      cal = getDefaultCalendar(); // This boils down to the default calendar used in Postgres all around which is the system calendar
    }
    int col = i - 1;
    int oid = fields[col].getOID();
    if (isBinary(i)) {
      if (oid == Oid.TIMESTAMPTZ || oid == Oid.TIMESTAMP) {
        boolean hasTimeZone = oid == Oid.TIMESTAMPTZ;
        TimeZone tz = cal.getTimeZone();
        return connection.getTimestampUtils().toTimestampBin(tz, this_row[col], hasTimeZone);
      } else {
        // JDBC spec says getTimestamp of Time and Date must be supported
        long millis;
        if (oid == Oid.TIME || oid == Oid.TIMETZ) {
          millis = getTime(i, cal).getTime();
        } else if (oid == Oid.DATE) {
          millis = getDate(i, cal).getTime();
        } else {
          throw new PSQLException(
              GT.tr("Cannot convert the column of type {0} to requested type {1}.",
                  Oid.toString(oid), "timestamp"),
              PSQLState.DATA_TYPE_MISMATCH);
        }
        return new Timestamp(millis);
      }
    }

Which eventually ends up here:

private TimeZone getDefaultTz() {
    // Fast path to getting the default timezone.
    if (DEFAULT_TIME_ZONE_FIELD != null) {
      try {
        TimeZone defaultTimeZone = (TimeZone) DEFAULT_TIME_ZONE_FIELD.get(null);
        if (defaultTimeZone == prevDefaultZoneFieldValue) {
          return defaultTimeZoneCache;
        }
        prevDefaultZoneFieldValue = defaultTimeZone;
      } catch (Exception e) {
        // If this were to fail, fallback on slow method.
      }
    }
    TimeZone tz = TimeZone.getDefault(); // Here is uses the system default timezone
    defaultTimeZoneCache = tz;
    return tz;
  }
@davecramer
Copy link
Member

Well Postgres always stores all dates and times in UTC see
'For timestamp with time zone, the internally stored value is always in UTC (Universal Coordinated Time, traditionally known as Greenwich Mean Time, GMT).'

If you want to output your data as UTC why not just use getTimestatmp(i,Cal) ?

@Davio
Copy link
Author

Davio commented Feb 12, 2018

I am not calling the driver directly, but using it through JPA. I think JPA just uses default methods from PgResultSet (which implements ResultSet) to obtain its values. PgResultSet has some overloads for getTimeStamp(String columnName or int columnIndex) which delegate to getTimeStamp(column, Calendar cal), passing null as the Calendar here. This results in the JDBC driver obtaining the default system calendar instead of UTC.

@vlsi
Copy link
Member

vlsi commented Feb 12, 2018

This results in the JDBC driver obtaining the default system calendar instead of UTC.

I assume you are using 42.2.1, then pgjdbc works as designed / specified by JDBC spec.

You might have luck with UTC-only environment if you set all the default time zones to UTC, and pass -Duser.timezone=UTC to the JVM arguments.

I'm inclined to close this as the issues does sound like a coding issue rather than pgjdbc defect.

@bokken
Copy link
Member

bokken commented Feb 12, 2018

It really sounds like timestamp with timezone is the behavior you want.

@vlsi
Copy link
Member

vlsi commented Mar 9, 2018

@Davio , would you please provide more details? For instance, a sample code that "does not work" as expected would be great.

Otherwise I'm inlined to close the issue. Open issues do consume maintenance time.

@Davio
Copy link
Author

Davio commented Mar 10, 2018 via email

@vladimir-lu
Copy link

vladimir-lu commented Sep 5, 2018

As far as I can see, the JDBC 4.1 specification itself says nothing about the default timezone a java.sql.Timestamp class should have.

However the current situation is problematic. Suppose you create a table:

create table entries(
  timestamp timestamp without time zone
);

insert into entries values ('2018-07-11 13:33:09');

And then you run the following code on a machine where the local timezone is not GMT:

import java.sql.DriverManager
import java.time.{ZoneId, ZonedDateTime}
 
object jdbctest extends App {
  // Ensure your timezone is not UTC
 
  val url = "jdbc:postgresql://localhost/postgres?user=postgres"
  val conn = DriverManager.getConnection(url)
 
  val instant = ZonedDateTime.of(2018, 7, 11, 13, 33, 9, 0, ZoneId.of("UTC")).toInstant
  println(instant)
 
  val stmt = conn.prepareStatement("select count(1) from entries where timestamp = ?")
  stmt.setTimestamp(1, java.sql.Timestamp.from(instant))
 
  // case 1: converted instant 
  val rs = stmt.executeQuery()
  if (rs.next())
    println(s"Count with converted instant: ${rs.getInt(1)}")
 
  // case 2: literal timestamp
  stmt.setTimestamp(1, java.sql.Timestamp.valueOf("2018-07-11 13:33:09"))
 
  val rs2 = stmt.executeQuery()
  if (rs2.next())
    println(s"Count with timestamp.valueOf: ${rs2.getInt(1)}")
}

The example is in Scala, but I think it should be understandable. The output is:

2018-07-11T13:33:09Z
Count with converted instant: 0
Count with timestamp.valueOf: 1

So a java.time.Instant (which is always a UTC time) when converted to a java.sql.Timestamp is not the same as the same instant in time parsed as a string manually.

This behavior is unexpected - an Instant and a timestamp without time zone should be directly convertible, without being dependent on the JVM's local timezone.

I am not sure where the right place for a fix is though - we can't change the JVM behaviour or probably even existing behaviour of the library :/

Edit:

The setting of the JVM timezone seems to be the workaround everybody uses, see 1

@mpilone
Copy link

mpilone commented Mar 10, 2019

PG JDBC: 42.2.2
PG: 9.6

I just ran into a similar issue when using LocalDateTime. I originally ported some code over from using setTimestamp(int, Timestamp, Calendar) to using LocalDateTime and everything seemed fine until 9PM EST March 9th. At that point I found that I had a bug in production because all times were being shifted to 10PM for some reason.

I store all times in the DB as UTC in a TIMESTAMP column and my JVM's run in America/New_York TZ. My assumption was that the TIMESTAMP DB type doesn't have a time zone so mapping to/from a LocalDateTime (the equivalent Java type) should be straight forward. Just use the exact same date-time information I'm providing and store it in the column.

However what I found while looking into the 9PM EST bug is that the LocalDateTime is converted to a ZonedDateTime in TimestampUtils.java:777 and assigned the local time zone. This causes an issue because my code is converting March 9th 2100 EST to March 10th 0200 UTC in the LocalDateTime, the value I want to store in the DB. The JDBC driver is then creating a ZonedDateTime of March 10th 0200 America/New_York but this time doesn't exist because of the DST transition. ZonedDateTime then adjusts to March 10th 0300 and stores that value in the DB!

So for this one hour of the year, saving LocalDateTime doesn't work!

I find the date-time handling really odd in the driver. Maybe some of this is the result of the specification but I've included a test case that shows that for two different date-times, you can get really different results with the different driver methods. The only one that seems to get the result I actually want (UTC in the DB) and seems to handle DST issues is the older setTimestamp(int, Timestamp, Calendar). I would have expected LocalDateTime to work by not modifing the date-time provided or OffsetDateTime to work because I'm providing a TZ offset similar to using Calendar.

Output of the test:

Test time: 2019-03-09T20:00:00-05:00
LocalDateTime UTC before insert: 2019-03-10T01:00
LocalDateTime UTC after query: 2019-03-10T01:00
DB time string: 2019-03-10 01:00:00
OffsetDateTime UTC before insert: 2019-03-10T01:00Z
OffsetDateTime UTC after query: 2019-03-10T01:00Z
DB time string: 2019-03-09 20:00:00   // Stored in local time?!?
Timestamp Calendar UTC before insert: 2019-03-09 20:00:00.0 (2019-03-10T01:00Z[Etc/UTC])
Timestamp Calendar UTC after query: 2019-03-09 20:00:00.0 (2019-03-10T01:00Z[Etc/UTC])
DB time string: 2019-03-10 01:00:00
Timestamp before insert: 2019-03-09 20:00:00.0 (2019-03-10T01:00Z[Etc/UTC])
Timestamp after query: 2019-03-09 20:00:00.0 (2019-03-10T01:00Z[Etc/UTC])
DB time string: 2019-03-09 20:00:00  // Stored in local time?!?

=================
Test time: 2019-03-09T21:00:00-05:00
LocalDateTime UTC before insert: 2019-03-10T02:00
LocalDateTime UTC after query: 2019-03-10T03:00
DB time string: 2019-03-10 03:00:00 // Completely broken. Wrong UTC time!?!
OffsetDateTime UTC before insert: 2019-03-10T02:00Z
OffsetDateTime UTC after query: 2019-03-10T02:00Z
DB time string: 2019-03-09 21:00:00  // Stored in local time?!?
Timestamp Calendar UTC before insert: 2019-03-09 21:00:00.0 (2019-03-10T02:00Z[Etc/UTC])
Timestamp Calendar UTC after query: 2019-03-09 21:00:00.0 (2019-03-10T02:00Z[Etc/UTC])
DB time string: 2019-03-10 02:00:00
Timestamp before insert: 2019-03-09 21:00:00.0 (2019-03-10T02:00Z[Etc/UTC])
Timestamp after query: 2019-03-09 21:00:00.0 (2019-03-10T02:00Z[Etc/UTC])
DB time string: 2019-03-09 21:00:00  // Stored in local time?!?

Test code attached.
PgTimeZoneBug1108Test.java.zip

@davecramer
Copy link
Member

JDBC 2.1 specifies that the driver use the default calendar for timestamps that do not provide a timezone. Section 10.5 about Date, Time and Timestamps

Since a Calendar was not supplied explicitly to getDate(), the default time zone (really the default Calendar) is used by the JDBC driver
internally to create the appropriate millisecond value assuming that the underlying database doesn’t store time zone information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants