PomeloFoundation / Pomelo.EntityFrameworkCore.MySql

Entity Framework Core provider for MySQL and MariaDB built on top of MySqlConnector
MIT License
2.69k stars 383 forks source link

TimeSpan.TotalHours could not be translated #1910

Open Lyra1337 opened 6 months ago

Lyra1337 commented 6 months ago

Steps to reproduce

  1. Create a Database:
    CREATE DATABASE test1;
    USE test1;
    CREATE TABLE `test` (
    `Id` INT NOT NULL,
    `Time` TIME NULL,
    PRIMARY KEY (`Id`)
    )
    COLLATE='utf8mb4_general_ci';
  2. Scaffold new context
  3. Try to sum all values of the Time column

context.Test.Sum(x => x.Time.TotalHours)

neither works:

context.Test.Sum(x => EF.Functions.DateDiffMinute(x.Time.TotalHours, TimeSpan.Zero) / 60d)

The issue

System.InvalidOperationException: The LINQ expression [...] could not be translated. Either rewrite the query in a form that can be translated, or switch to client evaluation explicitly by inserting a call to 'AsEnumerable', 'AsAsyncEnumerable', 'ToList', or 'ToListAsync'. See https://go.microsoft.com/fwlink/?linkid=2101038 for more information.

Further technical details

Operating system: Windows / Linux Pomelo.EntityFrameworkCore.MySql version: 8.0.0 Microsoft.AspNetCore.App version: 8.0.200


So, it's dear to me that TotalHours can't be mapped. Is there an other way to sum up all time valued-rows? Can I add the function TIME_TO_SEC to the EF Functions to call it an then divide by 3600 to get the hours?

Any help will be appreciated.

Thank you in advance.

lauxjpn commented 6 months ago

We currently still map the CLR type TimeSpan to the MySQL type time. But since time only supports a range from -838:59:59 to 838:59:59, it is not a good fit for representing a TimeSpan (the CLR type TimeOnly matches quite well though).

There is no good type in MySQL to represent a TimeSpan at the moment, so we are likely going to map TimeSpan to a bigint for the 9.0 release, that then represents ticks.


Currently, we only translate TimeSpan.Hours, TimeSpan.Minutes, TimeSpan.Seconds and TimeSpan.Milliseconds (the same as the other major EF Core providers do).

So you could use context.Test.Sum(x => x.Time.Hours) or context.Test.Sum(x => (x.Time.Hours * 60 * 60 * 1_000 + x.Time.Minutes * 60 * 1_000 + x.Time.Seconds * 1_000 + x.Time.Milliseconds) / 1_000.0 / 60.0 / 60.0).


If you want to add a custom implementation/mapping of TIME_TO_SEC to your app instead, I can provide you with some code if you want.

ajcvickers commented 6 months ago

@lauxjpn Interesting you are planning on changing the TimeSpan mapping to bigint. We have the same problem with the mapping in SQL Server, and it may be that bigint is better, but it would be a break for existing Code First models. We're wondering if we should do the same thing, but concerned about the break. Any thoughts?

/cc @roji

lauxjpn commented 6 months ago

@ajcvickers Pomelo has existing facilities in place to let users customize some type mappings (e.g. MySqlDefaultDataTypeMappings), so users can do something like .UseMySql([...], o => o.DefaultDataTypeMappings(m => m.WithClrTimeSpan(MySqlTimeSpanType.Time6))).

We would then add MySqlTimeSpanType.BigInt, internally resolve MySqlTimeSpanType.Default to it and implement the TimeSpan -> bigint support in addition to the already existing TimeSpan mapping variants, so users can still choose to keep the old behavior if they want to.

When it comes to scaffolding, we have existing pseudo connection string parameters that only work at design time to customize the scaffolding process (we implemented theses before there was official support to pass parameters to providers at design time). So we could add another one for letting users switch between scaffolding time to TimeOnly or TimeSpan (though I believe we already made time -> TimeOnly the default).

