fix(bigquery): Fix timestamp scientific notation handling in _helpers.py#17278
fix(bigquery): Fix timestamp scientific notation handling in _helpers.py#17278pkenjora wants to merge 1 commit into
Conversation
Casting to float first handles scientific notation returned by API. Otherwise its a noop.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Code Review
This pull request modifies the timestamp conversion helper in _helpers.py to cast values to float before converting them to integers. The reviewer noted that casting all values to float can cause precision loss for large integer timestamps (beyond the year 2285) and suggested attempting to parse as an integer first, falling back to float only if a ValueError occurs.
| if _not_null(value, field): | ||
| # value will be a integer in seconds, to microsecond precision, in UTC. | ||
| return _datetime_from_microseconds(int(value)) | ||
| return _datetime_from_microseconds(int(float(value))) |
There was a problem hiding this comment.
Directly casting all values to float before converting to int can lead to precision loss for very large integer values (specifically, integers greater than 2^53 - 1, which corresponds to timestamps beyond the year 2285). Since BigQuery supports timestamps up to the year 9999, we should attempt to parse the value as an integer first to preserve full precision, and only fall back to float parsing if a ValueError occurs (e.g., when scientific notation or a decimal point is present).
try:
microseconds = int(value)
except ValueError:
microseconds = int(float(value))
return _datetime_from_microseconds(microseconds)|
Hello @pkenjora ! Could you also provide test coverage for this change? Thanks a lot! |
Casting to float first handles scientific notation returned by API. Otherwise its a noop.
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:
Fixes #17277🦕