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

ConnectionString way to specify timezone? #487

Closed
JustArchi opened this issue Apr 24, 2018 · 4 comments
Closed

ConnectionString way to specify timezone? #487

JustArchi opened this issue Apr 24, 2018 · 4 comments

Comments

@JustArchi
Copy link

JustArchi commented Apr 24, 2018

From the information I've gathered it looks like currently there is no way to specify timezone at connection level. This means that you need to manually prepend SET time_zone = '+00:00'; to every SQL query, assuming one would want to operate in UTC. Setting timezone is a very important thing when you're dealing with TIMESTAMP fields, since mysql automatically converts them:

MySQL converts TIMESTAMP values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval.

Would it be a good idea to add appropriate option to connection string in order to allow specifying timezone that should be used for the connection by default? I believe this setting is general enough to be included on connection level and it'd be a good idea to specify default timezone that can still be overridden by appropriate SET time_zone SQL instruction, if one would like.

Thanks.

@bgrainger
Copy link
Member

The connection string options currently control data in the MySQL protocol that's otherwise inaccessible to ADO.NET, e.g., setting client capability flags in the initial handshake packet. I'm not convinced there's sufficient value in adding connection string settings to send SQL to change MySQL session variables; this is very easy to work around by adding a helper method:

async Task<DbConnection> OpenConnectionAsync()
{
    var connection = new MySqlConnection(connectionStringFromConfig);
    await connection.OpenAsync().ConfigureAwait(false);
    await connection.ExecuteAsync(@"SET time_zone = '+00:00';").ConfigureAwait(false); // using Dapper
    return connection;
}

There would be only minor efficiency gains from moving the execution of this SQL into MySqlConnector; the bulk of the elapsed time would be sending the command to the server and waiting for the response. (To optimise that, you could prepend the SET command to the first SQL statement that is executed on the open connection, which wouldn't be possible if MySqlConnector were handling it.)

Finally, in this specific scenario I would strongly recommend setting the server time zone to UTC to avoid all timezone conversions, rather than trying to work around the conversion in the client.

@JustArchi
Copy link
Author

JustArchi commented Apr 24, 2018

this is very easy to work around by adding a helper method

I'm using a very similar way right now that prepends above SET statement to first executed query over given connection. This way I don't need to execute two independent instructions but rather one two-instruction SQL query, saving round trip.

There would be only minor efficiency gains from moving the execution of this SQL into MySqlConnector

I was thinking about automatic timezone setting the moment connection returns to the pool, after ConnectionReset happens. This way we could have already ready connection with given timezone set, ready to handle the command without a need of executing (very cheap, but still) SET operation. Whether it would be beneficial depends on expected usage (and very likely not in majority of cases), but from-idle time could be one instruction faster.

Finally, in this specific scenario I would strongly recommend setting the server time zone to UTC to avoid all timezone conversions, rather than trying to work around the conversion in the client.

This assumes that we don't only have the control over given MySQL server but also that we're permitted to change it. That is not always possible or wanted, especially in combination with other apps and services that access the very same resources. In order to do that effectively you'd basically need to change entire server's clock to UTC, and that once again might prove challenging as there are fixed scenarios where apps might require real server timezone for various operations, for example web client providing given web server his real timezone for data display purposes.

Although I fully agree with your reasoning and I do not want to push it at all cost if you believe that driver wouldn't benefit from having such option set on connection level. I was just wondering if there isn't any better way to achieve it, apart from changing MySQL server settings.

@bgrainger
Copy link
Member

I'm using a very similar way right now that prepends above SET statement to first executed query over given connection. This way I don't need to execute two independent instructions but rather one two-instruction SQL query, saving round trip.

You would lose this optimisation (and probably see a performance regression) if we moved the SET statement into MySqlConnector, unless...

I was thinking about automatic timezone setting the moment connection returns to the pool

We've discussed this before in #178. It's currently blocked on some problems encountered during implementation (particularly when under heavy load).

Finally, in this specific scenario I would strongly recommend setting the server time zone to UTC to avoid all timezone conversions, rather than trying to work around the conversion in the client.

This assumes that we don't only have the control over given MySQL server but also that we're permitted to change it.

Yes, I should have made it clearer that this isn't always (often?) a viable option.

If we can develop an efficient way of resetting in the background when sessions are returned to the pool (#178) then this could be a reasonable extension to add at that time. Otherwise, I think it's best for now for users who need it to work around the problem as you're already doing (by manually executing the statement as part of the first query).

@JustArchi
Copy link
Author

Alright then, thank you a lot for your support, I really appreciate entire work you put into this awesome library! ❤️

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

No branches or pull requests

2 participants