What is the proper way to set the timezone of datetime columns in finagle-mysql? #190

Open
eric opened this Issue Aug 3, 2013 · 12 comments

Comments

5 participants
@eric
Contributor

eric commented Aug 3, 2013

finagle-mysql writes DATETIME and TIMESTAMP columns in the current timezone and does not provide a way to specify a timezone (or force them to be in UTC).

Is this intentional? Are there plans to provide a way to force them to be sent in UTC?

@roanta

This comment has been minimized.

Show comment
Hide comment
@roanta

roanta Aug 5, 2013

Member

According to the mysql documentation[1], values for TIMESTAMP columns are converted from your mysql server time_zone to UTC for storage and from UTC to the time_zone for retrieval.

[1] http://dev.mysql.com/doc/refman/5.5/en/time-zone-support.html

Member

roanta commented Aug 5, 2013

According to the mysql documentation[1], values for TIMESTAMP columns are converted from your mysql server time_zone to UTC for storage and from UTC to the time_zone for retrieval.

[1] http://dev.mysql.com/doc/refman/5.5/en/time-zone-support.html

@eric

This comment has been minimized.

Show comment
Hide comment
@eric

eric Aug 5, 2013

Contributor

The problem I see is that I have my mysql server time_zone set to UTC, but it's being sent by finagle as local. 

It seems to me I would need a way to specify what timezone the server is in.

On Mon, Aug 5, 2013 at 10:15 AM, Ruben Oanta notifications@github.com
wrote:

According to the mysql documentation[1], values for TIMESTAMP columns are converted from your mysql server time_zone to UTC for storage and from UTC to the time_zone for retrieval.

[1] http://dev.mysql.com/doc/refman/5.5/en/time-zone-support.html

Reply to this email directly or view it on GitHub:
#190 (comment)

Contributor

eric commented Aug 5, 2013

The problem I see is that I have my mysql server time_zone set to UTC, but it's being sent by finagle as local. 

It seems to me I would need a way to specify what timezone the server is in.

On Mon, Aug 5, 2013 at 10:15 AM, Ruben Oanta notifications@github.com
wrote:

According to the mysql documentation[1], values for TIMESTAMP columns are converted from your mysql server time_zone to UTC for storage and from UTC to the time_zone for retrieval.

[1] http://dev.mysql.com/doc/refman/5.5/en/time-zone-support.html

Reply to this email directly or view it on GitHub:
#190 (comment)

@roanta

This comment has been minimized.

Show comment
Hide comment
@roanta

roanta Aug 5, 2013

Member

Okay, so it seems the issue is that the java.sql.Timestamp object doesn't encode a timezone and finagle-mysql is using a Calendar object, which does assume a timezone, to serialize the Timestamp.

Try setting your timezone using java.util.TimeZone#setDefault.

Member

roanta commented Aug 5, 2013

Okay, so it seems the issue is that the java.sql.Timestamp object doesn't encode a timezone and finagle-mysql is using a Calendar object, which does assume a timezone, to serialize the Timestamp.

Try setting your timezone using java.util.TimeZone#setDefault.

@eric

This comment has been minimized.

Show comment
Hide comment
@eric

eric Aug 5, 2013

Contributor

Correct.

I believe it would be helpful to be able to pass the TimeZone I want into Calendar.getInstance() so the timezone of the JVM process doesn't have to match the MySQL server.

Contributor

eric commented Aug 5, 2013

Correct.

I believe it would be helpful to be able to pass the TimeZone I want into Calendar.getInstance() so the timezone of the JVM process doesn't have to match the MySQL server.

@roanta

This comment has been minimized.

Show comment
Hide comment
@roanta

roanta Aug 5, 2013

Member

The serialization happens behind the scenes and isn't exposed, I think this is best. You're probably right though, we might want more granular control here. Maybe Timezones can be encoded with a Timestamp into a new composed object.

Member

roanta commented Aug 5, 2013

The serialization happens behind the scenes and isn't exposed, I think this is best. You're probably right though, we might want more granular control here. Maybe Timezones can be encoded with a Timestamp into a new composed object.

@eric

This comment has been minimized.

Show comment
Hide comment
@eric

eric Aug 5, 2013

Contributor

Yeah, I was definitely having trouble figuring out how I would get the info through all of the serialization stuff. I'll investigate the composite object thing.

I think over-all, the correct solution is to set a timezone for the connection — it isn't something that is going to vary from one Timestamp object to the next and having to create composite objects every time you pass it to finagle-mysql seems like a poor interface.

Contributor

eric commented Aug 5, 2013

Yeah, I was definitely having trouble figuring out how I would get the info through all of the serialization stuff. I'll investigate the composite object thing.

I think over-all, the correct solution is to set a timezone for the connection — it isn't something that is going to vary from one Timestamp object to the next and having to create composite objects every time you pass it to finagle-mysql seems like a poor interface.

@eric

This comment has been minimized.

Show comment
Hide comment
@eric

eric Aug 5, 2013

Contributor

I realized it would be really easy to add an (optional) TimeZone parameter to the constructor for Client and then that can be passed from Client.execute() to the ExecuteRequest constructor.

This would provide a simple way to have a connection-wide timezone specified and allow writeParameter() to have access to the TimeZone.

Does this sound reasonable to you? If so, I'll put together a pull request.

Contributor

eric commented Aug 5, 2013

I realized it would be really easy to add an (optional) TimeZone parameter to the constructor for Client and then that can be passed from Client.execute() to the ExecuteRequest constructor.

This would provide a simple way to have a connection-wide timezone specified and allow writeParameter() to have access to the TimeZone.

Does this sound reasonable to you? If so, I'll put together a pull request.

@eric

This comment has been minimized.

Show comment
Hide comment
@eric

eric Aug 5, 2013

Contributor

@roanta: Please let me know if #191 is a direction you would endorse.

Contributor

eric commented Aug 5, 2013

@roanta: Please let me know if #191 is a direction you would endorse.

@mosesn

This comment has been minimized.

Show comment
Hide comment
@mosesn

mosesn May 29, 2016

Contributor

I think this is a solid starter issue. We might be able to rehydrate #191, add a test and merge it in.

Contributor

mosesn commented May 29, 2016

I think this is a solid starter issue. We might be able to rehydrate #191, add a test and merge it in.

@vtatai

This comment has been minimized.

Show comment
Hide comment
@vtatai

vtatai May 10, 2017

@mosesn I will take a stab at it, hopefully will be able to get some progress.

vtatai commented May 10, 2017

@mosesn I will take a stab at it, hopefully will be able to get some progress.

@mkantor

This comment has been minimized.

Show comment
Hide comment
@mkantor

mkantor Nov 18, 2017

Unless I'm misunderstanding it looks like this was solved way back in 2014 with 4258d70.

mkantor commented Nov 18, 2017

Unless I'm misunderstanding it looks like this was solved way back in 2014 with 4258d70.

@eric

This comment has been minimized.

Show comment
Hide comment
@eric

eric Nov 19, 2017

Contributor

Looks like this would work great to solve the problem. I’m not using finagle anymore but I’m glad this has a solution. 🎉

Contributor

eric commented Nov 19, 2017

Looks like this would work great to solve the problem. I’m not using finagle anymore but I’m glad this has a solution. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment