dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.75k stars 3.18k forks source link

Migration fails in EF7 when mapping TVF using a model with 'string' properties #30142

Open srofi opened 1 year ago

srofi commented 1 year ago

After updating to EF7, when trying to add a migration I get an error with the following text:

An operation was scaffolded that may result in the loss of data. Please review the migration for accuracy.
System.InvalidOperationException: Cannot scaffold C# literals of type 'System.Reflection.NullabilityInfoContext'. The provider should implement CoreTypeMapping.GenerateCodeLiteral to support using it at design time.

After a lot of testing I found that the problem was mapping table valued functions from the DB, but only if the mapped model contains 'string' properties. Otherwise, it works normally.

Steps to reproduce

Here is a small code to reproduce the issue in a console app:

using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

namespace Test;

public class Program
{
    static void Main(string[] args)
    {
        using var host = Host.CreateDefaultBuilder(args)
            .ConfigureServices(services => services.AddDbContext<ApplicationDbContext>(options => options.UseSqlServer("ConnectionString")))
            .Build();

        host.Run();
    }
}

public class ApplicationDbContext : DbContext
{
    public ApplicationDbContext(DbContextOptions options) : base(options) { }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        base.OnModelCreating(modelBuilder);

        modelBuilder.HasDbFunction(typeof(ApplicationDbContext).GetMethod(nameof(TableValueFunctionTest), new[] { typeof(DateTime), typeof(decimal) }));
    }

    public IQueryable<Data> TableValueFunctionTest(DateTime param1, decimal param2) => FromExpression(() => TableValueFunctionTest(param1, param2));
}

[Keyless]
public class Data
{
    public string Text { get; set; }

    public int Number { get; set; }

    public long BigNumber { get; set; }
}

The .csproj file contains this:

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>
    <ItemGroup>
        <PackageReference Include="Microsoft.Extensions.Hosting" Version="7.0.0" />
        <PackageReference Include="Microsoft.EntityFrameworkCore" Version="7.0.2" />
        <PackageReference Include="Microsoft.EntityFrameworkCore.Design" Version="7.0.2">
            <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
            <PrivateAssets>all</PrivateAssets>
        </PackageReference>
        <PackageReference Include="Microsoft.EntityFrameworkCore.Relational" Version="7.0.2" />
        <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="7.0.2" />
        <PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="7.0.2">
            <PrivateAssets>all</PrivateAssets>
            <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
        </PackageReference>
    </ItemGroup>
</Project>

In the previous code, commenting out the property "Text", or changing its type (I tested with int, decimal, double, and DateTime) makes the migration work. Also downgrading to EF 6.0.10 works.

By the way, I tried adding the migration using the dotnet-ef tool (updated to 7.0.2) with the command:

dotnet ef migrations add Test

Also tried with the old

Add-Migration Test

Further technical details

EF Core version: 7.0.2 Database provider: Microsoft.EntityFrameworkCore.SqlServer Target framework: .NET 7.0 Operating system: Windows 11 IDE: Visual Studio 2022 17.4

srofi commented 1 year ago

I found that modifying the code, adding this into the OnModelCreating function, solves the issue:

modelBuilder.Entity<Data>().ToFunction(nameof(TableValueFunctionTest));

