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

Allow setting connection timezone and honor UTC For Date/DateTime #732

Closed
wants to merge 1 commit into from

Conversation

aikar
Copy link
Contributor

@aikar aikar commented Feb 16, 2018

This resolves #262

if timezone is set, we change the timezone of the connection upon opening.

If timezone is set to utc and not using dateStrings, we create Date objects
using UTC mode instead.

Also discovered missing config keys that affect packet parsers not
being included in the cache key for packet parser cache, so added them.

@sidorares
Copy link
Owner

looks good! Can you add tests for time changes? Not sure what's better way to test, real mysql server or mock server, but we need to test at least one happy path

@aikar
Copy link
Contributor Author

aikar commented Feb 16, 2018

I'll see if i can over weekend, hopefully won't forget.... was a late night in the office.

This resolves sidorares#262

if timezone is set, we change the timezone of the connection upon opening.

If timezone is set to utc and not using dateStrings, we create Date objects
using UTC mode instead.

Also discovered missing config keys that affect packet parsers not
being included in the cache key for packet parser cache, so added them.
@jasonlewicki
Copy link

Would love to see this pushed along.

I am trying to SELECT TIMESTAMP fields from a MySQL server set to UTC and they are converted into the future by my local machine's TZ offset.

@aikar
Copy link
Contributor Author

aikar commented Feb 27, 2018

@sidorares Would you be able to add some test for it so we can get this merged? I've been way too busy to find the time to setup testing for this.

@sidorares
Copy link
Owner

@aikar yes, I can work on tests myself. Hope to clean up high priority issues over weekend

@aikar
Copy link
Contributor Author

aikar commented Feb 28, 2018

@jasonlewicki make sure you set timezone: 'utc' in your pool config. This PR will not auto detect timezone settings.

We should follow up with another PR to add (configurable) automatic timezone detection.

@kibertoad
Copy link
Contributor

@sidorares Need any help with this PR?

@sidorares
Copy link
Owner

@kibertoad @aikar this is fairly big change, we need at least add some integration tests for this

@sidorares
Copy link
Owner

also a bit on the fence about "local" timezone and automatic "SET time_zone = ?" query. If we end up adding this it need at least good documentation, otherwise it will end up being dead code no one uses

@sidorares
Copy link
Owner

sidorares commented Aug 12, 2019

hey @aikar sorry I was not helpful enough to get this work into master
Closing in favour of #996

@sidorares sidorares closed this Aug 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Timezone is wrong on client when reading records from SQL server running in UTC time
4 participants