Topological sort should consider unique constraints for ordering #20870

shoooe · 2020-05-07T07:32:12Z

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:

IceCreamVariationQualitys are removed first
new IceCreamVariations are added
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:

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: -

The text was updated successfully, but these errors were encountered:

smitpatel · 2020-05-07T13:18:17Z

What kind of constraint your table have?

lauxjpn · 2020-05-07T15:36:56Z

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

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:

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).

smitpatel · 2020-05-07T15:56:02Z

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();

lauxjpn · 2020-05-07T16:46:41Z

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.

shoooe · 2020-05-08T12:16:12Z

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

shoe-diamente · 2020-05-12T08:37:53Z

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

lauxjpn · 2020-05-12T13:10:01Z

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

Don't use entry sharing for entities with the same AK values Only remove the entry with duplicate key value from the identity map if it's still there Fixes #20870

shoooe added the customer-reported label May 7, 2020

smitpatel changed the title ~~Inconvenient statement ordering on update of list of items that have sub items~~ Topological sort should consider unique constraints for ordering May 7, 2020

smitpatel added the type-enhancement label May 7, 2020

ajcvickers added area-save-changes type-bug and removed type-enhancement labels May 8, 2020

ajcvickers added this to the 5.0.0 milestone May 8, 2020

ajcvickers assigned AndriySvyryd May 8, 2020

AndriySvyryd added a commit that referenced this issue Jul 8, 2020

Include key values in the update topological sort

fc2c67c

Don't use entry sharing for entities with the same AK values Only remove the entry with duplicate key value from the identity map if it's still there Fixes #20870

AndriySvyryd removed their assignment Jul 8, 2020

AndriySvyryd added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Jul 8, 2020

AndriySvyryd mentioned this issue Jul 8, 2020

Include key values in the update topological sort #21556

Merged

AndriySvyryd added a commit that referenced this issue Jul 8, 2020

Include key values in the update topological sort

d8ee3b3

Don't use entry sharing for entities with the same AK values Only remove the entry with duplicate key value from the identity map if it's still there Fixes #20870

ghost closed this as completed in #21556 Jul 10, 2020

ghost pushed a commit that referenced this issue Jul 10, 2020

Include key values in the update topological sort (#21556)

78646d2

Don't use entry sharing for entities with the same AK values Only remove the entry with duplicate key value from the identity map if it's still there Fixes #20870

ajcvickers modified the milestones: 5.0.0, 5.0.0-preview8 Jul 14, 2020

ajcvickers modified the milestones: 5.0.0-preview8, 5.0.0 Nov 7, 2020

This issue was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Topological sort should consider unique constraints for ordering #20870

Topological sort should consider unique constraints for ordering #20870

shoooe commented May 7, 2020

smitpatel commented May 7, 2020

lauxjpn commented May 7, 2020 •

edited

Loading

smitpatel commented May 7, 2020

lauxjpn commented May 7, 2020 •

edited

Loading

shoooe commented May 8, 2020

shoe-diamente commented May 12, 2020

lauxjpn commented May 12, 2020

Topological sort should consider unique constraints for ordering #20870

Topological sort should consider unique constraints for ordering #20870

Comments

shoooe commented May 7, 2020

Steps to reproduce

Further technical details

smitpatel commented May 7, 2020

lauxjpn commented May 7, 2020 • edited Loading

smitpatel commented May 7, 2020

lauxjpn commented May 7, 2020 • edited Loading

shoooe commented May 8, 2020

shoe-diamente commented May 12, 2020

lauxjpn commented May 12, 2020

lauxjpn commented May 7, 2020 •

edited

Loading

lauxjpn commented May 7, 2020 •

edited

Loading