Skip to content

Commit

Permalink
fix: avoid timezone conversions when sending LocalDateTime to the dat…
Browse files Browse the repository at this point in the history
…abase

Previously, LocalDateTime was converted to OffsetDateTime using the default
timezone, which made certain values unrepresentable.
For instance, 2023-03-12T02:00 in America/New_York is hard to represent
as it undergoes DST change.

It does not change resultSet.getObject(int | string), and resultSet.getString(int | string)
and those methods would still have issues when accessing timestamps
like 2023-03-12T02:00 when the client time zone is America/New_York

Co-authored-by: Kevin Wooten <kevin@wooten.com>

Fixes #1390
Fixes #2850
Closes #1391
  • Loading branch information
vlsi committed Dec 6, 2023
1 parent 25fdfc3 commit c1a851c
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 28 deletions.
25 changes: 19 additions & 6 deletions pgjdbc/src/main/java/org/postgresql/jdbc/PgResultSet.java
Original file line number Diff line number Diff line change
Expand Up @@ -2322,6 +2322,25 @@ public boolean wasNull() throws SQLException {
// varchar in binary is same as text, other binary fields are converted to their text format
if (isBinary(columnIndex) && getSQLType(columnIndex) != Types.VARCHAR) {
Field field = fields[columnIndex - 1];
TimestampUtils ts = getTimestampUtils();
// internalGetObject is used in getObject(int), so we can't easily alter the returned type
// Currently, internalGetObject delegates to getTime(), getTimestamp(), so it has issues
// with timezone conversions.
// However, as we know the explicit oids, we can do a better job here
switch (field.getOID()) {
case Oid.TIME:
return ts.toString(ts.toLocalTimeBin(value));
case Oid.TIMETZ:
return ts.toStringOffsetTimeBin(value);
case Oid.DATE:
return ts.toString(ts.toLocalDateBin(value));
case Oid.TIMESTAMP:
return ts.toString(ts.toLocalDateTimeBin(value));
case Oid.TIMESTAMPTZ:
return ts.toStringOffsetDateTime(value);
}
// internalGetObject requires thisRow to be non-null
castNonNull(thisRow, "thisRow");
Object obj = internalGetObject(columnIndex, field);
if (obj == null) {
// internalGetObject() knows jdbc-types and some extra like hstore. It does not know of
Expand All @@ -2332,12 +2351,6 @@ public boolean wasNull() throws SQLException {
}
return obj.toString();
}
// hack to be compatible with text protocol
if (obj instanceof java.util.Date) {
int oid = field.getOID();
return getTimestampUtils().timeToString((java.util.Date) obj,
oid == Oid.TIMESTAMPTZ || oid == Oid.TIMETZ);
}
if ("hstore".equals(getPGType(columnIndex))) {
return HStoreConverter.toString((Map<?, ?>) obj);
}
Expand Down
82 changes: 78 additions & 4 deletions pgjdbc/src/main/java/org/postgresql/jdbc/TimestampUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@
import java.time.OffsetDateTime;
import java.time.OffsetTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.chrono.IsoEra;
import java.time.format.DateTimeParseException;
import java.time.temporal.ChronoField;
Expand Down Expand Up @@ -946,6 +945,36 @@ public String toString(OffsetTime offsetTime) {
}
}

/**
* Converts {@code timetz} to string taking client time zone ({@link #timeZoneProvider})
* into account.
* @param value binary representation of {@code timetz}
* @return string representation of {@code timetz}
*/
public String toStringOffsetTimeBin(byte[] value) throws PSQLException {
OffsetTime offsetTimeBin = toOffsetTimeBin(value);
return toString(withClientOffsetSameInstant(offsetTimeBin));
}

/**
* PostgreSQL does not store the time zone in the binary representation of timetz.
* However, we want to preserve the output of {@code getString()} in both binary and text formats
* So we try a client time zone when serializing {@link OffsetTime} to string.
* @param input input offset time
* @return adjusted offset time (it represents the same instant as the input one)
*/
public OffsetTime withClientOffsetSameInstant(OffsetTime input) {
if (input == OffsetTime.MAX || input == OffsetTime.MIN) {
return input;
}
TimeZone timeZone = timeZoneProvider.get();
int offsetMillis = timeZone.getRawOffset();
return input.withOffsetSameInstant(
offsetMillis == 0
? ZoneOffset.UTC
: ZoneOffset.ofTotalSeconds(offsetMillis / 1000));
}

public String toString(OffsetDateTime offsetDateTime) {
try (ResourceLock ignore = lock.obtain()) {
if (offsetDateTime.isAfter(MAX_OFFSET_DATETIME)) {
Expand Down Expand Up @@ -974,6 +1003,41 @@ public String toString(OffsetDateTime offsetDateTime) {
}
}

/**
* Converts {@code timestamptz} to string taking client time zone ({@link #timeZoneProvider})
* into account.
* @param value binary representation of {@code timestamptz}
* @return string representation of {@code timestamptz}
*/
public String toStringOffsetDateTime(byte[] value) throws PSQLException {
OffsetDateTime offsetDateTime = toOffsetDateTimeBin(value);
return toString(withClientOffsetSameInstant(offsetDateTime));
}

/**
* PostgreSQL does not store the time zone in the binary representation of timestamptz.
* However, we want to preserve the output of {@code getString()} in both binary and text formats
* So we try a client time zone when serializing {@link OffsetDateTime} to string.
* @param input input offset date time
* @return adjusted offset date time (it represents the same instant as the input one)
*/
public OffsetDateTime withClientOffsetSameInstant(OffsetDateTime input) {
if (input == OffsetDateTime.MAX || input == OffsetDateTime.MIN) {
return input;
}
int offsetMillis;
TimeZone timeZone = timeZoneProvider.get();
if (isSimpleTimeZone(timeZone.getID())) {
offsetMillis = timeZone.getRawOffset();
} else {
offsetMillis = timeZone.getOffset(input.toEpochSecond() * 1000L);
}
return input.withOffsetSameInstant(
offsetMillis == 0
? ZoneOffset.UTC
: ZoneOffset.ofTotalSeconds(offsetMillis / 1000));
}

/**
* Formats {@link LocalDateTime} to be sent to the backend, thus it adds time zone.
* Do not use this method in {@link java.sql.ResultSet#getString(int)}
Expand All @@ -988,9 +1052,19 @@ public String toString(LocalDateTime localDateTime) {
return "-infinity";
}

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

if (nanosExceed499(localDateTime.getNano())) {
localDateTime = localDateTime.plus(ONE_MICROSECOND);
}

LocalDate localDate = localDateTime.toLocalDate();
appendDate(sbuf, localDate);
sbuf.append(' ');
appendTime(sbuf, localDateTime.toLocalTime());
appendEra(sbuf, localDate);

return sbuf.toString();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@
import org.postgresql.test.TestUtil;
import org.postgresql.test.jdbc2.BaseTest4;

import org.junit.After;
import org.junit.Before;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;

import java.sql.Connection;
import java.sql.PreparedStatement;
import java.sql.ResultSet;
import java.sql.SQLException;
Expand Down Expand Up @@ -83,22 +84,30 @@ public static Iterable<Object[]> data() {
return ids;
}

@Before
public void setUp() throws Exception {
super.setUp();
TestUtil.createTable(con, "table1", "timestamp_without_time_zone_column timestamp without time zone,"
+ "timestamp_with_time_zone_column timestamp with time zone,"
+ "date_column date,"
+ "time_without_time_zone_column time without time zone,"
+ "time_with_time_zone_column time with time zone"
);
@BeforeClass
public static void createTables() throws Exception {
try (Connection con = TestUtil.openDB();) {
TestUtil.createTable(con, "table1", "timestamp_without_time_zone_column timestamp without time zone,"
+ "timestamp_with_time_zone_column timestamp with time zone,"
+ "date_column date,"
+ "time_without_time_zone_column time without time zone,"
+ "time_with_time_zone_column time with time zone"
);
}
}

@After
public void tearDown() throws SQLException {
@AfterClass
public static void dropTables() throws Exception {
try (Connection con = TestUtil.openDB();) {
TestUtil.dropTable(con, "table1");
}
TimeZone.setDefault(saveTZ);
TestUtil.dropTable(con, "table1");
super.tearDown();
}

@Override
public void setUp() throws Exception {
super.setUp();
TestUtil.execute(con, "delete from table1");
}

private void insert(Object data, String columnName, Integer type) throws SQLException {
Expand Down Expand Up @@ -232,9 +241,7 @@ public void testSetLocalDateTime() throws SQLException {
ZoneId zone = ZoneId.of(zoneId);
for (String date : datesToTest) {
LocalDateTime localDateTime = LocalDateTime.parse(date);
String expected = localDateTime.atZone(zone)
.format(DateTimeFormatter.ISO_LOCAL_DATE_TIME)
.replace('T', ' ');
String expected = date.replace('T', ' ');
localTimestamps(zone, localDateTime, expected);
}
}
Expand Down

0 comments on commit c1a851c

Please sign in to comment.