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

BigQuery: FieldValue.getTimestamp() use of Double causes precision loss in some cases. #16

Closed
slilichenko opened this issue Oct 11, 2019 · 5 comments · Fixed by #279
Closed
Assignees
Labels
api: bigquery priority: p2 🚨 status: blocked type: bug

Comments

@slilichenko
Copy link

@slilichenko slilichenko commented Oct 11, 2019

Environment details

  1. Specify the API at the beginning of the title (for example, "BigQuery: ...")
    General, Core, and Other are also allowed as types
  2. OS type and version: any
  3. Java version: any
  4. google-cloud-java version(s): 1.96.0

Steps to reproduce

Create a row with this timestamp: 1337-09-02 11:43:21.622894 UTC, query and use fieldValue.getTimestamp(). Microseconds wouldn't be equal.

Code example

@test
public void testFloatingPointPrecisionLoss() {
FieldValue fieldValue = FieldValue.of(Attribute.PRIMITIVE, "-1.9954383398377106E10");
long received = fieldValue.getTimestampValue();
long expected = -19954383398377106L;
assertEquals(expected, received);
}

Any additional information below

Cause - floating point conversation causes it; need to use BigDecimal (or equivalent) to perform lossless conversion.

@slilichenko
Copy link
Author

@slilichenko slilichenko commented Oct 14, 2019

Sorry, but it's not a question, it's a bug...

@Obstinaty
Copy link

@Obstinaty Obstinaty commented Oct 14, 2019

退订

@shollyman shollyman self-assigned this Oct 17, 2019
@shollyman
Copy link
Contributor

@shollyman shollyman commented Nov 6, 2019

This is a bug that necessitates changes in the API backend. There's work in progress to address this.

@slilichenko
Copy link
Author

@slilichenko slilichenko commented Nov 6, 2019

I think it's much simpler than that: in FieldValue.getTimestamp() replace
new Double(Double.valueOf(getStringValue()) * MICROSECONDS).longValue()
with:
BigDecimal secondsWithMicro = new BigDecimal(getStringValue());
BigDecimal scaled = secondsWithMicro.scaleByPowerOfTen(6);
return scaled.longValue();
Obviously, can be made faster with a bit of parsing by hand rather than using BigDecimal.

@pmakani pmakani transferred this issue from googleapis/google-cloud-java Dec 10, 2019
@yoshi-automation yoshi-automation added triage me 🚨 labels Dec 10, 2019
@pmakani pmakani added priority: p2 type: bug and removed 🚨 triage me labels Dec 10, 2019
@stephaniewang526 stephaniewang526 self-assigned this Dec 27, 2019
@google-cloud-label-sync google-cloud-label-sync bot added the api: bigquery label Jan 29, 2020
@yoshi-automation yoshi-automation added the 🚨 label Apr 8, 2020
@meredithslota meredithslota added status: blocked and removed 🚨 labels Apr 11, 2020
@meredithslota
Copy link

@meredithslota meredithslota commented Apr 11, 2020

Added the "blocked" label until the backend changes @shollyman referenced are known.

@yoshi-automation yoshi-automation added the 🚨 label Apr 11, 2020
@pmakani pmakani removed the 🚨 label Apr 16, 2020
@yoshi-automation yoshi-automation added the 🚨 label Apr 16, 2020
gcf-merge-on-green bot pushed a commit that referenced this issue Apr 16, 2020
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
- [X] Make sure to open an issue as a [bug/issue](https://github.com/googleapis/java-bigquery/issues/new/choose) before writing your code!  That way we can discuss the change, evaluate designs, and agree on the general idea
- [X] Ensure the tests and linter pass
- [X] Code coverage does not decrease (if any source code was changed)
- [X] Appropriate docs were updated (if necessary)

Fixes #16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery priority: p2 🚨 status: blocked type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants