Allow FKs to have a different type from PKs in the snapshot

[renagaev]

I have database schema where FK column type differs from PK column type it points to. Every new migration contains stange changes even when there is no model changes.

Steps to reproduce:

Create project with this code:

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Design;
using Npgsql;

namespace EfIssue;

public class Bank
{
    public long Id { get; set; }
    public ICollection<Account> Accounts { get; set; }
}

public class Account
{
    public long Id { get; set; }
    public Bank Bank { get; set; }
    public long BankId { get; set; }
}

public class AppDbContext : DbContext
{
    public DbSet<Bank> Banks { get; set; }
    public DbSet<Account> Accounts { get; set; }

    public AppDbContext(DbContextOptions<AppDbContext> options) : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        builder.Entity<Bank>();
        builder.Entity<Account>()
            .HasOne(x => x.Bank)
            .WithMany(x => x.Accounts)
            .HasForeignKey(x => x.BankId);

        builder.Entity<Account>().Property(x => x.BankId).HasColumnType("smallint");
    }
}

internal class DbDesignFactory : IDesignTimeDbContextFactory<AppDbContext>
{
    public AppDbContext CreateDbContext(string[] args)
    {
        var dataSourceBuilder = new NpgsqlDataSourceBuilder(
            "Host=localhost;Database=eftest;Username=postgres;Password=postgres;");
        var optionsBuilder = new DbContextOptionsBuilder<AppDbContext>();
        optionsBuilder.UseNpgsql(dataSourceBuilder.Build());
        return new AppDbContext(optionsBuilder.Options);
    }
}

Used packages:

<PackageReference Include="Microsoft.EntityFrameworkCore" Version="7.0.5" />
<PackageReference Include="Microsoft.EntityFrameworkCore.Tools" Version="7.0.5">
  <PrivateAssets>all</PrivateAssets>
  <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="Npgsql.EntityFrameworkCore.PostgreSQL" Version="7.0.4" />

Run dotnet ef migrations add

Every new migration after initial will contains these changes:

migrationBuilder.DropForeignKey(
    name: "FK_Accounts_Banks_BankId",
    table: "Accounts");

migrationBuilder.DropUniqueConstraint(
    name: "AK_Banks_TempId",
    table: "Banks");

migrationBuilder.DropColumn(
    name: "TempId",
    table: "Banks");

migrationBuilder.AddForeignKey(
    name: "FK_Accounts_Banks_BankId",
    table: "Accounts",
    column: "BankId",
    principalTable: "Banks",
    principalColumn: "Id",
    onDelete: ReferentialAction.Cascade);

Possibly related: #27619 #27549
Is there any workaround?

[ajcvickers]

Note for triage: still repros on latest daily, and on SQL Server.

[bricelam]

The model snapshot looks correct. I suspect this is an issue/difference in model building with shadow/indexer properties.

modelBuilder.Entity("Account", b =>
{
    b.Property<short>("BankId")
        .HasColumnType("smallint");
});

modelBuilder.Entity("Bank", b =>
{
    b.Property<long>("Id")
        .HasColumnType("bigint");
});

modelBuilder.Entity("Account", b =>
{
    b.HasOne("Bank", "Bank")
        .WithMany("Accounts")
        .HasForeignKey("BankId");
});

  Model: 
    EntityType: Account
      Properties: 
        Id (long) Required PK AfterSave:Throw ValueGenerated.OnAdd
-       BankId (long) Required FK Index
+       BankId (short) Required FK Index
      Navigations: 
        Bank (Bank) ToPrincipal Bank Inverse: Accounts
      Keys: 
        Id PK
      Foreign keys: 
-       Account {'BankId'} -> Bank {'Id'} Required Cascade ToDependent: Accounts ToPrincipal: Bank
+       Account {'BankId'} -> Bank {'TempId'} Required Cascade ToDependent: Accounts ToPrincipal: Bank
      Indexes: 
        BankId
    EntityType: Bank
      Properties: 
        Id (long) Required PK AfterSave:Throw ValueGenerated.OnAdd
+       TempId (short) Required AlternateKey AfterSave:Throw
      Navigations: 
        Accounts (ICollection<Account>) Collection ToDependent Account Inverse: Bank
      Keys: 
        Id PK

[bricelam]

Oh wait, Property<short>("BankId") looks wrong. That should be long not short.

Changing it in the snapshot fixes the incorrect operations.

[bricelam]

Hmm, this is happening because the model has a value converter (between int and short) on the property, and we erase type converters in the model snapshot. This is to avoid referencing app-specific types (like enums) that might evolve over the lifetime of the application.

What are your thoughts on fixing this, @AndriySvyryd and @ajcvickers? Can we somehow detect that it's a simple cast conversion between two provider types and thus safe to keep the value conversion in the snapshot?

[AndriySvyryd]

The snapshot should always store the provider type to avoid dealing with value converters.

[bricelam]

Ok, then this is a model building error. The snapshot model should use the existing Id column (even though it has a different CLR type) rather than introducing TempId.

[AndriySvyryd]

We do not support FKs that point to principal properties of a different type even if they could be compatible. We should also validate provider types in the same way as the snapshot model doesn't support this mismatch.

[ajcvickers]

@AndriySvyryd

We do not support FKs that point to principal properties of a different type even if they could be compatible.

The model above appears to work fine outside of Migrations.

using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Design;
using Microsoft.Extensions.Logging;

var options = new DbContextOptionsBuilder<AppDbContext>()
    .UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow")
    .LogTo(Console.WriteLine, LogLevel.Information)
    .EnableSensitiveDataLogging()
    .Options;

