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

[Regression] Insertion order is wrong when owned entity is involved #23668

Closed
Neme12 opened this issue Dec 12, 2020 · 2 comments
Closed

[Regression] Insertion order is wrong when owned entity is involved #23668

Neme12 opened this issue Dec 12, 2020 · 2 comments
Labels
area-save-changes closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Milestone

Comments

@Neme12
Copy link

Neme12 commented Dec 12, 2020

Steps to reproduce

  1. Start with this project file and a C# file:
<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net5.0</TargetFramework>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Microsoft.EntityFrameworkCore.SqlServer" Version="5.0.1" />
  </ItemGroup>

</Project>
using Microsoft.EntityFrameworkCore;
using System;

namespace EfCore5Test
{
    public sealed class FileMetadata
    {
        public Guid Id { get; set; }
    }

    public sealed class Category
    {
        public Guid Id { get; set; }

        public FileSource Picture { get; set; }
    }

    public sealed class FileSource
    {
        public Guid? FileId { get; set; }
    }

    public sealed class ApplicationDbContext : DbContext
    {
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            base.OnConfiguring(optionsBuilder);
            optionsBuilder.UseSqlServer("Server=(localdb)\\mssqllocaldb;Database=aspnet-WebApplication-6E483A89-BEE6-4A76-96CC-CEB276E5E112;Trusted_Connection=True;MultipleActiveResultSets=true");
        }

        protected override void OnModelCreating(ModelBuilder modelBuilder)
        {
            base.OnModelCreating(modelBuilder);

            modelBuilder.Entity<Category>(builder =>
            {
                builder.OwnsOne(x => x.Picture, fileSource =>
                {
                    fileSource.HasOne<FileMetadata>().WithOne().HasForeignKey<FileSource>(x => x.FileId);
                });
            });
        }

        public DbSet<FileMetadata> FileMetadata { get; set; }
        public DbSet<Category> Categories { get; set; }
    }

    class Program
    {
        static void Main(string[] args)
        {
            using (var context = new ApplicationDbContext())
            {
                context.Database.EnsureDeleted();
                context.Database.EnsureCreated();

                var category = new Category { };
                context.Categories.Add(category);

                context.SaveChanges();

                var fileMetadata = new FileMetadata { };
                context.FileMetadata.Add(fileMetadata);
                category.Picture = new FileSource { FileId = fileMetadata.Id };

                context.SaveChanges();
            }
        }
    }
}
  1. Run. An exception is thrown:
Unhandled exception. Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details.
 ---> Microsoft.Data.SqlClient.SqlException (0x80131904): The UPDATE statement conflicted with the FOREIGN KEY constraint "FK_Categories_FileMetadata_Picture_FileId". The conflict occurred in database "aspnet-WebApplication-6E483A89-BEE6-4A76-96CC-CEB276E5E112", table "dbo.FileMetadata", column 'Id'.
The statement has been terminated.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlDataReader.TryConsumeMetaData()
   at Microsoft.Data.SqlClient.SqlDataReader.get_MetaData()
   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean isAsync, Int32 timeout, Task& task, Boolean asyncWrite, Boolean inRetry, SqlDataReader ds, Boolean describeParameterEncryptionRequest)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean& usedCache, Boolean asyncWrite, Boolean inRetry, String method)
   at Microsoft.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
   at Microsoft.Data.SqlClient.SqlCommand.ExecuteDbDataReader(CommandBehavior behavior)
   at System.Data.Common.DbCommand.ExecuteReader()
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReader(RelationalCommandParameterObject parameterObject)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
ClientConnectionId:5c5233b1-320d-44fe-9fc0-fa2caa84e122
Error Number:547,State:0,Class:16
   --- End of inner exception stack trace ---
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(DbContext _, Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()
   at EfCore5Test.Program.Main(String[] args) in C:\Users\SimonaKonickova\source\repos\EfCore5Test\EfCore5Test\Program.cs:line 67

This seems to be a regression because it worked properly in EF Core 3.1 - downgrading the Microsoft.EntityFrameworkCore.SqlServer package makes this work. Is there any workaround for this (other than adding one more roundtrip to the database to make sure that FileMetadata is inserted first)?

Include provider and version information

EF Core version: 5.0.1
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 5.0
Operating system: Windows 10 Version 1909
IDE: Visual Studio 16.9.0 Preview 2.0

@Neme12
Copy link
Author

Neme12 commented Dec 12, 2020

I'm sorry but this is so frustrating. With every release, EF Core has so many regressions and breaking changes while .NET Core itself and also ASP.NET Core are much more stable now. I test my app on EF Core 5 and report bugs that prevent me from testing any further, Some get fixed in preview releases, some in RTM and others in 5.0.1, only for me to then try again and run into other bugs or breaking changes. It is so incredibly difficult to upgrade to a new EF Core version every time 😞

From the outside, it just seems like EF Core must have very poor test coverage with only basic cases covered with so many regressions. Maybe this will get fixed for owned entity types. But what about other interesting scenarios. TPH inheritance or TPT inheritance? Do they work or do they have no test coverage either? 😕 Especially issues like #23401, which made owned entities completely unusable if their owner has a PK composed of more than 1 property, really surprise me - it's such a basic scenario. How did this creep into the final release? Sometimes I feel like I'm the only one actually using EF Core for anything serious 😟 Sorry for being so unpleasant but it's just so tiring at this point finding so many regressions with every release. I just hope this is going to get better in the future

@Neme12
Copy link
Author

Neme12 commented Dec 12, 2020

I'm guessing the problem is somewhere in CommandBatchPreparer.TopologicalSort:
https://github.com/dotnet/efcore/blob/release/5.0/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs#L281

I got a workaround by overriding CommandBatchPreparer:

class CustomCommandBatchPreparer : CommandBatchPreparer
{
    public CustomCommandBatchPreparer(CommandBatchPreparerDependencies dependencies) : base(dependencies)
    {
    }

    protected override IReadOnlyList<List<ModificationCommand>> TopologicalSort(IEnumerable<ModificationCommand> commands)
    {
        var result = base.TopologicalSort(commands);

        var commandsToPrepend = new List<ModificationCommand>();

        foreach (var modificationList in result)
        {
            for (int i = modificationList.Count - 1; i >= 0; --i)
            {
                var modification = modificationList[i];
                if (modification is { TableName: "FileMetadata", EntityState: EntityState.Added })
                {
                    modificationList.RemoveAt(i);
                    commandsToPrepend.Add(modification);
                }
            }
        }

        if (commandsToPrepend.Count > 0)
            ((List<List<ModificationCommand>>)result).Insert(0, commandsToPrepend);

        return result;
    }
}

@ajcvickers ajcvickers added this to the 5.0.3 milestone Dec 15, 2020
AndriySvyryd added a commit that referenced this issue Dec 15, 2020
AndriySvyryd added a commit that referenced this issue Dec 15, 2020
@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 Dec 15, 2020
@AndriySvyryd AndriySvyryd removed their assignment Dec 15, 2020
This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-save-changes closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. customer-reported regression Servicing-approved type-bug
Projects
None yet
Development

No branches or pull requests

3 participants