The problem is that the 'Data' type, in my case, is being used into multiple functions, but that can be solved by calling it for the first function, or creating multiple classes that inherit from 'Data' for every case. Nevertheless, this is not explained in the documentation for user defined function mapping (https://learn.microsoft.com/en-us/ef/core/querying/user-defined-function-mapping), nor is detailed as a breaking change in EF7.

ajcvickers commented 1 year ago

Note for triage: I am able to reproduce this. Regression from 7. Still repros on latest daily. Full stack trace:

System.InvalidOperationException: Cannot scaffold C# literals of type 'System.Reflection.NullabilityInfoContext'. The provider should implement CoreTypeMapping.GenerateCodeLiteral to support using it at design time.
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpHelper.UnknownLiteral(Object value)
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpHelper.<Fragment>g__AppendMethodCall|52_0(IMethodCallCodeFragment current, <>c__DisplayClass52_0&)
   at Microsoft.EntityFrameworkCore.Design.Internal.CSharpHelper.Fragment(IMethodCallCodeFragment fragment, Int32 indent)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.GenerateAnnotations(String builderName, IAnnotatable annotatable, IndentedStringBuilder stringBuilder, Dictionary`2 annotations, Boolean inChainedCall, Boolean leadingNewline)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpSnapshotGenerator.Generate(String modelBuilderName, IModel model, IndentedStringBuilder stringBuilder)
   at Microsoft.EntityFrameworkCore.Migrations.Design.CSharpMigrationsGenerator.GenerateMetadata(String migrationNamespace, Type contextType, String migrationName, String migrationId, IModel targetModel)
   at Microsoft.EntityFrameworkCore.Migrations.Design.MigrationsScaffolder.ScaffoldMigration(String migrationName, String rootNamespace, String subNamespace, String language)
   at Microsoft.EntityFrameworkCore.Design.Internal.MigrationsOperations.AddMigration(String name, String outputDir, String contextType, String namespace)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigrationImpl(String name, String outputDir, String contextType, String namespace)
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.AddMigration.<>c__DisplayClass0_0.<.ctor>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.<>c__DisplayClass3_0`1.<Execute>b__0()
   at Microsoft.EntityFrameworkCore.Design.OperationExecutor.OperationBase.Execute(Action action)
ajcvickers commented 1 year ago

The workaround here is to call modelBuilder.Entity<Data>().ToTable(null); before mapping the functions.

ebeaulieu-openmindt commented 1 year ago

@ajcvickers Didn't work on my side. :(

I'm getting a compilation error with the following message: The call is ambiguous between the following methods or properties: 'RelationalEntityTypeBuildExtensions.ToTable(EntityTypeBuilder, string?)' and 'RelationalEntityTypeBuildExtensions.ToTable(EntityTypeBuilder, Action<TableBuilder>)'

AndriySvyryd commented 1 year ago

modelBuilder.Entity<Data>().ToTable((string?)null);

SLAVONchick commented 1 year ago

I found that modifying the code, adding this into the OnModelCreating function, solves the issue:

modelBuilder.Entity<Data>().ToFunction(nameof(TableValueFunctionTest));

The problem is that the 'Data' type, in my case, is being used into multiple functions, but that can be solved by calling it for the first function, or creating multiple classes that inherit from 'Data' for every case. Nevertheless, this is not explained in the documentation for user defined function mapping (https://learn.microsoft.com/en-us/ef/core/querying/user-defined-function-mapping), nor is detailed as a breaking change in EF7.

Thanks a lot! This seems to work. (Even for functions defined in separate static class mapped to built-in DBMS function with [DbFunction] attribute)

LeonarddeR commented 1 year ago

This issue is still present as of EF Core 8 RC2.

DanielStout5 commented 12 months ago

The solution of adding modelBuilder.Entity<Data>().ToTable((string?)null); causes an exception when attempting to remove migrations:

System.ArgumentNullException: Value cannot be null. (Parameter 'name')
   at Microsoft.EntityFrameworkCore.Utilities.Check.NotNull[T](T value, String parameterName)
   at Microsoft.EntityFrameworkCore.RelationalEntityTypeBuilderExtensions.ToTable(EntityTypeBuilder entityTypeBuilder, String name, Action`1 buildAction)

This comes from this line in the migration designer file:

                    b.ToTable((string)null, t =>
                        {
                            t.ExcludeFromMigrations();
                        });

This is the code for ToTable in Microsoft.EntityFrameworkCore.Relational:

    public static EntityTypeBuilder ToTable(
        this EntityTypeBuilder entityTypeBuilder,
        string name,
        Action<TableBuilder> buildAction)
    {
        Check.NotNull(name, nameof(name));
        Check.NotNull(buildAction, nameof(buildAction));

        entityTypeBuilder.Metadata.SetTableName(name);
        entityTypeBuilder.Metadata.SetSchema(null);
        buildAction(new TableBuilder(StoreObjectIdentifier.Table(name), entityTypeBuilder));

        return entityTypeBuilder;
    }

Check.NotNull(name, nameof(name)); is the problem, because the suggested workaround passes null for the table name

Edit: Turns out this was related to code in this specific project that called SetIsTableExcludedFromMigrations(true). The override of ToTable that includes a buildAction requires a non-null table name.. Is there another workaround for this issue besides ToTable(null)?

AndriySvyryd commented 11 months ago

Edit: Turns out this was related to code in this specific project that called SetIsTableExcludedFromMigrations(true). The override of ToTable that includes a buildAction requires a non-null table name.. Is there another workaround for this issue besides ToTable(null)?

A null table cannot be excluded from migrations. Why do you need to exclude the table?

DanielStout5 commented 11 months ago

This DbContext is in a class library that is added to multiple projects which may extend the DbContext to add their own tables. The base project tables need to be excluded from migrations in the subclassed DbContexts because otherwise the migrations in the sub-projects will attempt to add changes that will be migrated by the base DbContext.

I did try checking if the entity was keyless and if so not excluding it from the migrations, but there was some issue with that approach (I don't remember what exactly). The solution that worked was builder.Entity<Data>().ToTable(nameof(Data)); instead of null. Doesn't seem to have had any side effects.

TWhidden commented 10 months ago

I had success with ToTable((string?)null) - the ToFunction(nameof(MyFunc)) resulted in an internal ef null reference exception.

I was hoping ef8 would have a fix for this. Thank you for the work around @AndriySvyryd .

AndriySvyryd commented 2 days ago

Related: https://github.com/dotnet/efcore/issues/34996