Pomelo.EntityFrameworkCore.MySql icon indicating copy to clipboard operation
Pomelo.EntityFrameworkCore.MySql copied to clipboard

Feature Request : Set timezone Per-session

Open omar-haddad opened this issue 5 years ago • 17 comments

I’m a very newbie here so I apologize if I had to post this in different place

MySql retrieves timestamps in its server time zone.

But it also offers the ability to set the preferred time zone by the client using the SQL query.

SET time_zone = '+00:00';

My Question: is there a way to prepend the SQL query above to each call.

omar-haddad avatar Nov 10 '19 23:11 omar-haddad

@omar-haddad

From https://docs.microsoft.com/en-us/ef/core/what-is-new/ef-core-3.0/#interception-of-database-operations

1- Create the interceptor class that sets the time zone:

public class UTCTimeInterceptor : DbCommandInterceptor
{
    public override InterceptionResult<DbDataReader> ReaderExecuting(
        DbCommand command, 
        CommandEventData eventData, 
        InterceptionResult<DbDataReader> result)
    {
        command.CommandText = string.Format("SET time_zone = '+00:00'; {0}", command.CommandText);
        return result;
    }
}

2- Register it in Setup.cs:

services.AddDbContext<DbContext>(options => options
            .UseMySql(Configuration.GetConnectionString("DbConnectionString"))
            .AddInterceptors(new UTCTimeInterceptor()));

idreeshaddad avatar Nov 11 '19 21:11 idreeshaddad

@idreeshaddad Thank you it works :-)

omar-haddad avatar Nov 11 '19 23:11 omar-haddad

As we have apparently at least two cases now, where people have requested support for a connection specific timezone, we should consider implementing official support for this feature for as @omar-haddad suggested, as this might be a common enough issue.

This could be implemented in a similar way we did implement the support in the past to automatically set the sql_mode when a connection gets established.

It would mean that we could introduce a method like the following:

services.AddDbContext<DbContext>(options => options
    .UseMySql(connectionString, builder => builder
        .TimeZone("Europe/Berlin"))); // supports any valid MySQL syntax for time zones

I will reopen this issue.

lauxjpn avatar Nov 12 '19 01:11 lauxjpn

For consistency, shouldn't we add that option to MySqlOptionsExtension like we do for NoBackslashEscapes?

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/fabed9436317254bc26f5cc2d964004c0d7f1291/src/EFCore.MySql/Storage/Internal/MySqlRelationalConnection.cs#L163-L167

mguinness avatar Nov 12 '19 02:11 mguinness

Absolutely, I meant the whole shebang by:

This could be implemented in a similar way we did implement the support in the past to automatically set the sql_mode when a connection gets established.

lauxjpn avatar Nov 17 '19 07:11 lauxjpn

Being able to set the connection timezone to UTC 0 is essentially required for correct operation of the CURRENT_TIMESTAMP function with datetime columns and DateTimeOffset entity types, since by default Pomelo converts DateTimeOffsets to and from UTC 0 in and out of the database, regardless of the connection string timezone. Any timezone other than UTC 0 will cause the CURRENT_TIMESTAMP value to be stored as a non-UTC time value, and then incorrectly converted when it is read out of the database.

Reference

If/when this functionality is implemented, I suggest that setting the timezone to UTC 0 is potentially a sensible default, although this is a pretty tricky issue to get right in all use cases.

crozone avatar May 25 '20 07:05 crozone

As an aside, you can fix this without a DbCommandInterceptor if you're stuck on 2.1 by using the Database.GetDbConnection().StateChange event:

public ApplicationDbContext(DbContextOptions<ApplicationDbContext> options)
            : base(options)
        {
            Database.GetDbConnection().StateChange += Connection_StateChange;
        }

        /// <summary>
        /// Fixes the connection string timezone, such that it is always UTC 0.
        /// This corrects database generated datetimes (like CURRENT_TIMESTAMP)
        /// </summary>
        void Connection_StateChange(object sender, System.Data.StateChangeEventArgs e)
        {
            if (e.CurrentState == System.Data.ConnectionState.Open)
            {
                if (sender is System.Data.Common.DbConnection)
                {
                    // Set the session time zone to UTC - we will handle any date/time conversions.
                    var command = (sender as System.Data.Common.DbConnection).CreateCommand();
                    command.CommandText = "SET time_zone = '+00:00'";
                    command.CommandType = System.Data.CommandType.Text;
                    command.ExecuteNonQuery();
                }
            }
        }

crozone avatar May 25 '20 07:05 crozone

Being able to set the connection timezone to UTC 0 is essentially required for correct operation of the CURRENT_TIMESTAMP function with datetime columns and DateTimeOffset entity types, since by default Pomelo converts DateTimeOffsets to and from UTC 0 in and out of the database, regardless of the connection string timezone. Any timezone other than UTC 0 will cause the CURRENT_TIMESTAMP value to be stored as a non-UTC time value, and then incorrectly converted when it is read out of the database.