For migrations, users will suddenly be confronted with operations that change the column type out of the blue, so we should probably output a warning about it and how to keep the old mapping. We should/could also emit a data update operation to migrate the existing data over from time -> bigint (and then should probably support bigint -> time as well, though this could loose precision and should warn about it). I am not sure yet, if we want to just migrate the data by default, or if we want to add the data migration code with some explanation and throw to force users to manually confirm the change.

I think that's about the gist of it. What do you think?

Lyra1337 commented 6 months ago

@lauxjpn

If you want to add a custom implementation/mapping of TIME_TO_SEC to your app instead, I can provide you with some code if you want.

That would be great for the mean time.

I'd also love to contribute an implementation for this function or the translation of TimeSpan.Total* to the Pomelo code itself so others can also use this.


When it comes to scaffolding, we have existing pseudo connection string parameters that only work at design time to customize the scaffolding process (we implemented theses before there was official support to pass parameters to providers at design time). So we could add another one for letting users switch between scaffolding time to TimeOnly or TimeSpan (though I believe we already made time -> TimeOnly the default).

For migrations, users will suddenly be confronted with operations that change the column type out of the blue, so we should probably output a warning about it and how to keep the old mapping. We should/could also emit a data update operation to migrate the existing data over from time -> bigint (and then should probably support bigint -> time as well, though this could loose precision and should warn about it). I am not sure yet, if we want to just migrate the data by default, or if we want to add the data migration code with some explanation and throw to force users to manually confirm the change.

I think that's about the gist of it. What do you think?

I had this exact problem in the first place when migrating from .NET 6 to .NET 8. The scope was a legacy application with didn't use EF Core migrations. When upgrading to .NET 8 I switched to using migrations and the first step I took was to re-scaffold the entire database, so i have the correct current state of it when starting with EF migrations.

But then I had to migrate all my repositories to use TimeOnly and DateOnly and convert them back to TimeSpan / DateTime in the business layer. If there had been a switch to change the scaffolding process, it would had saved me many hours refactoring my app.

lauxjpn commented 6 months ago

I'd also love to contribute an implementation for this function [...] to the Pomelo code itself so others can also use this.

Feel free to push a PR for it. (Also, we are usually fine with backporting simple EF.Functions implementations. So even if it is being implemented for 9.0, we might backport it to 8.0.)


I'd also love to contribute an implementation for [...] **the translation of TimeSpan.Total*** to the Pomelo code itself so others can also use this.

That doesn't currently make too much sense because of:

There is no good type in MySQL to represent a TimeSpan at the moment, so we are likely going to map TimeSpan to a bigint for the 9.0 release, that then represents ticks.

I outlined the process of properly implementing this above in https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/1910#issuecomment-2068742564, that involves quite a bit and which we are likely to implement for the 9.0 release (but which will not be backported).


But then I had to migrate all my repositories to use TimeOnly and DateOnly and convert them back to TimeSpan / DateTime in the business layer. If there had been a switch to change the scaffolding process, it would had saved me many hours refactoring my app.

You should be able to scaffold the database (which would then generate TimeOnly values for time(n) columns) and just find/replace all occurrences of TimeOnly with TimeSpan in the generated model (same for DateOnly).

This should work fine for most scaffolding scenarios.

However, if you want to control or override the scaffolding process in any way possible, you can do so as well. I posted some sample code in https://github.com/PomeloFoundation/Pomelo.EntityFrameworkCore.MySql/issues/1584#issuecomment-990967123 to demonstrate this.

(The code is a bit older. Nowadays, the CustomMySqlDesignTimeServices class can just directly inherit from MySqlDesignTimeServices, because MySqlDesignTimeServices.ConfigureDesignTimeServices() is now virtual, so you can directly override it to add your custom services.)


If you want to add a custom implementation/mapping of TIME_TO_SEC to your app instead, I can provide you with some code if you want.

That would be great for the mean time.

