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

fix: adjust date, hour, minute, second when rounding timestamp #1212

Merged
merged 3 commits into from Jun 8, 2018
Merged
Changes from 2 commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.
+88 −11
Diff settings

Always

Just for now

@@ -20,6 +20,7 @@
import java.sql.Time;
import java.sql.Timestamp;
//#if mvn.project.property.postgresql.jdbc.spec >= "JDBC4.2"
import java.time.Duration;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
@@ -48,6 +49,15 @@
private static final char[] ZEROS = {'0', '0', '0', '0', '0', '0', '0', '0', '0'};
private static final char[][] NUMBERS;
private static final HashMap<String, TimeZone> GMT_ZONES = new HashMap<String, TimeZone>();
private static final int MAX_NANOS_BEFORE_WRAP_ON_ROUND = 999999500;
//#if mvn.project.property.postgresql.jdbc.spec >= "JDBC4.2"
private static final Duration ONE_MICROSECOND = Duration.ofNanos(1000);
// LocalTime.MAX is 23:59:59.999_999_999, and it wraps to 24:00:00 when nanos exceed 999_999_499
// since PostgreSQL has microsecond resolution only
private static final LocalTime MAX_TIME = LocalTime.MAX.minus(Duration.ofMillis(500));
private static final OffsetDateTime MAX_OFFSET_DATETIME = OffsetDateTime.MAX.minus(Duration.ofMillis(500));
private static final LocalDateTime MAX_LOCAL_DATETIME = LocalDateTime.MAX.minus(Duration.ofMillis(500));
//#endif

private static final Field DEFAULT_TIME_ZONE_FIELD;

@@ -551,13 +561,23 @@ public synchronized String toString(Calendar cal, Timestamp x,
}

cal = setupCalendar(cal);
cal.setTime(x);
long timeMillis = x.getTime();

// Round to microseconds
int nanos = x.getNanos();
if (nanos >= MAX_NANOS_BEFORE_WRAP_ON_ROUND) {
nanos = 0;
timeMillis++;
} else if (nanos % 1000 >= 500) {
nanos = nanos + 1000 - nanos % 1000;

This comment has been minimized.

Copy link
@bokken

bokken Jun 7, 2018

Member

it appears appendTime now simply truncates at micro precision. that would seem to mean that nanos += 1000; would be sufficient here.

}
cal.setTimeInMillis(timeMillis);

sbuf.setLength(0);

