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

row_event converts timestamps to UTC #182

Merged
merged 1 commit into from
Aug 23, 2016
Merged

row_event converts timestamps to UTC #182

merged 1 commit into from
Aug 23, 2016

Conversation

shlomi-noach
Copy link
Contributor

@shlomi-noach shlomi-noach commented Aug 22, 2016

Continued discussion on #161

This explicitly sets UTC for timestamps read by the go-mysql library.
Unfortunately when returning timestamp, it is returned as a string, just so that it is able to support 0000-00-00 00:00:00.
But then it uses the local machine's timezone, converting a real time into string, losing context. Because this is a string, it is not converted back to number when applied as DML event.
This is an attempt to fix that, with local tests already proving well.

cc @dveeden

  • contributed code is using same conventions as original code
  • code is formatted via gofmt (please avoid goimports)
  • code is built via ./build.sh
  • code is tested via ./test.sh

@dveeden
Copy link
Contributor

dveeden commented Aug 22, 2016

Ran a test and it looks ok. 👍

@siddontang
Copy link

@shlomi-noach

Do I need to fix it in go-mysql?

@shlomi-noach
Copy link
Contributor Author

@siddontang I'm not sure what the correct behavior is in general.

The thing is you have to convert the TIMESTAMP to string so as to support 0000-00-00 00:00:00, but then you lose the context of your time zone.

My take was to turn my app to UTC completely, and let MySQL figure out the timezone differences and do the math.

I think it would be at least nice to have this configurable: do I want to read timestamps in local timezone or in UTC.

@shlomi-noach shlomi-noach merged commit a1baa19 into master Aug 23, 2016
@shlomi-noach shlomi-noach deleted the utc-streamer branch August 23, 2016 06:13
@siddontang
Copy link

Got it, thanks @shlomi-noach

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.

None yet

3 participants