Throw better exception for a unique index cycle during update

[jwisener]

Describe what is not working as expected.

I have a pretty simple model. ( I have also attached a solution with db project and a basic console app to to reproduce the error). When the program runs, it will clean the tables, populate some data, swap the values and then try to save. You'll just need to set the connection string to a test database on line 19 of MyDBContext.cs

Tables are as follows:

1. Users
2. Location
3. UserLocation (Many to Many) between 1 & 2.

In addition I have a unique index on UserLocation that has a filter. When I try to update (basically swap the values for two rows), I am getting a unique index violation.

When I profile the sql, EF Core does not issue the update commands in the right order.

The actual production bug we have currently is a little more complex than this, but I have basically distilled it down to this.

If you are seeing an exception, include the full exceptions details (message and stack trace).

Exception message:
SqlException: Cannot insert duplicate key row in object 'dbo.UserLocation' with unique index 'UIX_UserLocation_Priority'. The duplicate key value is (1, 1).
The statement has been terminated.

Stack trace:

  | Name | Value | Type
-- | -- | -- | --
  | Source | "Core .Net SqlClient Data Provider" | string


  | Name | Value | Type
-- | -- | -- | --
  | StackTrace | "   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)\r\n   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)\r\n   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)\r\n   at System.Data.SqlClient.SqlDataReader.TryConsumeMetaData()\r\n   at System.Data.SqlClient.SqlDataReader.get_MetaData()\r\n   at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString)\r\n   at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, SqlDataReader ds)\r\n   at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)\r\n   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.Execute(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary`2 parameterValues)\r\n   at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteReader(IRelationalConnection connection, IReadOnlyDictionary`2 parameterValues)\r\n   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)" | string

Steps to reproduce

Include a complete code listing (or project/solution) that we can run to reproduce the issue.

Partial code listings, or multiple fragments of code, will slow down our response or cause us to push the issue back to you to provide code to reproduce the issue.

Console.WriteLine("Hello World!");

Further technical details

EF Core version: (found in project.csproj or packages.config)
CoreUpdateWrongOrderIssue.zip
CoreUpdateWrongOrderIssue.zip
Database Provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer)
Operating system:
IDE: (e.g. Visual Studio 2017 15.4)

[ajcvickers]

Note for triage: EF Core 2.2.4

[AndriySvyryd]

@jwisener When you are swapping values between two rows that have unique indexes there is no way to order the updates that doesn't result in an exception.
If the property being changed was nullable EF could set it to null temporarily if we implement cycle breaking

There are several workarounds for this:

• Make the index non-unique.
• Make the property being modified (Priority) nullable, set it to null and save, then do the swap.
• Delete one of the rows, modify the other, add the first one back.

[jwisener]

@AndriySvyryd for item #3 (bullet). is are implying that I should call a "save" between the delete, modify and add back? Because I don't see how EF Entity tracking could maintain a "delete" and an "add" within a "SaveEntities"? If so then all these workarounds require multiple saves which are no good, because we have critical messaging events that are fired as part of our save. So for one HTTP request, at the end we commit our unit of work.

[AndriySvyryd]

@jwisener Yes, without cycle-breaking support there is no way to accomplish this with one SaveChanges call while preserving uniqueness.

[jwisener]

Actually there is, I have overriden the OnBefore Save and run custom sql to get all these values to something other than the "unque filtered value". So I basically demote them all to Secondary, and then the existing code runs so that new Primary is promoted.

So that allows me to remove and then add properly (swap primary for a secondary or a secondary for a primary). All within one transaction.

The only problem now, is after the save is complete, if I re-issue the query against the database the new "primary" is not getting projected properly (primary is null). I have profiled the database and I see the query that is executed and it returns a value, but it's like the change tracker, is not properly materializing the object, or resolving it.

for example:

           user.SwapPrimaryForSecondar();
           _userRepo.UnitOfWork.Save();
          var userAfterSave = _userRepo.GetById(1);

         var whyNull = userAfterSave.PrimaryLocation;  //is now "null", even though the query from the 
                                                                                   //db that profiled shows data is actually there 
                                                                                  //and coming back. 

          Is the changeTracker continuing to "hold" on to the state tracking even on issuing a new query? Lifetime of the Context the one request. 

[AndriySvyryd]

The include issue is likely fixed in 3.0, try using the preview.

As a workaround can you query with a different context instance or detach the affected entities after saving.

[jwisener]

"Likely fixed" that doesn't sound too certain. Do you have a link to this issue (yet another)?

Creating a new context instance is a pain when you have laid down architectural patterns that define the lifetime scope of the DBContext to be "per request". In our case we are using .net core dependency injection, as it's per request. so that is out, we just don't "new up" db contexts everywhere.

if one detaches per your suggestion, does that mean if they need to use the objects after the save to say, set state again, would you have to attach them back? Now the code turns into a state management nightmare. In a DDD type architecture for an enterprise system, access to the "dbcontext" is a big NO NO inside our domain objects. In fact, they have no knowledge of it, as that is an infrastructure concern. So it's not like we are writing a simple application where we just pass around the DB context to whatever wants it.

Everything concerning EF has been a pain from the very start, I really see no good solution or workaround here for this major problem. Unfortunately using a "preview version" in a production environment, given the major problems we've had thus far, and I'm sure there are lots of breaking changes, I'm just not keen on using 3.0 "preview". Unfortunately, we don't have the luxury of time at moment, as we are trying to provide business value to the business. and not constantly "working around EF Core"

[jwisener]

As usual, no response for over a year, closing since it's been forgotten about.

[AndriySvyryd]

Creating a new context instance is a pain when you have laid down architectural patterns that define the lifetime scope of the DBContext to be "per request". In our case we are using .net core dependency injection, as it's per request. so that is out, we just don't "new up" db contexts everywhere.

In 5.0 you'll be able to register a factory: #18575

We still want to improve the exception message. We have over a thousand issues in the backlog, it will take some time to get to all to them, please be patient.

[AndriySvyryd]

After taking another look there's nothing we can do here. We would need to evaluate the filter on the index and that's out of scope for EF.