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: #1677 NumberFormatException when fetching PGInterval with small value #1678

Merged
merged 4 commits into from Jan 28, 2020

Conversation

baardsen
Copy link
Contributor

@baardsen baardsen commented Jan 24, 2020

Prevents getting a string with scientific notation by using an explicit format string. Uses a fixed precision which makes testing for the presence of decimal point and calculating a coefficient to get to microseconds unnecessary.

Fixes: #1677

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? If so please explain.
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable?
  • Have you successfully run tests with your changes locally?

days,
hours,
minutes,
wholeSeconds + microSeconds / 1e6d
Copy link
Member

@davecramer davecramer Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've started using TimeUnit.* to convert to and from time units. It's a bit more readable.

@@ -367,20 +376,10 @@ public int getMicroSeconds() {
* @param seconds seconds to set
*/
public void setSeconds(double seconds) {
String str = Double.toString(seconds);
String str = String.format(Locale.ROOT,"%.6f",seconds);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be something like

wholeSeconds = (int) seconds
microSeconds  = (seconds - wholeSeconds) / 1_000_000.0;

Copy link
Member

@davecramer davecramer Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well I have tried the _ thing and sadly java 6 doesn't understand it. Once we dump Java 6 we can use that.
If you really require double then just create a private static final CONSTANT. I'll be fine with that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely a constant can be introduced.
I mean that this operation can work only with numbers, without conversion to string.

and while discussing this, seems like there is an issue with getSeconds() which currently does
Double.parseDouble("" + wholeSeconds + '.' + microSeconds )
if wholeSeconds=1 and microSeconds=1 then result should be 1.000001 not 1.1

Copy link
Contributor Author

@baardsen baardsen Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can introduce a constant and fix the getSeconds method. I'll just use the conversion logic from getValue.

In terms of the setSeconds logic, should 9e-7 be rounded nearest to 1 microsecond or can we just cast it down to 0?

Copy link
Member

@davecramer davecramer Jan 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would appear we need a few more test cases!

@davecramer davecramer merged commit e38868b into pgjdbc:master Jan 28, 2020
1 check passed
paplorinc added a commit to paplorinc/pgjdbc that referenced this issue Feb 10, 2020
* origin/master: (427 commits)
  refactor: make PSQLState enum consts for integrity constraint violations (pgjdbc#1699)
  [maven-release-plugin] prepare for next development iteration
  [maven-release-plugin] prepare release REL42.2.10
  pass gpg key through arguments
  add passphrase to release mvn task
  chore: update signing key
  Metadata queries improvment (pgjdbc#1694)
  WIP release notes for 42.2.10 (pgjdbc#1688)
  chore(deps): bump checkstyle from 8.28 to 8.29 in /pgjdbc (pgjdbc#1691)
  Cleanup PGProperty, sort values, and add some missing to docs (pgjdbc#1686)
  fix: Fixes issue pgjdbc#1592 where one thread is reading the copy and another thread closes the connection (pgjdbc#1594)
  Fixing LocalTime rounding (losing precision) (pgjdbc#1570)
  sync error message value with tested value (pgjdbc#1664)
  add DatabaseMetaDataCacheTest to test suite to run it (pgjdbc#1685)
  Fix Network Performance of PgDatabaseMetaData.getTypeInfo() method (pgjdbc#1668)
  fix: Issue pgjdbc#1680 updating a boolean field requires special handling to set it to t or f instead of true or false (pgjdbc#1682)
  fix testSetNetworkTimeoutEnforcement test failure (pgjdbc#1681)
  minor: fix checkstyle violation of unused import (pgjdbc#1683)
  fix: pgjdbc#1677 NumberFormatException when fetching PGInterval with small value (pgjdbc#1678)
  fix: actually use milliseconds instead of microseconds for timeouts (pgjdbc#1653)
  ...
davecramer pushed a commit to davecramer/pgjdbc that referenced this issue Jul 5, 2021
…small value (pgjdbc#1678)

* fix: pgjdbc#1677 NumberFormatException when fetching PGInterval with small value

* fix: PGInterval.getValue for microsecond values

* ensure getValue always uses at least one decimal place for seconds

* fix: PGInterval.getSeconds for microsecond values
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.

3 participants