ModelBuilder: Do not silently remove key when a property is made optional

[patricknolan]

If I run the following SQL in SSMS it successfully creates the Section table and associated IX_Section_ApplicationName_SectionName_Aspect_Descriminator index.

CREATE TABLE [Configuration].[Section](
	[Id] [int] IDENTITY(1,1) NOT NULL,
	[ApplicationName] [nvarchar](50) NOT NULL,
	[Aspect] [nvarchar](50) NULL,
	[Descriminator] [nvarchar](450) NULL,
	[SectionName] [nvarchar](50) NOT NULL,
	[Modified] [datetime2](7) NULL,
	[ModifiedUser] [nvarchar](50) NULL,
	[Timestamp] [timestamp] NOT NULL,
    CONSTRAINT [PK_Section] PRIMARY KEY CLUSTERED 
    (
	    [Id] ASC
    ),
    CONSTRAINT [IX_Section_ApplicationName_SectionName_Aspect_Descriminator] UNIQUE NONCLUSTERED 
	(
		[ApplicationName] ASC,
		[Aspect] ASC,
		[SectionName] ASC,
		[Descriminator] ASC
	)
)

The following fluent config should produce the same result, however the IX_Section_ApplicationName_Aspect_SectionName_Descriminator index does not get generated. If I set IsRequired to true for the Descriminator field it does get created. Should this not produce the same result as the SQL outlined above?

        public override void Map(EntityTypeBuilder<SectionEntity> b)
        {
            b.ToTable("Section", "Configuration").HasKey(e => e.Id);

            b.HasAlternateKey(e => new {e.ApplicationName, e.Aspect, e.SectionName, e.Descriminator})
                .HasName("IX_Section_ApplicationName_Aspect_SectionName_Descriminator");

            b.Property(e => e.Descriminator)
                .IsRequired(false)
                .HasMaxLength(450);

            b.Property(e => e.ApplicationName)
                .IsRequired()
                .HasMaxLength(50);

            b.Property(e => e.Aspect).HasMaxLength(50);

            b.Property(e => e.Modified).HasDefaultValueSql("getdate()");

            b.Property(e => e.ModifiedUser).HasMaxLength(50);

            b.Property(e => e.SectionName)
                .IsRequired()
                .HasMaxLength(50);

            b.Property(e => e.Timestamp)
                .IsRequired()
                .HasColumnType("timestamp")
                .ValueGeneratedOnAddOrUpdate();
        }

[divega]

Seems to be an ordering problem. If I move the call to HasAlternateKey() to the bottom the constraint is created.

[rowanmiller]

It looks like you probably want a unique index rather than an alternate key...

From https://docs.efproject.net/en/latest/modeling/alternate-keys.html

If you just want to enforce uniqueness of a column then you want a unique index rather than an alternate key, see Indexes. In EF, alternate keys provide greater functionality than unique indexes because they can be used as the target of a foreign key.

It looks like we remove the key because Descriminator is later configured to be optional (not OK for a key, but OK for a unique index).

            b.HasAlternateKey(e => new {e.ApplicationName, e.Aspect, e.SectionName, e.Descriminator})
                .HasName("IX_Section_ApplicationName_Aspect_SectionName_Descriminator");

            b.Property(e => e.Descriminator)
                .IsRequired(false)
                .HasMaxLength(450);

This is a bug though. We shouldn't just silently remove the key, we should throw.

[divega]

Self-contained repro in case it saves some time:

using Microsoft.EntityFrameworkCore;

namespace ConsoleApplication92
{
    class Program
    {
        static void Main(string[] args)
        {
            using (var db = new MyContext())
            {
                db.Database.EnsureDeleted();
                db.Database.EnsureCreated();
            }
        }
    }

    public class MyContext : DbContext
    {

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            modelBuilder.Entity<SectionEntity>(b =>
            {

                b.ToTable("Section", "Configuration").HasKey(e => e.Id);
                b.HasAlternateKey(e => new { e.ApplicationName, e.Aspect, e.SectionName, e.Descriminator })
                    .HasName("IX_Section_ApplicationName_Aspect_SectionName_Descriminator");
                b.Property(e => e.Descriminator)
                    .IsRequired(false)
                    .HasMaxLength(450);
                b.Property(e => e.ApplicationName)
                    .IsRequired()
                    .HasMaxLength(50);
                b.Property(e => e.Aspect).HasMaxLength(50);
                b.Property(e => e.Modified).HasDefaultValueSql("getdate()");
                b.Property(e => e.ModifiedUser).HasMaxLength(50);
                b.Property(e => e.SectionName)
                    .IsRequired()
                    .HasMaxLength(50);
                b.Property(e => e.Timestamp)
                    .IsRequired()
                    .HasColumnType("timestamp")
                    .ValueGeneratedOnAddOrUpdate();
            });
        }
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer(@"Server=(localdb)\mssqllocaldb;Database=ConsoleApplication92.MyContext;Integrated Security=true");
        }

        public class SectionEntity
        {
            public string ApplicationName { get; internal set; }
            public string Aspect { get; internal set; }
            public string Descriminator { get; internal set; }
            public int Id { get; internal set; }
            public string Modified { get; internal set; }
            public string ModifiedUser { get; internal set; }
            public string SectionName { get; internal set; }
            public byte[] Timestamp { get; internal set; }
        }
    }
}

[patricknolan]

Hi @rowanmiller an unique index wouldn't work for me as I need to be able to cater for a null Descriminator.

If I have a unique index I can add two records with the same ApplicationName, Aspect and SectionName where the Descriminator is null.

With an AlternativeKey which should produce the same result as my sample SQL, if I try and add two records with the same ApplicationName, Aspect and SectionName where the Descriminator is null it fails the uniqueness constraint. This is the behaviour I need.

[patricknolan]

Hi @divega if I move the position of the HasAlternateKey() to the bottom the constraint is created but EF automatically sets the Descriminator to Required even though I have declared it as IsRequired(false). In my comment above I explain why I need an alternative unique key and have the Descriminator configured as nullable.

I believe I should be able to reproduce the same result as the sample SQL Statement. If I can do this in SQL it should not be prevented via the Fluent config.

[rowanmiller]

I think what you actually want is the ability to include null values in the index, which is tracked by #6794.

Alternate keys are effectively the same as primary keys and EF does not allow null values to be stored in them.

[AndriySvyryd]

This issue will be used to track that making a property optional removes any keys defined on it silently.