Good point! CURRENT_TIMESTAMP is just a synonym for NOW(), which returns the time zone adjusted (local) date/time.

Also, trying to use a different timestamp mechanism like UTC_TIMESTAMP fails immediately.

If/when this functionality is implemented, I suggest that setting the timezone to UTC 0 is potentially a sensible default, although this is a pretty tricky issue to get right in all use cases.

Using UTC as the default when explicitly using a .SetTimeZone() extension method in .UseMySql() makes sense.

However, setting it as the default in general should depend on whether MySqlConnector is able to set the time zone within the same connection establishing round trip, in which case a connection string option for the time zone could directly be added to MySqlConnector. Our extension method would then just set the connection string option. (/cc @bgrainger)

If the time zone information cannot be send within the same round trip, we should not set it at all, unless an extension method like .SetTimeZone() is explicitly used, due to performance concerns when using high latency connections.

lauxjpn avatar May 27 '20 12:05 lauxjpn

However, setting it as the default in general should depend on whether MySqlConnector is able to set the time zone within the same connection establishing round trip, in which case a connection string option for the time zone could directly be added to MySqlConnector.

This should be possible. MySqlConnector currently sends SET NAMES utf8mb4; after resetting the connection when retrieving a connection from the pool:

https://github.com/mysql-net/MySqlConnector/blob/60ceaeda8c66a35447e3e29d87e237055ee25b56/src/MySqlConnector/Core/ServerSession.cs#L442-L444

There would need to be new support added to set the timezone when a new connection is established for the first time.

The user should also be advised that there will be a (small) performance penalty of the latency of an extra roundtrip to the server when opening a connection. It would be preferable to set the server default timezone, but this could be used as a workaround in situations where that isn't possible.

bgrainger avatar May 27 '20 13:05 bgrainger

a connection string option for the time zone could directly be added to MySqlConnector. Our extension method would then just set the connection string option.

One thing that's been discussed in the past (but I can't find the issue right now) is support for executing a callback each time a connection is opened. This would allow any arbitrary SQL to be executed, and wouldn't require a new connection string option to be created for this (or any other new feature).

The downside is that it could be less discoverable, requires code (not just a simple connection string change), and might never be able to be implemented as efficiently as custom support in MySqlConnector.

bgrainger avatar May 27 '20 14:05 bgrainger

One thing that's been discussed in the past (but I can't find the issue right now) is support for executing a callback each time a connection is opened. This would allow any arbitrary SQL to be executed, and wouldn't require a new connection string option to be created for this (or any other new feature).

For Pomelo/EF Core, @crozone demonstrated that in his code above.

We would implement it in Pomelo in a similar way, if you think the time zone should not be part of the connection string options.

We already do something similar here, where we set the SQL_MODE if NO_BACKSLASH_ESCAPES has been requested by the user:

https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/blob/fabed9436317254bc26f5cc2d964004c0d7f1291/src/EFCore.MySql/Storage/Internal/MySqlRelationalConnection.cs#L146-L169

lauxjpn avatar May 27 '20 14:05 lauxjpn

One thing that's been discussed in the past (but I can't find the issue right now) is support for executing a callback each time a connection is opened.

Was the issue https://github.com/mysql-net/MySqlConnector/issues/420? I don't think that's worth pursuing though as even Oracle are dropping it's use.

mguinness avatar May 27 '20 17:05 mguinness

No, but it was this comment (linked from that issue): https://github.com/mysql-net/MySqlConnector/issues/519#issuecomment-397829071

You can already execute arbitrary SQL on new/pooled connections by attaching an event handler to MySqlConnection.StateChange. ... which @crozone already mentioned above (and which I didn't read carefully enough).

bgrainger avatar May 27 '20 19:05 bgrainger

We would implement it in Pomelo in a similar way

At the moment, it currently seems like there are adequate ways to handle this in "user mode" code, without having to push it down into MySqlConnector. I think you should go ahead and handle it directly in Pomelo. (If it ever were implemented directly in MySqlConnector then Pomelo's SetTimeZone method could be updated to check/use the connection string option without impacting its consumers?)

bgrainger avatar May 27 '20 19:05 bgrainger

I think you should go ahead and handle it directly in Pomelo.

Yeah, I agree.

If it ever were implemented directly in MySqlConnector then Pomelo's SetTimeZone method could be updated to check/use the connection string option without impacting its consumers?

Absolutely.

lauxjpn avatar May 27 '20 19:05 lauxjpn

So is it still pending?

uzairali001 avatar Jul 21 '22 11:07 uzairali001