-
Notifications
You must be signed in to change notification settings - Fork 304
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: preserve timestamp microsecond precision with rows from REST API #402
Conversation
@@ -82,7 +82,18 @@ def _timestamp_from_json(value, field): | |||
"""Coerce 'value' to a datetime, if set or not nullable.""" | |||
if _not_null(value, field): | |||
# value will be a float in seconds, to microsecond precision, in UTC. | |||
return _datetime_from_microseconds(1e6 * float(value)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to be modified to parse from an rfc timestamp string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, i didn't get the point. Before passing params["formatOptions.useInt64Timestamp"] = True
in client.list_row
, received a value in string with decimal in _timestamp_from_json
method , but after passing it, received a value in string without decimal and call the method _datetime_from_microseconds(int(value))
which returns datetime object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. Yes, that's expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case, we should update the comment.
Also, are there cases where floating point values are still passed in? If not, we should clean this up now. If so, let's add a TODO to identify those cases and file an issue to clean them up.
google/cloud/bigquery/_helpers.py
Outdated
elif value.isdigit(): | ||
value = int(value) | ||
else: | ||
value = 1e6 * float(value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this case needed? Couldn't the above isdigit()
check be removed and we always fall back to converting to integer? I don't expect floating point values to be serialized as strings in JSON.
…into bigquery_issue_395
google/cloud/bigquery/_helpers.py
Outdated
return _datetime_from_microseconds(1e6 * float(value)) | ||
# value will be a integer in seconds, to microsecond precision, in UTC. | ||
|
||
# if value is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove commented out code.
…into bigquery_issue_395
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 #395