using (var context = new AppDbContext(options))
{
    await context.Set<Account>().ExecuteDeleteAsync();
    await context.Set<Bank>().ExecuteDeleteAsync();

    context.AddRange(new Bank { Accounts = new List<Account> { new(), new(), new(), new() } },
        new Bank { Accounts = new List<Account> { new(), new(), new(), new() } });
    await context.SaveChangesAsync();
}

using (var context = new AppDbContext(options))
{
    var banks = context.Set<Bank>().Include(e => e.Accounts).ToList();

    for (var i = 0; i < 4; i++)
    {
        (banks[0].Accounts.ElementAt(i).BankId, banks[1].Accounts.ElementAt(i).BankId)
            = (banks[1].Accounts.ElementAt(i).BankId, banks[0].Accounts.ElementAt(i).BankId);
    }

    await context.SaveChangesAsync();
}

public class Bank
{
    public long Id { get; set; }
    public ICollection<Account> Accounts { get; set; }
}

public class Account
{
    public long Id { get; set; }
    public Bank Bank { get; set; }
    public long BankId { get; set; }
}

public class AppDbContext : DbContext
{
    public DbSet<Bank> Banks { get; set; }
    public DbSet<Account> Accounts { get; set; }

    public AppDbContext(DbContextOptions<AppDbContext> options) : base(options)
    {
    }

    protected override void OnModelCreating(ModelBuilder builder)
    {
        builder.Entity<Bank>();
        builder.Entity<Account>()
            .HasOne(x => x.Bank)
            .WithMany(x => x.Accounts)
            .HasForeignKey(x => x.BankId);

        builder.Entity<Account>().Property(x => x.BankId).HasColumnType("smallint");
    }
}

internal class DbDesignFactory : IDesignTimeDbContextFactory<AppDbContext>
{
    public AppDbContext CreateDbContext(string[] args)
    {
        var optionsBuilder = new DbContextOptionsBuilder<AppDbContext>();
             optionsBuilder.UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow")
             .LogTo(Console.WriteLine, LogLevel.Information)
             .EnableSensitiveDataLogging();
        return new AppDbContext(optionsBuilder.Options);
    }
}

warn: 9/28/2023 11:19:42.189 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: 9/28/2023 11:19:42.775 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed DbCommand (28ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      DELETE FROM [a]
      FROM [Accounts] AS [a]
info: 9/28/2023 11:19:42.791 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      DELETE FROM [b]
      FROM [Banks] AS [b]
info: 9/28/2023 11:19:42.995 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed DbCommand (17ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      INSERT INTO [Banks]
      OUTPUT INSERTED.[Id]
      DEFAULT VALUES;
      INSERT INTO [Banks]
      OUTPUT INSERTED.[Id]
      DEFAULT VALUES;
info: 9/28/2023 11:19:43.056 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed DbCommand (15ms) [Parameters=[@p0='5', @p1='5', @p2='5', @p3='5', @p4='6', @p5='6', @p6='6', @p7='6'], CommandType='Text', CommandTimeout='30']
      SET IMPLICIT_TRANSACTIONS OFF;
      SET NOCOUNT ON;
      MERGE [Accounts] USING (
      VALUES (@p0, 0),
      (@p1, 1),
      (@p2, 2),
      (@p3, 3),
      (@p4, 4),
      (@p5, 5),
      (@p6, 6),
      (@p7, 7)) AS i ([BankId], _Position) ON 1=0
      WHEN NOT MATCHED THEN
      INSERT ([BankId])
      VALUES (i.[BankId])
      OUTPUT INSERTED.[Id], i._Position;
info: 9/28/2023 11:19:43.282 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      SELECT [b].[Id], [a].[Id], [a].[BankId]
      FROM [Banks] AS [b]
      LEFT JOIN [Accounts] AS [a] ON [b].[Id] = [a].[BankId]
      ORDER BY [b].[Id]
info: 9/28/2023 11:19:43.308 RelationalEventId.CommandExecuted[20101] (Microsoft.EntityFrameworkCore.Database.Command) 
      Executed DbCommand (5ms) [Parameters=[@p1='9', @p0='6', @p3='10', @p2='6', @p5='11', @p4='6', @p7='12', @p6='6', @p9='13', @p8='5', @p11='14', @p10='5', @p13='15', @p12='5', @p15='16', @p14='5'], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      UPDATE [Accounts] SET [BankId] = @p0
      OUTPUT 1
      WHERE [Id] = @p1;
      UPDATE [Accounts] SET [BankId] = @p2
      OUTPUT 1
      WHERE [Id] = @p3;
      UPDATE [Accounts] SET [BankId] = @p4
      OUTPUT 1
      WHERE [Id] = @p5;
      UPDATE [Accounts] SET [BankId] = @p6
      OUTPUT 1
      WHERE [Id] = @p7;
      UPDATE [Accounts] SET [BankId] = @p8
      OUTPUT 1
      WHERE [Id] = @p9;
      UPDATE [Accounts] SET [BankId] = @p10
      OUTPUT 1
      WHERE [Id] = @p11;
      UPDATE [Accounts] SET [BankId] = @p12
      OUTPUT 1
      WHERE [Id] = @p13;
      UPDATE [Accounts] SET [BankId] = @p14
      OUTPUT 1
      WHERE [Id] = @p15;

[AndriySvyryd]

Yes, it appears that you've already implemented this https://github.com/dotnet/efcore/pull/29026

Still, we would need to add support for mismatched keys in model building and somehow limit it to the snapshot.