Efcore: Topological sort should consider unique constraints for ordering

Created on 7 May 2020  路  7Comments  路  Source: dotnet/efcore

Given these tables:

  • IceCream
  • IceCreamVariation
  • IceCreamVariationQuality

And these relationships:

  • IceCream has a one to many to IceCreamVariation
  • IceCreamVariation has a one to many to IceCreamVariationQuality

If I try to replace the list of variations of an IceCream I see the following behavior:

  1. IceCreamVariationQualitys are removed first
  2. new IceCreamVariations are added
  3. old IceCreamVariations are removed

Now #1 is fine. But the ordering of #2 and #3 causes problems with regards for example to unique constraints. I would have expected #3 to happen before #2.

It's true both orderings could cause issues because it depends on the kind of constraints the table has, but I would guess going #3 then #2 would generally be better.

Steps to reproduce

The example below uses Pomelo (MySQL) driver but I've been told the same can be reproduced in SQL Server:

``` C#
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

namespace IssueConsoleTemplate
{
public class IceCream
{
public int IceCreamId { get; set; }
public string Name { get; set; }

    public ICollection<IceCreamVariation> Variations { get; set; } = new HashSet<IceCreamVariation>();
}

public class IceCreamVariation
{
    public int IceCreamVariationId { get; set; }
    public string Name { get; set; }
    public int IceCreamId { get; set; }

    public IceCream IceCream { get; set; }

    public ICollection<IceCreamVariationQuality> Qualities { get; set; } = new HashSet<IceCreamVariationQuality>();
}

public class IceCreamVariationQuality
{
    public int IceCreamVariationQualityId { get; set; }
    public string Name { get; set; }
    public int IceCreamVariationId { get; set; }

    public IceCreamVariation IceCreamVariation { get; set; }
}

public class Context : DbContext
{
    public DbSet<IceCream> IceCreams { get; set; }
    public DbSet<IceCreamVariation> IceCreamVariations { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseMySql(
                "server=127.0.0.1;port=3306;user=root;password=root;database=So61383388",
                b => b.ServerVersion("8.0"))
            .UseLoggerFactory(
                LoggerFactory.Create(
                    b => b
                        .AddConsole()
                        .AddFilter(level => level >= LogLevel.Information)))
            .EnableSensitiveDataLogging()
            .EnableDetailedErrors();
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<IceCream>()
            .HasData(
                new IceCream {IceCreamId = 1, Name = "Vanilla"}
            );

        modelBuilder.Entity<IceCreamVariation>()
            .HasData(
                new IceCreamVariation {IceCreamVariationId = 1, Name = "Double Vanilla Bourbon", IceCreamId = 1},
                new IceCreamVariation {IceCreamVariationId = 2, Name = "Vanilla Caramel", IceCreamId = 1}
            );

        modelBuilder.Entity<IceCreamVariationQuality>()
            .HasData(
                new IceCreamVariationQuality {IceCreamVariationQualityId = 1, Name = "Fresh", IceCreamVariationId = 1},
                new IceCreamVariationQuality {IceCreamVariationQualityId = 2, Name = "Yummy", IceCreamVariationId = 1},
                new IceCreamVariationQuality {IceCreamVariationQualityId = 3, Name = "Woops", IceCreamVariationId = 2}
            );
    }
}

internal class Program
{
    private static void Main()
    {
        using (var context = new Context())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            var iceCreamWithOldVariations = context.IceCreams
                .Include(i => i.Variations)
                    .ThenInclude(i => i.Qualities)
                .OrderBy(i => i.IceCreamId)
                .FirstOrDefault();

            Debug.Assert(iceCreamWithOldVariations.Variations.Count == 2);

            var vanillaIceCream = iceCreamWithOldVariations;
            vanillaIceCream.Variations.Clear();
            vanillaIceCream.Variations.Add(
                new IceCreamVariation
                {
                    Name = "Vanilla Cheesecake",
                    Qualities = new IceCreamVariationQuality[]
                    {
                        new IceCreamVariationQuality { Name = "Healthy" },
                    },
                });
            vanillaIceCream.Variations.Add(
                new IceCreamVariation
                {
                    Name = "Vanilla Cheesecake",
                    Qualities = new IceCreamVariationQuality[]
                    {
                        new IceCreamVariationQuality { Name = "Fresh" },
                        new IceCreamVariationQuality { Name = "Cool" },
                    },
                });

            context.SaveChanges();

            var iceCreamWithNewVariations = context.IceCreams
                .Include(i => i.Variations)
                    .ThenInclude(i => i.Qualities)
                .OrderBy(i => i.IceCreamId)
                .FirstOrDefault();

            Debug.Assert(iceCreamWithNewVariations.Variations.Count == 2);
        }
    }
}

}


The code above yields the following log:

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
Executed DbCommand (20ms) [Parameters=[@p0='1', @p1='2', @p2='3', @p3='1', @p4='Vanilla Cheesecake' (Size = 4000), @p5='1', @p6='Vanilla Cheesecake' (Size = 4000)], CommandType='Text', CommandTimeout='30']
DELETE FROM IceCreamVariationQuality
WHERE IceCreamVariationQualityId = @p0;
SELECT ROW_COUNT();

  DELETE FROM `IceCreamVariationQuality`
  WHERE `IceCreamVariationQualityId` = @p1;
  SELECT ROW_COUNT();

  DELETE FROM `IceCreamVariationQuality`
  WHERE `IceCreamVariationQualityId` = @p2;
  SELECT ROW_COUNT();

  INSERT INTO `IceCreamVariations` (`IceCreamId`, `Name`)
  VALUES (@p3, @p4);
  SELECT `IceCreamVariationId`
  FROM `IceCreamVariations`
  WHERE ROW_COUNT() = 1 AND `IceCreamVariationId` = LAST_INSERT_ID();

  INSERT INTO `IceCreamVariations` (`IceCreamId`, `Name`)
  VALUES (@p5, @p6);
  SELECT `IceCreamVariationId`
  FROM `IceCreamVariations`
  WHERE ROW_COUNT() = 1 AND `IceCreamVariationId` = LAST_INSERT_ID();

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
Executed DbCommand (4ms) [Parameters=[@p7='3', @p8='Healthy' (Size = 4000), @p9='4', @p10='Fresh' (Size = 4000), @p11='4', @p12='Cool' (Size = 4000), @p13='1', @p14='2'], CommandType='Text', CommandTimeout='30']
INSERT INTO IceCreamVariationQuality (IceCreamVariationId, Name)
VALUES (@p7, @p8);
SELECT IceCreamVariationQualityId
FROM IceCreamVariationQuality
WHERE ROW_COUNT() = 1 AND IceCreamVariationQualityId = LAST_INSERT_ID();

  INSERT INTO `IceCreamVariationQuality` (`IceCreamVariationId`, `Name`)
  VALUES (@p9, @p10);
  SELECT `IceCreamVariationQualityId`
  FROM `IceCreamVariationQuality`
  WHERE ROW_COUNT() = 1 AND `IceCreamVariationQualityId` = LAST_INSERT_ID();

  INSERT INTO `IceCreamVariationQuality` (`IceCreamVariationId`, `Name`)
  VALUES (@p11, @p12);
  SELECT `IceCreamVariationQualityId`
  FROM `IceCreamVariationQuality`
  WHERE ROW_COUNT() = 1 AND `IceCreamVariationQualityId` = LAST_INSERT_ID();

  DELETE FROM `IceCreamVariations`
  WHERE `IceCreamVariationId` = @p13;
  SELECT ROW_COUNT();

  DELETE FROM `IceCreamVariations`
  WHERE `IceCreamVariationId` = @p14;
  SELECT ROW_COUNT();

```

