-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 timestamp and interval arithmetic bug #11017
Conversation
This pull request was exported from Phabricator. Differential Revision: D62811693 |
✅ Deploy Preview for meta-velox canceled.
|
This pull request was exported from Phabricator. Differential Revision: D62811693 |
Summary: Pull Request resolved: facebookincubator#11017 For timestamps plus or minus intervals, the arithmetic needs to be done at the local timezone (not UTC) to account for intervals than span a daylight savings time boundary. This is already handled for timestamp with timezone types, and not needed for IntervalDayTime. Reviewed By: spershin Differential Revision: D62811693
c929b96
to
87e80da
Compare
This pull request was exported from Phabricator. Differential Revision: D62811693 |
Summary: Pull Request resolved: facebookincubator#11017 For timestamps plus or minus intervals, the arithmetic needs to be done at the local timezone (not UTC) to account for intervals than span a daylight savings time boundary. This is already handled for timestamp with timezone types, and not needed for IntervalDayTime. Reviewed By: spershin Differential Revision: D62811693
87e80da
to
4184841
Compare
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.
Thanks!
Looks good, few "[Ubuntu debug]" tests are failing (hard to see which ones, as usual).
Double-check that's not new tests.
This pull request was exported from Phabricator. Differential Revision: D62811693 |
Summary: Pull Request resolved: facebookincubator#11017 For timestamps plus or minus intervals, the arithmetic needs to be done at the local timezone (not UTC) to account for intervals than span a daylight savings time boundary. This is already handled for timestamp with timezone types, and not needed for IntervalDayTime. Reviewed By: spershin Differential Revision: D62811693
4184841
to
62bf3c2
Compare
Summary: Pull Request resolved: facebookincubator#11017 For timestamps plus or minus intervals, the arithmetic needs to be done at the local timezone (not UTC) to account for intervals than span a daylight savings time boundary. This is already handled for timestamp with timezone types, and not needed for IntervalDayTime. Reviewed By: spershin Differential Revision: D62811693
This pull request was exported from Phabricator. Differential Revision: D62811693 |
62bf3c2
to
a1d2b81
Compare
This pull request has been merged in d8b60db. |
Conbench analyzed the 1 benchmark run on commit There were no benchmark performance regressions. 🎉 The full Conbench report has more details. |
Summary:
For timestamps plus or minus intervals, the arithmetic needs to be
done at the local timezone (not UTC) to account for intervals than span a
daylight savings time boundary. This is already handled for timestamp with
timezone types, and not needed for IntervalDayTime.
Reviewed By: spershin
Differential Revision: D62811693