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: proleptic java.time support #1539

Merged
merged 1 commit into from
Aug 27, 2019

Conversation

marschall
Copy link
Contributor

Make the java.time support proleptic by not going through
TimestampUtils.toJavaSecs when in binary mode.

Fixes #1534

All Submissions:

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

Changes to Existing Features:

  • Does this break existing behaviour? For timestamps in the Julian calendar different java.time objects are returned. Code relying on the old (buggy) behavior will break.
  • Have you added an explanation of what your changes do and why you'd like us to include them? Old behaviour is wrong.
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

@AppVeyorBot
Copy link

Build pgjdbc 1.0.385 completed (commit 005379f0a9 by @marschall)

@marschall
Copy link
Contributor Author

marschall commented Aug 3, 2019

Some comments about the change:

  • I wanted the keep the existing #toJavaSecs and #toPgSecs logic for the old data types. That's why I factored out the binary parsing.
  • Binding does not go through TimestampUtils#toPgSecs because binary binding for java.time types is currently not implemented. I would like to address this in a different change.
  • SetObject310Test had a bug where it would ignore the second column
  • I skip the tests on anything before 8.4. Writing test that also work on older versions would be harder because
    • timestamp and timestamptz support in generate_series was added in 8.4
    • CTE support in added in 8.4

@AppVeyorBot
Copy link

Build pgjdbc 1.0.386 completed (commit b482339489 by @marschall)

@codecov-io
Copy link

codecov-io commented Aug 3, 2019

Codecov Report

Merging #1539 into master will increase coverage by 0.03%.
The diff coverage is 70.83%.

@@             Coverage Diff              @@
##             master    #1539      +/-   ##
============================================
+ Coverage     68.94%   68.97%   +0.03%     
- Complexity     3956     3972      +16     
============================================
  Files           179      179              
  Lines         16491    16546      +55     
  Branches       2678     2688      +10     
============================================
+ Hits          11369    11412      +43     
- Misses         3877     3887      +10     
- Partials       1245     1247       +2

@AppVeyorBot
Copy link

Build pgjdbc 1.0.387 completed (commit 6cc67e1971 by @marschall)

@AppVeyorBot
Copy link

Build pgjdbc 1.0.388 completed (commit bb5228d277 by @marschall)

Make the java.time support proleptic by not going through
TimestampUtils.toJavaSecs when in binary mode.

Fixes pgjdbc#1534
@AppVeyorBot
Copy link

Build pgjdbc 1.0.394 completed (commit 882bbab3ac by @marschall)

@davecramer davecramer merged commit 60fa6d3 into pgjdbc:master Aug 27, 2019
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.

fix times for 1852
4 participants