Further technical details

EF Core version: 3.1.1
Database provider: Pomelo.EntityFrameworkCore.MySql 3.1.1
Target framework: .NET Core 3.1
Operating system: macOS Mojave
IDE: -

area-save-changes closed-fixed customer-reported type-bug
Source
picture shoooe

Most helpful comment

No, this should not be closed, because alternate keys should generally also work in the described case, but don't at the moment.

picture lauxjpn on 12 May 2020
👍5

All 7 comments

What kind of constraint your table have?

smitpatel picture smitpatel on 7 May 2020

@smitpatel The issue can be reproduced with SQL Server as well:

```c#
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

namespace IssueConsoleTemplate
{
public class IceCream
{
public int IceCreamId { get; set; }
public string Name { get; set; }

    public ICollection<IceCreamVariation> Variations { get; set; } = new HashSet<IceCreamVariation>();
}

public class IceCreamVariation
{
    public int IceCreamVariationId { get; set; }
    public string Name { get; set; }
    public int UniqueId { get; set; }
    public int IceCreamId { get; set; }

    public IceCream IceCream { get; set; }

    public ICollection<IceCreamVariationQuality> Qualities { get; set; } = new HashSet<IceCreamVariationQuality>();
}

public class IceCreamVariationQuality
{
    public int IceCreamVariationQualityId { get; set; }
    public string Name { get; set; }
    public int IceCreamVariationId { get; set; }

    public IceCreamVariation IceCreamVariation { get; set; }
}

public class Context : DbContext
{
    public DbSet<IceCream> IceCreams { get; set; }
    public DbSet<IceCreamVariation> IceCreamVariations { get; set; }
    public DbSet<IceCreamVariationQuality> IceCreamVariationQualities { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlServer(@"Data Source=.\MSSQL14;Integrated Security=true;Initial Catalog=So61383388_01")
            //.UseMySql(
            //    "server=127.0.0.1;port=3306;user=root;password=;database=So61383388_01",
            //    b => b.ServerVersion("8.0.20-mysql"))
            .UseLoggerFactory(
                LoggerFactory.Create(
                    b => b
                        .AddConsole()
                        .AddFilter(level => level >= LogLevel.Information)))
            .EnableSensitiveDataLogging()
            .EnableDetailedErrors();
    }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.Entity<IceCream>()
            .HasData(
                new IceCream {IceCreamId = 1, Name = "Vanilla"}
            );

        modelBuilder.Entity<IceCreamVariation>(
            entity =>
            {
                entity.HasAlternateKey(e => e.UniqueId);

                entity.HasData(
                    new IceCreamVariation
                    {
                        IceCreamVariationId = 1,
                        Name = "Double Vanilla Bourbon",
                        UniqueId = 42, // <-- this value is part of a unique index
                        IceCreamId = 1
                    }
                );
            });

        modelBuilder.Entity<IceCreamVariationQuality>()
            .HasData(
                new IceCreamVariationQuality
                {
                    IceCreamVariationQualityId = 1,
                    Name = "Yummy",
                    IceCreamVariationId = 1
                }
            );
    }
}

internal class Program
{
    private static void Main()
    {
        using (var context = new Context())
        {
            context.Database.EnsureDeleted();
            context.Database.EnsureCreated();

            var iceCreamWithOldVariations = context.IceCreams
                .Include(i => i.Variations)
                    .ThenInclude(i => i.Qualities)
                .OrderBy(i => i.IceCreamId)
                .First();

            Debug.Assert(iceCreamWithOldVariations.Variations.Count == 1);
            Debug.Assert(iceCreamWithOldVariations.Variations.Single().UniqueId == 42);
            Debug.Assert(iceCreamWithOldVariations.Variations.Single().Qualities.First().Name == "Yummy");

            iceCreamWithOldVariations.Variations.Clear();
            iceCreamWithOldVariations.Variations.Add(
                new IceCreamVariation 
                {
                    Name = "Vanilla Cheesecake",
                    UniqueId = 42, // <-- use same value again; should work because previous entity was removed
                    Qualities = new[]
                    {
                        new IceCreamVariationQuality { Name = "Healthy" },
                    },
                });

            context.SaveChanges();

            var iceCreamWithNewVariations = context.IceCreams
                .Include(i => i.Variations)
                    .ThenInclude(i => i.Qualities)
                .OrderBy(i => i.IceCreamId)
                .First();

            Debug.Assert(iceCreamWithNewVariations.Variations.Count == 1);
            Debug.Assert(iceCreamWithNewVariations.Variations.Single().UniqueId == 42);
            Debug.Assert(iceCreamWithNewVariations.Variations.Single().Qualities.First().Name == "Healthy");
        }
    }
}

}


If executed when the `UniqueId` property has been declared (as in the SQL Server sample), an exception is being thrown when calling `SaveChanges()` and the generated SQL is even more unexpected:

```sql
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (15ms) [Parameters=[@p0='1'], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      DELETE FROM [IceCreamVariationQualities]
      WHERE [IceCreamVariationQualityId] = @p0;
      SELECT @@ROWCOUNT;

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (3ms) [Parameters=[@p1='1', @p0='Vanilla Cheesecake' (Size = 4000)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      UPDATE [IceCreamVariations] SET [Name] = @p0
      WHERE [IceCreamVariationId] = @p1;
      SELECT [IceCreamVariationId]
      FROM [IceCreamVariations]
      WHERE @@ROWCOUNT = 1 AND [IceCreamVariationId] = scope_identity();

Here, an UPDATE statement has been generated to change the name of a IceCreamVariation entity (which is wrong), that assumes the entity has just been inserted (which is not the case), because it uses scope_identity().

This is only an issue, when deleting grand child and child entities (hierarchy must be two levels deep). It works as expected for one level deep deletes.

This issue discussion started at StackOverflow.

What kind of constraint your table have?

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (8ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE TABLE [IceCreams] (
          [IceCreamId] int NOT NULL IDENTITY,
          [Name] nvarchar(max) NULL,
          CONSTRAINT [PK_IceCreams] PRIMARY KEY ([IceCreamId])
      );

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE TABLE [IceCreamVariations] (
          [IceCreamVariationId] int NOT NULL IDENTITY,
          [Name] nvarchar(max) NULL,
          [UniqueId] int NOT NULL, /* <-- uses UNIQUE constraint/index */
          [IceCreamId] int NOT NULL,
          CONSTRAINT [PK_IceCreamVariations] PRIMARY KEY ([IceCreamVariationId]),
          CONSTRAINT [AK_IceCreamVariations_UniqueId] UNIQUE ([UniqueId]),
          CONSTRAINT [FK_IceCreamVariations_IceCreams_IceCreamId] FOREIGN KEY ([IceCreamId]) REFERENCES [IceCreams] ([IceCreamId]) ON DELETE CASCADE
      );

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE TABLE [IceCreamVariationQualities] (
          [IceCreamVariationQualityId] int NOT NULL IDENTITY,
          [Name] nvarchar(max) NULL,
          [IceCreamVariationId] int NOT NULL,
          CONSTRAINT [PK_IceCreamVariationQualities] PRIMARY KEY ([IceCreamVariationQualityId]),
          CONSTRAINT [FK_IceCreamVariationQualities_IceCreamVariations_IceCreamVariationId] FOREIGN KEY ([IceCreamVariationId]) REFERENCES [IceCreamVariations] ([IceCreamVariatio
nId]) ON DELETE CASCADE
      );

/* ... */

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE INDEX [IX_IceCreamVariationQualities_IceCreamVariationId] ON [IceCreamVariationQualities] ([IceCreamVariationId]);

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[], CommandType='Text', CommandTimeout='30']
      CREATE INDEX [IX_IceCreamVariations_IceCreamId] ON [IceCreamVariations] ([IceCreamId]);

So the child table IceCreamVariations has a foreign key to the parent table IceCreams (with ON DELETE CASCADE) and contains a unique index on its UniqueId column.

The grand child table IceCreamVariationQualities has a foreign key to the child table IceCreamVariations (with ON DELETE CASCADE).

lauxjpn picture lauxjpn on 7 May 2020

Does it work if you configure unique index in EF core rather than alternate key?
Changing entity.HasAlternateKey(e => e.UniqueId); to
entity.HasIndex(e => e.UniqueId).IsUnique();

smitpatel picture smitpatel on 7 May 2020
👍3

Does it work if you configure unique index in EF core rather than alternate key?

Yes, using an unique index instead of an alternate key does work as expected! (/cc @shoooe)

info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (18ms) [Parameters=[@p0='1'], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      DELETE FROM [IceCreamVariationQualities]
      WHERE [IceCreamVariationQualityId] = @p0;
      SELECT @@ROWCOUNT;
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (3ms) [Parameters=[@p1='1'], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      DELETE FROM [IceCreamVariations]
      WHERE [IceCreamVariationId] = @p1;
      SELECT @@ROWCOUNT;
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (3ms) [Parameters=[@p2='1', @p3='Vanilla Cheesecake' (Size = 4000), @p4='42'], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      INSERT INTO [IceCreamVariations] ([IceCreamId], [Name], [UniqueId])
      VALUES (@p2, @p3, @p4);
      SELECT [IceCreamVariationId]
      FROM [IceCreamVariations]
      WHERE @@ROWCOUNT = 1 AND [IceCreamVariationId] = scope_identity();
info: Microsoft.EntityFrameworkCore.Database.Command[20101]
      Executed DbCommand (2ms) [Parameters=[@p5='2', @p6='Healthy' (Size = 4000)], CommandType='Text', CommandTimeout='30']
      SET NOCOUNT ON;
      INSERT INTO [IceCreamVariationQualities] ([IceCreamVariationId], [Name])
      VALUES (@p5, @p6);
      SELECT [IceCreamVariationQualityId]
      FROM [IceCreamVariationQualities]
      WHERE @@ROWCOUNT = 1 AND [IceCreamVariationQualityId] = scope_identity();

Interesting though, that without the unique constraint (when we remove UniqueId from the code), the order of the INSERT and DELETE statements is all over the place (as can be seen in the SQL log of the original post). Couldn't this lead to other potential issues, or is this a deliberate choice (probably to solve some other issues that could arise)?

I would have expected a dependency walker to figure out the dependency order, and then first to execute all deletes (in dependency order), then all inserts (in dependency order) and finally all updates (in dependency order). But I am probably missing something here.

lauxjpn picture lauxjpn on 7 May 2020

My specific constraint is a two column index, I'll give that a try and update the issue.

shoooe picture shoooe on 8 May 2020

Yes, I can confirm that specifying the index solves the issue.
We can close this, thanks.

shoe-diamente picture shoe-diamente on 12 May 2020

No, this should not be closed, because alternate keys should generally also work in the described case, but don't at the moment.

lauxjpn picture lauxjpn on 12 May 2020
👍5
Was this page helpful?
0 / 5 - 0 ratings

Related issues

januszj picture januszj  路  3Comments
yukozh picture yukozh  路  3Comments
TPJ11 picture TPJ11  路  3Comments
HappyNomad picture HappyNomad  路  3Comments
julienshepherd picture julienshepherd  路  3Comments