Here is a sample console app, that demonstrates how to translate a custom EF.Functions extension method to a SQL function:

Program.cs ```csharp using System; using System.Collections.Generic; using System.Diagnostics; using System.Linq; using System.Linq.Expressions; using System.Reflection; using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Query; using Microsoft.EntityFrameworkCore.Query.SqlExpressions; using Microsoft.EntityFrameworkCore.Storage; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Pomelo.EntityFrameworkCore.MySql.Query.Internal; namespace IssueConsoleTemplate; public class IceCream { public int IceCreamId { get; set; } public string Name { get; set; } public TimeSpan MaxAllowedDurationWithoutRefrigeration { get; set; } public DateTime? TakenOutOfFreezer { get; set; } } public static class CustomMySqlDbFunctionsExtensions { public static int TimeToSec(this DbFunctions _, TimeSpan time) => throw new InvalidOperationException(CoreStrings.FunctionOnClient(nameof(TimeToSec))); } public class CustomMethodCallTranslator : IMethodCallTranslator { private static readonly MethodInfo TimeToSecMethodInfo = typeof(CustomMySqlDbFunctionsExtensions) .GetRuntimeMethod( nameof(CustomMySqlDbFunctionsExtensions.TimeToSec), [typeof(DbFunctions), typeof(TimeSpan)]); private readonly MySqlSqlExpressionFactory _sqlExpressionFactory; private readonly IRelationalTypeMappingSource _typeMappingSource; public CustomMethodCallTranslator( MySqlSqlExpressionFactory sqlExpressionFactory, IRelationalTypeMappingSource typeMappingSource) { _sqlExpressionFactory = sqlExpressionFactory; _typeMappingSource = typeMappingSource; } public SqlExpression Translate( SqlExpression instance, MethodInfo method, IReadOnlyList arguments, IDiagnosticsLogger logger) { return method == TimeToSecMethodInfo ? _sqlExpressionFactory.NullableFunction( "TIME_TO_SEC", new[] { arguments[1] }, typeof(int), _typeMappingSource.FindMapping(typeof(int))) : null; } } public class CustomMethodCallTranslatorPlugin : IMethodCallTranslatorPlugin { public CustomMethodCallTranslatorPlugin( IRelationalTypeMappingSource typeMappingSource, ISqlExpressionFactory sqlExpressionFactory) { var mySqlSqlExpressionFactory = (MySqlSqlExpressionFactory)sqlExpressionFactory; Translators = new IMethodCallTranslator[] { new CustomMethodCallTranslator(mySqlSqlExpressionFactory, typeMappingSource), }; } public IEnumerable Translators { get; } } public class CustomEvaluatableExpressionFilterPlugin : IEvaluatableExpressionFilterPlugin { public bool IsEvaluatableExpression(Expression expression) => expression is not MethodCallExpression methodCallExpression || methodCallExpression.Method.DeclaringType != typeof(CustomMySqlDbFunctionsExtensions); } public class CustomDbContextOptionsExtension : IDbContextOptionsExtension { private ExtensionInfo _info; public virtual DbContextOptionsExtensionInfo Info => _info ??= new ExtensionInfo(this); public void ApplyServices(IServiceCollection services) { new EntityFrameworkRelationalServicesBuilder(services) // <-- inject services .TryAdd() .TryAdd(); } public void Validate(IDbContextOptions options) { } private class ExtensionInfo : DbContextOptionsExtensionInfo { public ExtensionInfo(IDbContextOptionsExtension extension) : base(extension) { } public override int GetServiceProviderHashCode() => 0; public override bool ShouldUseSameServiceProvider(DbContextOptionsExtensionInfo other) => other is ExtensionInfo otherInfo; public override void PopulateDebugInfo(IDictionary debugInfo) { } public override bool IsDatabaseProvider => false; public override string LogFragment => string.Empty; } } public static class CustomDbContextOptionsBuilderExtensions { public static DbContextOptionsBuilder ApplyCustomServices(this DbContextOptionsBuilder optionsBuilder) { ((IDbContextOptionsBuilderInfrastructure)optionsBuilder) .AddOrUpdateExtension( optionsBuilder.Options.FindExtension() ?? new CustomDbContextOptionsExtension()); return optionsBuilder; } } public class Context : DbContext { public DbSet IceCreams { get; set; } protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder) { if (!optionsBuilder.IsConfigured) { var connectionString = "server=127.0.0.1;port=3306;user=root;password=;Database=Issue1910"; var serverVersion = ServerVersion.AutoDetect(connectionString); optionsBuilder .UseMySql(connectionString, serverVersion) .ApplyCustomServices() // <-- call custom extension method to inject services .LogTo(Console.WriteLine, LogLevel.Information) .EnableSensitiveDataLogging() .EnableDetailedErrors(); } } protected override void OnModelCreating(ModelBuilder modelBuilder) { modelBuilder.Entity( entity => { entity.HasData( new IceCream { IceCreamId = 1, Name = "Vanilla", MaxAllowedDurationWithoutRefrigeration = TimeSpan.FromMinutes(10), TakenOutOfFreezer = new DateTime(2024, 1, 1, 12, 0, 0), }, new IceCream { IceCreamId = 2, Name = "Chocolate", MaxAllowedDurationWithoutRefrigeration = TimeSpan.FromMinutes(15), TakenOutOfFreezer = new DateTime(2024, 1, 1, 12, 0, 0), }, new IceCream { IceCreamId = 3, Name = "Matcha", MaxAllowedDurationWithoutRefrigeration = TimeSpan.FromMinutes(10), TakenOutOfFreezer = null, }); }); } } internal static class Program { private static void Main() { using var context = new Context(); context.Database.EnsureDeleted(); context.Database.EnsureCreated(); var now = new DateTime(2024, 1, 1, 12, 13, 0); var timeOfDayInSeconds = (now.TimeOfDay.Hours * 60 + now.TimeOfDay.Minutes) * 60 + now.TimeOfDay.Seconds; var nonRefreezableIceCreams = context.IceCreams .Where(c => c.TakenOutOfFreezer != null) .Select(i => new { i.IceCreamId, i.Name, i.MaxAllowedDurationWithoutRefrigeration, OutOfFreezerInSeconds1 = EF.Functions.DateDiffSecond(i.TakenOutOfFreezer, now), OutOfFreezerInSeconds2 = timeOfDayInSeconds - ((i.TakenOutOfFreezer.Value.TimeOfDay.Hours * 60 + i.TakenOutOfFreezer.Value.TimeOfDay.Minutes) * 60 + i.TakenOutOfFreezer.Value.TimeOfDay.Seconds), OutOfFreezerInSeconds3 = EF.Functions.TimeToSec(now.TimeOfDay) - EF.Functions.TimeToSec(i.TakenOutOfFreezer.Value.TimeOfDay), }) .Where(t => t.OutOfFreezerInSeconds1 == t.OutOfFreezerInSeconds2 && t.OutOfFreezerInSeconds1 == t.OutOfFreezerInSeconds3 && t.OutOfFreezerInSeconds1 > (t.MaxAllowedDurationWithoutRefrigeration.Hours * 60 + t.MaxAllowedDurationWithoutRefrigeration.Minutes) * 60 + t.MaxAllowedDurationWithoutRefrigeration.Seconds) .ToList(); Trace.Assert(nonRefreezableIceCreams.Count == 1); Trace.Assert(nonRefreezableIceCreams[0].Name == "Vanilla"); } } ```
Output (SQL) ```sql warn: 25.04.2024 19:15:02.784 CoreEventId.SensitiveDataLoggingEnabledWarning[10400] (Microsoft.EntityFrameworkCore.Infrastructure) Sensitive data logging is enabled. Log entries and exception messages may include sensitive application data; this mode should only be enabled during development. info: 25.04.2024 19:15:03.156 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) Executed DbCommand (27ms) [Parameters=[], CommandType='Text', CommandTimeout='30'] DROP DATABASE `Issue1910`; info: 25.04.2024 19:15:03.386 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) Executed DbCommand (8ms) [Parameters=[], CommandType='Text', CommandTimeout='30'] CREATE DATABASE `Issue1910`; info: 25.04.2024 19:15:03.584 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) Executed DbCommand (13ms) [Parameters=[], CommandType='Text', CommandTimeout='30'] ALTER DATABASE CHARACTER SET utf8mb4; info: 25.04.2024 19:15:03.614 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) Executed DbCommand (29ms) [Parameters=[], CommandType='Text', CommandTimeout='30'] CREATE TABLE `IceCreams` ( `IceCreamId` int NOT NULL AUTO_INCREMENT, `Name` longtext CHARACTER SET utf8mb4 NULL, `MaxAllowedDurationWithoutRefrigeration` time(6) NOT NULL, `TakenOutOfFreezer` datetime(6) NULL, CONSTRAINT `PK_IceCreams` PRIMARY KEY (`IceCreamId`) ) CHARACTER SET=utf8mb4; info: 25.04.2024 19:15:03.622 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) Executed DbCommand (7ms) [Parameters=[], CommandType='Text', CommandTimeout='30'] INSERT INTO `IceCreams` (`IceCreamId`, `MaxAllowedDurationWithoutRefrigeration`, `Name`, `TakenOutOfFreezer`) VALUES (1, TIME '00:10:00', 'Vanilla', TIMESTAMP '2024-01-01 12:00:00'), (2, TIME '00:15:00', 'Chocolate', TIMESTAMP '2024-01-01 12:00:00'), (3, TIME '00:10:00', 'Matcha', NULL); info: 25.04.2024 19:15:03.955 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) Executed DbCommand (26ms) [Parameters=[@__now_1='2024-01-01T12:13:00.0000000' (Nullable = true) (DbType = DateTime), @__timeOfDayInSeconds_2='43980', @__now_TimeOfDay_3='12:13:00'], CommandType='Text', CommandTimeout='30'] SELECT `i`.`IceCreamId`, `i`.`Name`, `i`.`MaxAllowedDurationWithoutRefrigeration`, TIMESTAMPDIFF(SECOND, `i`.`TakenOutOfFreezer`, @__now_1) AS `OutOfFreezerInSeconds1`, @__timeOfDayInSeconds_2 - ((((EXTRACT(hour FROM CAST(` i`.`TakenOutOfFreezer` AS time(6))) * 60) + EXTRACT(minute FROM CAST(`i`.`TakenOutOfFreezer` AS time(6)))) * 60) + EXTRACT(second FROM CAST(`i`.`TakenOutOfFreezer` AS time(6)))) AS `OutOfFreezerInSeconds2`, TIME_TO_SEC(@__now_Tim eOfDay_3) - TIME_TO_SEC(CAST(`i`.`TakenOutOfFreezer` AS time(6))) AS `OutOfFreezerInSeconds3` FROM `IceCreams` AS `i` WHERE `i`.`TakenOutOfFreezer` IS NOT NULL AND (((TIMESTAMPDIFF(SECOND, `i`.`TakenOutOfFreezer`, @__now_1) = (@__timeOfDayInSeconds_2 - ((((EXTRACT(hour FROM CAST(`i`.`TakenOutOfFreezer` AS time(6))) * 60) + EXTRACT(minute F ROM CAST(`i`.`TakenOutOfFreezer` AS time(6)))) * 60) + EXTRACT(second FROM CAST(`i`.`TakenOutOfFreezer` AS time(6)))))) AND (TIMESTAMPDIFF(SECOND, `i`.`TakenOutOfFreezer`, @__now_1) = (TIME_TO_SEC(@__now_TimeOfDay_3) - TIME_TO_SE C(CAST(`i`.`TakenOutOfFreezer` AS time(6)))))) AND (TIMESTAMPDIFF(SECOND, `i`.`TakenOutOfFreezer`, @__now_1) > ((((EXTRACT(hour FROM `i`.`MaxAllowedDurationWithoutRefrigeration`) * 60) + EXTRACT(minute FROM `i`.`MaxAllowedDuratio nWithoutRefrigeration`)) * 60) + EXTRACT(second FROM `i`.`MaxAllowedDurationWithoutRefrigeration`)))) ```
ajcvickers commented 6 months ago

