Skip to content
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

Wrong model creation for alternate keys using annotations and removed index #33531

Closed
Sielnix opened this issue Apr 13, 2024 · 0 comments · Fixed by #33611
Closed

Wrong model creation for alternate keys using annotations and removed index #33531

Sielnix opened this issue Apr 13, 2024 · 0 comments · Fixed by #33611
Labels
area-conventions closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug

Comments

@Sielnix
Copy link

Sielnix commented Apr 13, 2024

Hello,
Models created for database are wrong if we use alternate keys and have removed ForeignKeyIndexConvention. Code snippet:

    public abstract class Context101 : DbContext
    {
        protected override void ConfigureConventions(ModelConfigurationBuilder configurationBuilder)
        {
            base.ConfigureConventions(configurationBuilder);
            configurationBuilder.Conventions.Remove<ForeignKeyIndexConvention>();
        }

        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
            => optionsBuilder.UseSqlServer("");
    }

    public class Context0 : Context101
    {
        public class Blog
        {
            public int Id { get; set; }
            public int AlternateId { get; set; }
            public ICollection<Post> Posts { get; } = new List<Post>();
        }

        public class Post
        {
            public int Id { get; set; }
            public int BlogId { get; set; }
            public Blog Blog { get; set; }
        }

        public DbSet<Blog> Blogs
            => Set<Blog>();

        public DbSet<Post> Posts
            => Set<Post>();

        protected override void OnModelCreating(ModelBuilder modelBuilder)
            => modelBuilder.Entity<Blog>()
                .HasMany(e => e.Posts)
                .WithOne(e => e.Blog)
                .HasPrincipalKey(e => e.AlternateId);
    }

    public class ContextAnnotated0 : Context101
    {
        public class Blog
        {
            public int Id { get; set; }
            public int AlternateId { get; set; }

            [InverseProperty("Blog")]
            public ICollection<Post> Posts { get; } = new List<Post>();
        }

        public class Post
        {
            public int Id { get; set; }

            [ForeignKey("Blog")]
            public int BlogId { get; set; }

            [ForeignKey("BlogId")]
            [Required]
            [InverseProperty("Posts")]
            public Blog Blog { get; set; }
        }

        public DbSet<Blog> Blogs
            => Set<Blog>();

        public DbSet<Post> Posts
            => Set<Post>();

        protected override void OnModelCreating(ModelBuilder modelBuilder)
            => modelBuilder.Entity<Blog>()
                .HasMany(e => e.Posts)
                .WithOne(e => e.Blog)
                .HasPrincipalKey(e => e.AlternateId);
    }

The only difference between Context0 and ContextAnnotated0 is that ContextAnnotated0 has additional attributes on properties, which might and should be redundant in this case.

If we run following code:

            await using Context0 context = new();
            await context.Database.EnsureDeletedAsync();
            await context.Database.EnsureCreatedAsync();

then we will have two tables with two columns each - Blog(Id, AlternateId) and Post(Id, BlogId).

However, if we run it for annotated context:

            await using ContextAnnotated0 context = new();
            await context.Database.EnsureDeletedAsync();
            await context.Database.EnsureCreatedAsync();

Then we will have additional column BlogAlternateId in Posts table.

This code is mostly copied from ModelBuilding101TestBase.OneToManyRequiredWithAlternateKeyTest. This test passes for SqlServer, because SqlServer by default have ForeignKeyIndexConvention. I am developing EFCore.Snowflake, where this convention is removed (Snowflake doesn't have indexes) and constructed model is incorrect.

Include provider and version information

EF Core version: v8.0.4
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 8.0
Operating system: Windows 11 23H2
IDE: Visual Studio 2022 17.9.6

AndriySvyryd added a commit that referenced this issue Apr 24, 2024
@AndriySvyryd AndriySvyryd added this to the 9.0.0 milestone Apr 24, 2024
@AndriySvyryd AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 25, 2024
@AndriySvyryd AndriySvyryd removed their assignment Apr 25, 2024
AndriySvyryd added a commit that referenced this issue Apr 25, 2024
AndriySvyryd added a commit that referenced this issue Apr 30, 2024
@ajcvickers ajcvickers modified the milestones: 9.0.0, 9.0.0-preview5 May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-conventions closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported type-bug
Projects
None yet
3 participants