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

Proposal about drive Location and DB Timezone in session #1114

Open
felipecaputo opened this issue May 26, 2020 · 4 comments
Open

Proposal about drive Location and DB Timezone in session #1114

felipecaputo opened this issue May 26, 2020 · 4 comments

Comments

@felipecaputo
Copy link

Problem

We have found that the time difference was happening when validating a time difference
between the moment an action was taken from the user and now, and the data from
the action was stored in the database.

When we were running locally, everything was working perfectly but, after deploy
to staging there was an unexpected error. The system was telling that the difference
was 3 hours more.

After we analyzed the problem we have found that:

  • Driver uses, when not set, location from UTC
  • Driver does not read the Timezone from the global or session from DB
  • MySQL timestamp store data according to Timezone, based on session
  • Combined loc argument and timezone argument from the connection string produced some different behaviors.

Behavior

You can view the source and run this example in this repo

ATTENTION: Database is en Europe/Sofia and images are considering America/Sao_Paulo
Considering: both loc and db variable
                Local time: 2020-05-11 18:29:54 -0300 -03,
                DB time: 2020-05-11 18:29:54 -0300 -03,
                Difference Local to DB: 0s

Considering: only loc
                Local time: 2020-05-11 18:29:54 -0300 -03,
                DB time: 2020-05-12 01:29:54 -0300 -03,
                Difference Local to DB: -7h0m0s

Considering: only DB variable
                Local time: 2020-05-11 21:29:54 +0000 UTC,
                DB time: 2020-05-11 18:29:54 +0000 UTC,
                Difference Local to DB: 3h0m0s

Considering: using none configuration
                Local time: 2020-05-11 21:29:54 +0000 UTC,
                DB time: 2020-05-12 01:29:54 +0000 UTC,
                Difference Local to DB: -4h0m0s

How to reproduce

I've built a simple app that exposes a post endpoint that allows you to post data to DB and read automatically.

There are 4 instances in the docker-compose.yml file and a MySQL database in this repo.

Source code from the program is in main.go

To run, you just need to run run.sh (and probably, need to give execution permissions to it before running chmod +x run.sh)

Proposal

Considering other database drivers, seems plausible that the driver could handle the timezone automatically, based on
some configuration, considering it, and avoiding breaking anyone using the connection as it is now, I think that, creating
a configTZ as a boolean configuration parameter, that default value will be false, to implement the timezone configuration behavior bellow, when set as true.

loc nil America/SaoPaulo nil America/Sao_Paulo
timezone nil nil America/Sao_Paulo Europa/Sofia
Behavior Set loc to UTC and
DB session to UTC
OR
Read from the
database and
set loc to the
same timezone
After connect
to database,
configure session
to America/Sao_Paulo
timezone, and use it
when parsing time
After connect
to database,
configure session
to America/Sao_Paulo
timezone, and use it
when parsing time
Configure as
specified by the
user, but generates
a warning, telling
that TIMEZONE fields
could face a difference
after parse.
@methane
Copy link
Member

methane commented May 28, 2020

I believe using UTC is the only safe way. Only it can be recommended.
For example, there are no guarantee that Go application and MySQL server refers same tzdata.

So I am not so fever in the proposed configTZ parameter. Experts can configure loc and time_zone already. And it can not help most of non-experts who don't understand loc and tzdata because it is false by default.

To make this driver more safe for many users, I propose to send SET time_zone="UTC" by default when loc is UTC or not specified. It is backward compatible. It is safe and default. So it will help most users.

@felipecaputo
Copy link
Author

Hi @methane, thank you for your fast response. I think it makes sense too, and it's much simpler and objective approach.

In the case of loc being set, makes sense to set the same TZ to the connection?

Can you assign me to this issue, I'ld love to help.

@methane
Copy link
Member

methane commented May 28, 2020

In the case of loc being set, makes sense to set the same TZ to the connection?

I don't think it is backward compatible when loc is not UTC. I believe UTC is +0000 without DST in all systems in the world. But other time zones may be vary.

Think about one MySQL server which is running on time_zone A but with old tzdata.
The time zone A in Go may be not equal to the time zone A in that MySQL server.
When a programmer find time zone B in Go behaves same to time zone A in MySQL, they may use it in the loc. Now it works well.
If go-sql-drvier/mysql sends the time_zone suddenly, the system will be broken.
That's why it is backward incompatible.

We already mention about time_zone system variable in document of the loc. (https://github.com/go-sql-driver/mysql#loc).
Helping people who use loc option without reading document is not important than people using just default option.

@felipecaputo
Copy link
Author

felipecaputo commented May 28, 2020

I agree with you, even our cause, we were using the connection without any parameter.

Can I proceed with this, implementing as your proposal:

  • Set timezone to UTC in database, only if loc is empty or UTC

I was thinking and it seems that, one good solution was to add this login in func parseDSNParams(cfg *Config, params string) (err error) just after the switch and validate if time_zone param was not specified and loc is nil or UTC, and if all this is true, add the param time_zone=UTC

But I couldn't think a way of writing a test case that breaks, since it would mean to change the global timezone from test database, disconnect and run a test, and it could cause side effects in other tests

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

No branches or pull requests

2 participants