I think that's about the gist of it. What do you think?

Sounds like a plan! Let's see how it goes.

Lyra1337 commented 6 months ago

@lauxjpn Thank you so much for your work. I finally found time to wrap my head around your code. It works flawless in my setup.

Your code sparked the idea to look deeper into how Pomelo handles the SQL translation and I loved what I saw and couldn't stop adding a translation for the TimeSpan.Total* properties. (#1917)

I added another virtual mapping for TimeToHour to avoid additional arithmetics in the queries:

public class CustomTimeToHourMethodCallTranslator : IMethodCallTranslator
{
    private static readonly MethodInfo TimeToHourMethodInfo = typeof(CustomMySqlDbFunctionsExtensions)
        .GetRuntimeMethod(
            nameof(CustomMySqlDbFunctionsExtensions.TimeToHour),
            [typeof(DbFunctions), typeof(TimeSpan)]);

    private readonly MySqlSqlExpressionFactory sqlExpressionFactory;
    private readonly IRelationalTypeMappingSource typeMappingSource;

    public CustomTimeToHourMethodCallTranslator(
        MySqlSqlExpressionFactory sqlExpressionFactory,
        IRelationalTypeMappingSource typeMappingSource)
    {
        this.sqlExpressionFactory = sqlExpressionFactory;
        this.typeMappingSource = typeMappingSource;
    }