appendDate(sbuf, cal);
sbuf.append(' ');
appendTime(sbuf, cal, x.getNanos());
appendTime(sbuf, cal, nanos);
if (withTimeZone) {
appendTimeZone(sbuf, cal);
}
@@ -654,18 +674,17 @@ private static void appendTime(StringBuilder sb, int hours, int minutes, int sec
sb.append(':');
sb.append(NUMBERS[seconds]);

// Add microseconds, rounded.
// Add nanoseconds.
// This won't work for server versions < 7.2 which only want
// a two digit fractional second, but we don't need to support 7.1
// anymore and getting the version number here is difficult.
//
int microseconds = (nanos / 1000) + (((nanos % 1000) + 500) / 1000);
if (microseconds == 0) {
if (nanos < 1000) {
return;
}
sb.append('.');
int len = sb.length();
sb.append(microseconds);
sb.append(nanos / 1000); // append microseconds
int needZeros = 6 - (sb.length() - len);
if (needZeros > 0) {
sb.insert(len, ZEROS, 0, needZeros);
@@ -733,25 +752,33 @@ public synchronized String toString(LocalTime localTime) {

sbuf.setLength(0);

if (localTime.equals( LocalTime.MAX )) {
if (localTime.isAfter(MAX_TIME)) {
return "24:00:00";
}

int nano = localTime.getNano();
if (nano % 1000 >= 500) {

This comment has been minimized.

Copy link
@lukeindykiewicz

lukeindykiewicz Jun 8, 2018

This expression (nano % 1000 >= 500) is used in more than one place, why don't extract it to a function?

This comment has been minimized.

Copy link
@vlsi

vlsi Jun 8, 2018

Author Member

Any ideas for the function name?

This comment has been minimized.

Copy link
@lukeindykiewicz

lukeindykiewicz Jun 8, 2018

isNanoCarry?
isNanoOverflow may be misleading.

localTime = localTime.plus(ONE_MICROSECOND);
}
appendTime(sbuf, localTime);

return sbuf.toString();
}


public synchronized String toString(OffsetDateTime offsetDateTime) {
if (OffsetDateTime.MAX.equals(offsetDateTime)) {
if (offsetDateTime.isAfter(MAX_OFFSET_DATETIME)) {
return "infinity";
} else if (OffsetDateTime.MIN.equals(offsetDateTime)) {
return "-infinity";
}

sbuf.setLength(0);

int nano = offsetDateTime.getNano();
if (nano % 1000 >= 500) {
offsetDateTime = offsetDateTime.plus(ONE_MICROSECOND);
}
LocalDateTime localDateTime = offsetDateTime.toLocalDateTime();
LocalDate localDate = localDateTime.toLocalDate();
appendDate(sbuf, localDate);
@@ -768,7 +795,7 @@ public synchronized String toString(OffsetDateTime offsetDateTime) {
* Do not use this method in {@link java.sql.ResultSet#getString(int)}
*/
public synchronized String toString(LocalDateTime localDateTime) {
if (LocalDateTime.MAX.equals(localDateTime)) {
if (localDateTime.isAfter(MAX_LOCAL_DATETIME)) {
return "infinity";
} else if (LocalDateTime.MIN.equals(localDateTime)) {
return "-infinity";
@@ -336,6 +336,8 @@ public void testGetTimestampWOTZ() throws SQLException {
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS8WOTZ_PGFORMAT + "'")));
assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS9WOTZ_PGFORMAT + "'")));
assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS10WOTZ_PGFORMAT + "'")));

assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS1WOTZ_PGFORMAT + "'")));
@@ -355,6 +357,8 @@ public void testGetTimestampWOTZ() throws SQLException {
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS8WOTZ_PGFORMAT + "'")));
assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS9WOTZ_PGFORMAT + "'")));
assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS10WOTZ_PGFORMAT + "'")));

assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS1WOTZ_PGFORMAT + "'")));
@@ -374,6 +378,8 @@ public void testGetTimestampWOTZ() throws SQLException {
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS8WOTZ_PGFORMAT + "'")));
assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS9WOTZ_PGFORMAT + "'")));
assertEquals(1,
stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE, "'" + TS10WOTZ_PGFORMAT + "'")));

