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

Nanosecond part of binary-received float8-timestamp needs to be rounded. #1793

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

horiguti
Copy link
Contributor

@horiguti horiguti commented Jun 5, 2020

If a float8-timestamp is received on a binaryTransfer connection, its
nanosecond part is just truncated in toParsedTimestampBinPlain. If the
result of the subtraction just before gets slightly smaller, the
result gets smaller by 1 microsecond. Nanosecond part is rounded off
on server side so it should be handled the same way.

This problem can be seen by the following steps, with pgjdbc versions from 9_2 where binaryTransfer was introduced.

Create a table on a PostgreSQL server built with --disable-integer-datetimes:
CREATE TABLE t AS (SELECT '2020-6-5 0:0:0.464'::timestamp as a);

Access by a program like the follows:
public static void main (String args[]) {

 String url = "jdbc:postgresql:postgres?binaryTransfer=true"; // URL

 try {
   Class.forName("org.postgresql.Driver");
   Connection con = DriverManager.getConnection(url);
   PreparedStatement pst = con.prepareStatement("SELECT a from t;");
   for (int i = 0 ; i < 6 ; i++) {
     ResultSet rs = pst.executeQuery();
     while (rs.next())
       System.out.format("%d: %s\n", i, rs.getTimestamp(1).toString());
     rs.close();
   }

You should see the following result. The last one is the issue here.

0: 2020-06-05 00:00:00.464
1: 2020-06-05 00:00:00.464
2: 2020-06-05 00:00:00.464
3: 2020-06-05 00:00:00.464
4: 2020-06-05 00:00:00.464
5: 2020-06-05 00:00:00.463999

reagrds.

If a float8-timestamp is received on a binaryTransfer connection, its
nanosecond part is just truncated in toParsedTimestampBinPlain. If the
result of the subtraction just before gets slightly smaller, the
result gets smaller by 1 microsecond. Nanosecond part is rounded off
on server side so it should be handled the same way.
@horiguti
Copy link
Contributor Author

horiguti commented Jun 5, 2020

In second thought, just adding 0.5 seems to be better than using Math.round().

The result in double is not needed, so it is enough to just add +0.5
before casting to integer.
@davecramer
Copy link
Member

I'm surprised anyone uses float8 timestamps. Is there a reason for this ?

@horiguti
Copy link
Contributor Author

horiguti commented Jun 8, 2020

I'm surprised anyone uses float8 timestamps. Is there a reason for this ?

I'm surprised, too:p, and I'm not sure about the reason for the system(w/ PG9.6) being configured with float timestamps, nor the reason for upgrading JDBC driver from 9.0 to 42..

@davecramer
Copy link
Member

So how do we test this ?

@horiguti
Copy link
Contributor Author

The added test checks the behavior for the specific value, but that would be sufficient.
It fails with 9.6/float-datetimes without this fix, and succeeds with 9.6/float-datetimes with this fix. It skips the test when testing server is configured with integer-datetimes.

@davecramer
Copy link
Member

@horiguti can you fix the style issues? Ideally I would like the name of the test to be more aligned with what it is actually testing.

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

Successfully merging this pull request may close these issues.

None yet

2 participants