    public SqlExpression Translate(
        SqlExpression instance,
        MethodInfo method,
        IReadOnlyList<SqlExpression> arguments,
        IDiagnosticsLogger<DbLoggerCategory.Query> logger)
    {
        if (method == TimeToHourMethodInfo)
        {
            var timeToSecExpression = this.sqlExpressionFactory.NullableFunction(
                name: "TIME_TO_SEC",
                arguments: new[] { arguments[1] },
                returnType: typeof(int),
                typeMapping: this.typeMappingSource.FindMapping(typeof(int))
            );

            var divideBy3600Expression = this.sqlExpressionFactory.Divide(timeToSecExpression, this.sqlExpressionFactory.Constant(3600d, this.typeMappingSource.FindMapping(typeof(double))));

            return divideBy3600Expression;
        }
        else
        {
            return null;
        }
    }
}

public static class CustomMySqlDbFunctionsExtensions
{
    public static int TimeToSec(this DbFunctions _, TimeSpan time)
        => throw new InvalidOperationException(CoreStrings.FunctionOnClient(nameof(TimeToSec)));

    public static int TimeToHour(this DbFunctions _, TimeSpan time)
        => throw new InvalidOperationException(CoreStrings.FunctionOnClient(nameof(TimeToHour)));
}

public class CustomMethodCallTranslatorPlugin : IMethodCallTranslatorPlugin
{
    public IEnumerable<IMethodCallTranslator> Translators { get; }

    public CustomMethodCallTranslatorPlugin(
        IRelationalTypeMappingSource typeMappingSource,
        ISqlExpressionFactory sqlExpressionFactory)
    {
        var mySqlSqlExpressionFactory = (MySqlSqlExpressionFactory)sqlExpressionFactory;

        this.Translators = new IMethodCallTranslator[]
        {
            new CustomTimeToSecMethodCallTranslator(mySqlSqlExpressionFactory, typeMappingSource),
            new CustomTimeToHourMethodCallTranslator(mySqlSqlExpressionFactory, typeMappingSource),
        };
    }
}