assertEquals(1, stmt.executeUpdate(TestUtil.insertSQL(TSWOTZ_TABLE,
"'" + tsu.toString(null, new java.sql.Timestamp(tmpDate1WOTZ.getTime())) + "'")));
@@ -412,7 +418,7 @@ public void testGetTimestampWOTZ() throws SQLException {
// Fall through helper
timestampTestWOTZ();

assertEquals(43, stmt.executeUpdate("DELETE FROM " + TSWOTZ_TABLE));
assertEquals(46, stmt.executeUpdate("DELETE FROM " + TSWOTZ_TABLE));

stmt.close();
}
@@ -458,6 +464,9 @@ public void testSetTimestampWOTZ() throws SQLException {
pstmt.setTimestamp(1, TS9WOTZ);
assertEquals(1, pstmt.executeUpdate());

pstmt.setTimestamp(1, TS10WOTZ);
assertEquals(1, pstmt.executeUpdate());

// With java.sql.Timestamp
pstmt.setObject(1, TS1WOTZ, Types.TIMESTAMP);
assertEquals(1, pstmt.executeUpdate());
@@ -477,6 +486,8 @@ public void testSetTimestampWOTZ() throws SQLException {
assertEquals(1, pstmt.executeUpdate());
pstmt.setObject(1, TS9WOTZ, Types.TIMESTAMP);
assertEquals(1, pstmt.executeUpdate());
pstmt.setObject(1, TS10WOTZ, Types.TIMESTAMP);
assertEquals(1, pstmt.executeUpdate());

// With Strings
pstmt.setObject(1, TS1WOTZ_PGFORMAT, Types.TIMESTAMP);
@@ -497,6 +508,8 @@ public void testSetTimestampWOTZ() throws SQLException {
assertEquals(1, pstmt.executeUpdate());
pstmt.setObject(1, TS9WOTZ_PGFORMAT, Types.TIMESTAMP);
assertEquals(1, pstmt.executeUpdate());
pstmt.setObject(1, TS10WOTZ_PGFORMAT, Types.TIMESTAMP);
assertEquals(1, pstmt.executeUpdate());

// With java.sql.Date
pstmt.setObject(1, tmpDate1WOTZ, Types.TIMESTAMP);
@@ -536,7 +549,7 @@ public void testSetTimestampWOTZ() throws SQLException {
// Fall through helper
timestampTestWOTZ();

assertEquals(43, stmt.executeUpdate("DELETE FROM " + TSWOTZ_TABLE));
assertEquals(46, stmt.executeUpdate("DELETE FROM " + TSWOTZ_TABLE));

pstmt.close();
stmt.close();
@@ -715,6 +728,15 @@ private void timestampTestWOTZ() throws SQLException {
tString = rs.getString(1);
assertNotNull(tString);
assertEquals(TS9WOTZ_ROUNDED_PGFORMAT, tString);

assertTrue(rs.next());
t = rs.getTimestamp(1);
assertNotNull(t);
assertEquals(TS10WOTZ_ROUNDED, t);

tString = rs.getString(1);
assertNotNull(tString);
assertEquals(TS10WOTZ_ROUNDED_PGFORMAT, tString);
}

// Testing for Date
@@ -889,6 +911,13 @@ private void timestampTestWOTZ() throws SQLException {
getTimestamp(2000, 2, 7, 15, 0, 0, 1000, null);
private static final String TS9WOTZ_ROUNDED_PGFORMAT = "2000-02-07 15:00:00.000001";

private static final java.sql.Timestamp TS10WOTZ =
getTimestamp(2000, 2, 7, 23, 59, 59, 999999500, null);
private static final String TS10WOTZ_PGFORMAT = "2000-02-07 23:59:59.999999500";
private static final java.sql.Timestamp TS10WOTZ_ROUNDED =
getTimestamp(2000, 2, 8, 0, 0, 0, 0, null);
private static final String TS10WOTZ_ROUNDED_PGFORMAT = "2000-02-08 00:00:00";

private static final String TSWTZ_TABLE = "testtimestampwtz";
private static final String TSWOTZ_TABLE = "testtimestampwotz";
private static final String DATE_TABLE = "testtimestampdate";
@@ -279,6 +279,27 @@ private void offsetTimestamps(ZoneId dataZone, LocalDateTime localDateTime, Stri
}
}

@Test
public void testLocalDateTimeRounding() throws SQLException {
LocalDateTime dateTime = LocalDateTime.parse("2018-06-03T23:59:59.999999500");
localTimestamps(ZoneOffset.UTC, dateTime, "2018-06-04 00:00:00");
}

@Test
public void testTimeStampRounding() throws SQLException {
LocalTime time = LocalTime.parse("23:59:59.999999500");
Time actual = insertThenReadWithoutType(time, "time_without_time_zone_column", Time.class);
assertEquals(Time.valueOf("24:00:00"), actual);
}

@Test
public void testTimeStampRoundingWithType() throws SQLException {
LocalTime time = LocalTime.parse("23:59:59.999999500");
Time actual =
insertThenReadWithType(time, Types.TIME, "time_without_time_zone_column", Time.class);
assertEquals(Time.valueOf("24:00:00"), actual);
}

This comment has been minimized.

Copy link
@lukeindykiewicz

lukeindykiewicz Jun 8, 2018

Please consider writing a test for year change, like for '2018-12-31 23:59:59.999999500' to prevent potential bugs after refactorings in the future.

/**
* Test the behavior of setObject for timestamp columns.
*/
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.