-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Using FK with HasColumnType causes strange migration changes #30827
Comments
Note for triage: still repros on latest daily, and on SQL Server. |
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 |
Oh wait, Changing it in the snapshot fixes the incorrect operations. |
Hmm, this is happening because the model has a value converter (between 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? |
The snapshot should always store the provider type to avoid dealing with value converters. |
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. |
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. |
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);
}
}
|
Yes, it appears that you've already implemented this #29026 Still, we would need to add support for mismatched keys in model building and somehow limit it to the snapshot. |
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:
Used packages:
Run
dotnet ef migrations add
Every new migration after initial will contains these changes:
Possibly related: #27619 #27549
Is there any workaround?
The text was updated successfully, but these errors were encountered: