SaveChanges circular dependency in unique filtered index

[jgilchrist]

While testing out the newest preview, I'm running into an issue with a previously working bit of code - BatchingTopologicalSort is throwing errors for a case that was working in EFCore 6.

It's reproducible for me with the following cut-down code:

Entities:

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

public class TestDependentEntity
{
    public int Id { get; set; }
    public int TestEntityId { get; set; }

    public bool? UniqueOn { get; set; }
    public int? ToChange { get; set; }

    public class Config : IEntityTypeConfiguration<TestDependentEntity>
    {
        public void Configure(EntityTypeBuilder<TestDependentEntity> builder)
        {
            builder.HasIndex(e => new { e.TestEntityId, e.UniqueOn })
                .IsUnique();
        }
    }
}

To reproduce:

var dbContext = new TestDbContext();

var t1 = new TestEntity();

db.Tests.Add(t1);

await db.SaveChangesAsync();

var dependent = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = null,
    ToChange = 1,
};

var dependent2 = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = null,
    ToChange = 2,
};

db.TestDependents.Add(dependent);
db.TestDependents.Add(dependent2);

await db.SaveChangesAsync();

dependent.ToChange = null;
dependent2.ToChange = null;

await db.SaveChangesAsync();

The final SaveChangesAsync() will throw the following error:

System.InvalidOperationException: Unable to save changes because a circular dependency was detected in the data to be saved: 'TestDependentEntity { 'Id': 2 } [Modified] <-
Index { 'test_entity_id': 1, 'unique_on':  } TestDependentEntity { 'Id': 3 } [Modified] <-
Index { 'test_entity_id': 1, 'unique_on':  } TestDependentEntity { 'Id': 2 } [Modified]'.
   at Microsoft.EntityFrameworkCore.Utilities.Multigraph`2.ThrowCycle(List`1 cycle, Func`2 formatCycle, Func`2 formatException)
   at Microsoft.EntityFrameworkCore.Utilities.Multigraph`2.BatchingTopologicalSort(Func`4 tryBreakEdge, Func`2 formatCycle)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.TopologicalSort(IEnumerable`1 commands)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList`1 entries, IUpdateAdapter updateAdapter)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

It appears that this is due to the unique index, and indeed removing the index makes this work. Although I can't tell for sure, my hunch is that the code working with the index is not considering the fact that the two rows are considered unique by the index even though they both have null in UniqueOn because NULL != NULL?

Happy to provide any more info if needed.

Info

EF Core version: 7.0.0-preview.4.22229.2
Database provider: Npgsql.EntityFrameworkCore.PostgreSQL
Target framework: .NET 6
Operating system: macOS
IDE: Rider 2022.1

[jgilchrist]

Just adding on that I can also reproduce this with a filtered unique index which does not allow nulls:

// Same code for TestEntity

public class TestDependentEntity
{
    public int Id { get; set; }
    public int TestEntityId { get; set; }

    public string UniqueOn { get; set; } = null!;
    public int? ToChange { get; set; }

    public class Config : IEntityTypeConfiguration<TestDependentEntity>
    {
        public void Configure(EntityTypeBuilder<TestDependentEntity> builder)
        {
            builder.HasIndex(e => new { e.TestEntityId, e.UniqueOn })
                .HasFilter("unique_on != 'a'")
                .IsUnique();
        }
    }
}

Reproduce with:

var t1 = new TestEntity();

db.Tests.Add(t1);

await db.SaveChangesAsync();

var dependent = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = "a",
    ToChange = 1,
};

var dependent2 = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = "a",
    ToChange = 2,
};

var dependent3 = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = "b",
    ToChange = 3,
};

db.TestDependents.Add(dependent);
db.TestDependents.Add(dependent2);
db.TestDependents.Add(dependent3);

await db.SaveChangesAsync();

dependent.ToChange = null;
dependent2.ToChange = null;
dependent3.ToChange = null;

// Errors
await db.SaveChangesAsync();

Stacktrace:

System.InvalidOperationException: Unable to save changes because a circular dependency was detected in the data to be saved: 'TestDependentEntity { 'Id': 1 } [Modified] <-
Index { 'test_entity_id': 1, 'unique_on': a } TestDependentEntity { 'Id': 2 } [Modified] <-
Index { 'test_entity_id': 1, 'unique_on': a } TestDependentEntity { 'Id': 1 } [Modified]'.
   at Microsoft.EntityFrameworkCore.Utilities.Multigraph`2.ThrowCycle(List`1 cycle, Func`2 formatCycle, Func`2 formatException)
   at Microsoft.EntityFrameworkCore.Utilities.Multigraph`2.BatchingTopologicalSort(Func`4 tryBreakEdge, Func`2 formatCycle)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.TopologicalSort(IEnumerable`1 commands)
   at Microsoft.EntityFrameworkCore.Update.Internal.CommandBatchPreparer.BatchCommands(IList`1 entries, IUpdateAdapter updateAdapter)+MoveNext()
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

Similarly, it looks like the filter is not being taken into account so the two entries in the index { 'test_entity_id': 1, 'unique_on': a } end up causing the issue?

[roji]

Thanks for the code - confirmed that I can repro this. I've indeed done some changes in this area, will investigate.

Repro code

using System;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

var t1 = new TestEntity();

ctx.Tests.Add(t1);

await ctx.SaveChangesAsync();

var dependent = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = null,
    ToChange = 1,
};

var dependent2 = new TestDependentEntity() {
    TestEntityId = t1.Id,
    UniqueOn = null,
    ToChange = 2,
};

ctx.TestDependents.Add(dependent);
ctx.TestDependents.Add(dependent2);

await ctx.SaveChangesAsync();

dependent.ToChange = null;
dependent2.ToChange = null;

await ctx.SaveChangesAsync();

public class BlogContext : DbContext
{
    public DbSet<TestEntity> Tests { get; set; }
    public DbSet<TestDependentEntity> TestDependents { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlite(@"Data Source=/tmp/foo.sqlite")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<TestDependentEntity>().HasIndex(e => new { e.TestEntityId, e.UniqueOn })
            .IsUnique();
    }
}

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

public class TestDependentEntity
{
    public int Id { get; set; }
    public int TestEntityId { get; set; }

    public bool? UniqueOn { get; set; }
    public int? ToChange { get; set; }
}

[roji]

@AndriySvyryd this is caused by 246cc8692915c72d77e393952c1c6797dea92fc9g. I didn't dig too deep, CommandBatchPreparer.IsModified - called from AddUniqueValueEdges - returns true, as if the commands modify columns covered by the index (which they don't). The cause seems to be the new ProviderValueComparer check.

[AndriySvyryd]

This happens because the index has a filter. We shouldn't try to sort filtered index values as we can't evaluate it

[jgilchrist]

This happens because the index has a filter. We shouldn't try to sort filtered index values as we can't evaluate it

Just for the avoidance of doubt, is this also the cause for the first reproducer I posted which contains a unique index rather than an index with a filter?

[AndriySvyryd]

Just for the avoidance of doubt, is this also the cause for the first reproducer I posted which contains a unique index rather than an index with a filter?

No, the two repros have different causes

[jgilchrist]

Thanks for confirming, and thanks for taking a look 👍

[jmalczak]

Just for the avoidance of doubt, is this also the cause for the first reproducer I posted which contains a unique index rather than an index with a filter?

No, the two repros have different causes

Hi Guys

Is the first scenario without filter fixed as well? I have an issue where I attach entity and mark property as modified to force ef to send update for just this column. But instead I get the same exception regarding unique index. This approach was working fine in ef core 6. It fails now after update to ef core 7.

[Shaddix]

The first example still fails for me.

using System;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

var db = new TestDbContext();
await db.Database.EnsureDeletedAsync();
await db.Database.EnsureCreatedAsync();

var t1 = new TestEntity();

db.Tests.Add(t1);

await db.SaveChangesAsync();

var dependent = new TestDependentEntity()
{
    TestEntityId = t1.Id,
    UniqueOn = null,
    ToChange = 1,
};

var dependent2 = new TestDependentEntity()
{
    TestEntityId = t1.Id,
    UniqueOn = null,
    ToChange = 2,
};

db.TestDependents.Add(dependent);
db.TestDependents.Add(dependent2);

await db.SaveChangesAsync();

dependent.ToChange = null;
dependent2.ToChange = null;

await db.SaveChangesAsync();

public class TestDbContext : DbContext
{
    public DbSet<TestEntity> Tests { get; set; }
    public DbSet<TestDependentEntity> TestDependents { get; set; }
    
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlite(@"Data Source=foo.sqlite")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<TestDependentEntity>().HasIndex(e => new { e.TestEntityId, e.UniqueOn })
            .IsUnique();
    }
}

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

public class TestDependentEntity
{
    public int Id { get; set; }
    public int TestEntityId { get; set; }

    public bool? UniqueOn { get; set; }
    public int? ToChange { get; set; }

}

[Shaddix]

@roji can we reopen this, since it doesn't seem to be fixed?

[roji]

Confirmed that the unfiltered unique index case still fails on 7.0 (and works on 6.0).

@jmalczak @Shaddix I've opened #29647 to track this.