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: handle Timestamp values with fractional seconds < 1 microsecond correctly in PreparedStatement arguments #1119

Merged
merged 1 commit into from Feb 17, 2018

Conversation

Projects
None yet
3 participants
@stellingsimon
Contributor

stellingsimon commented Feb 17, 2018

When Timestamp values with nanosecond values < 1000 are submitted as arguments to a PreparedStatement,
avoid formatting them with a trailing dot '.' after the seconds part. This fixes a regression introduced
with PR #896.

closes #1117

Implementation note: the rounding with Math.round is necessary to ensure consistent behavior when inserting a timestamp value inlined or as prepared statement argument:

This will insert 2000-02-07 15:00:00.000001:

INSERT INTO testtable (col)
VALUES (timestamp '2000-02-07 15:00:00.000000789')

whereas this Statement

INSERT INTO testtable (col)
VALUES (timestamp ?)

will first format the timestamp as a string with microsecond precision and send that to the server. Without rounding, there would be no way to make both the testSetTimestampWOTZ and testGetTimestampWOTZ green at the same time, no matter whether I expect 0.000000789 to be rounded up or down.

@codecov-io

This comment has been minimized.

codecov-io commented Feb 17, 2018

Codecov Report

Merging #1119 into master will increase coverage by 0.02%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master    #1119      +/-   ##
============================================
+ Coverage     67.23%   67.25%   +0.02%     
- Complexity     3667     3669       +2     
============================================
  Files           170      170              
  Lines         15631    15632       +1     
  Branches       2528     2528              
============================================
+ Hits          10510    10514       +4     
+ Misses         3932     3931       -1     
+ Partials       1189     1187       -2
// 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.
//
if (nanos == 0) {
int microseconds = (int) Math.round(nanos / 1000.0);

This comment has been minimized.

@vlsi

vlsi Feb 17, 2018

Member

It looks like a precision loss is possible here.
I think nanos/1000 + (nanos%1000+500)/1000 is better (or just if (nanos%1000>500) micros++;)

@@ -675,6 +676,7 @@ private static void appendTime(StringBuilder sb, int hours, int minutes, int sec
sb.deleteCharAt(end);
end--;
}

This comment has been minimized.

@vlsi

vlsi Feb 17, 2018

Member

is it required?

fix: handle Timestamp values with fractional seconds < 1 microsecond …
…correctly in PreparedStatement arguments

When Timestamp values with nanosecond values < 1000 are submitted as arguments to a PreparedStatement,
avoid formatting them with a trailing dot '.' after the seconds part. This fixes a regression introduced
with PR #896.

closes #1117

@stellingsimon stellingsimon force-pushed the stellingsimon:bugfix/1117-sub-microsecond-timestamps branch from 8d39cf1 to 1d610e8 Feb 17, 2018

@stellingsimon

This comment has been minimized.

Contributor

stellingsimon commented Feb 17, 2018

@vlsi Thanks for the quick response. I'm not sure if there is in fact a relevant precision loss, but your version works just as well, so here we go again :-)

@vlsi vlsi added this to the 42.2.2 milestone Feb 17, 2018

@vlsi vlsi merged commit 8ff2a61 into pgjdbc:master Feb 17, 2018

2 checks passed

codecov/project 67.25% (+0.02%) compared to a8ef9f9
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

rhavermans added a commit to bolcom/pgjdbc that referenced this pull request Jul 13, 2018

fix: handle Timestamp values with fractional seconds < 1 microsecond …
…correctly in PreparedStatement arguments (pgjdbc#1119)

When Timestamp values with nanosecond values < 1000 are submitted as arguments to a PreparedStatement,
avoid formatting them with a trailing dot '.' after the seconds part. This fixes a regression introduced
with PR pgjdbc#896.

closes pgjdbc#1117

rhavermans added a commit to bolcom/pgjdbc that referenced this pull request Jul 13, 2018

fix: handle Timestamp values with fractional seconds < 1 microsecond …
…correctly in PreparedStatement arguments (pgjdbc#1119)

When Timestamp values with nanosecond values < 1000 are submitted as arguments to a PreparedStatement,
avoid formatting them with a trailing dot '.' after the seconds part. This fixes a regression introduced
with PR pgjdbc#896.

closes pgjdbc#1117
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment