-
-
Notifications
You must be signed in to change notification settings - Fork 34
[Timestamp] Use microsecond precision to support full range of BigQuery timestamps #133
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
Conversation
Codecov Report
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff
8000
@@
## main #133 +/- ##
==========================================
+ Coverage 45.82% 45.88% +0.06%
==========================================
Files 47 47
Lines 17800 17820 +20
==========================================
+ Hits 8156 8176 +20
+ Misses 8182 8180 -2
- Partials 1462 1464 +2 |
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.
Thank you for your great PR ! I've commented
return time.Time{}, fmt.Errorf("invalid timestamp string (multiple delimiters) %s", v) | ||
} | ||
seconds, err := strconv.ParseInt(parts[0], 10, 64) | ||
micros := int64(0) |
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.
Since err
variable may be overwritten by L26, so error handling should be done here.
seconds, err := ...
if err != nil {
return time.Time{}, err
}
micros := int64(0)
timestamp.go
Outdated
for len(microsString) < 6 { | ||
microsString += "0" | ||
} | ||
micros, err = strconv.ParseInt(parts[1], 10, 64) |
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.
m, err = strconv.ParseInt(parts[1], 10, 64)
if err != nil {
return time.Time{}, err
}
micros = m
Thank you for your contribution ! LGTM 👍 |
We were previously encoding / decoding timestamp values using nanoseconds, which only covers a partial range of the supported timestamps as we hit the lowest / highest allowed int64 values more quickly.
Since BigQuery only supports microsecond precision, this switches us to it.
Closes #132