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

Microsoft.EntityFrameworkCore.DbUpdateConcurrencyException with Temporal Table and a non-clustered index on history table #28281

Closed
IT-CASADO opened this issue Jun 21, 2022 · 9 comments

Comments

@IT-CASADO
Copy link

File a bug

Found a new issue with generated MERGE statement in a constellation with TEMPORAL TABLE, COLUMNSTORE INDEX and NON-CLUSTERED INDEXES:

I get the following Entity Framework error:
"Database operation expected to affect 300 row(s) but actually affected 0 row(s). Data may have been modified or deleted since entities were loaded."

This error appears currently ONLY within SQL Azure Databases. I don't see this error with SQL 2017/2019.

In general this is an issue inside SQL Azure Databases and I opened a question about this here:
https://dba.stackexchange.com/questions/313569/why-merge-doesnt-insert-more-than-277-records-into-a-table-which-is-configured

In the past I stumbled over many issues with the MERGE statement.
Could it be better to replace MERGE by a simple INSERT INTO to don't trap into such issues?

Include your code

You can see this error in this runnable project: https://github.com/IT-CASADO/ef-core-sql-server-merge-bug

  1. After cloning this repo please change the connection string in MyContext.cs to a SQL Azure Database
  2. Run the test at least twice
    Because of cached execution plans (?) there is a high chance for a green test on first execution.

Include provider and version information

EF Core version: 6.x
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: (e.g. .NET 6.0)
Operating system: Windows

@roji
Copy link
Member

roji commented Jun 21, 2022

Your test never fails for me - it's been running in a loop for a while now and always passes. I'm using the latest SQL Server 2019.

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 21, 2022

@roji It's an Azure SQL DB bug...

@roji
Copy link
Member

roji commented Jun 21, 2022

Oh sorry, I missed the SQL Azure part.

I can try to repro this on SQL Azure, but if it reproduces, then it really is something to be reported to SQL Azure...

@ErikEJ
Copy link
Contributor

ErikEJ commented Jun 21, 2022

Has been done already!

@roji
Copy link
Member

roji commented Jun 21, 2022

@ErikEJ @IT-CASADO is this the issue tracking this on the SQL Azure side (found via the SO link above)?

In any case, there's not much we can do here except point to the workaround, which is to set MaxBatchSize to 1; it's bad for perf (when multiple commands are done via the same SaveChanges), but it avoids MERGE.

@IT-CASADO
Copy link
Author

No, I opened yesterday an official support ticket on Microsoft (by our service provider).

@roji: Your link could be related to this one: #28282

@roji
Copy link
Member

roji commented Jun 21, 2022

@IT-CASADO if you have some sort of publicly-accessible link to the ticket, it would be great if you could post it here.

Your link could be related to this one: #28282

I don't think so, #28282 is purely an EF translation question that has nothing to do with SQL Server.

@IT-CASADO
Copy link
Author

Yes, it is a problem inside SQL Azure Database (a support ticket exists already).

But for me the question is, is the translation to a MERGE statement really the correct decision!?
In the last month I stumbled over multiple issues with MERGE:

@roji
Copy link
Member

roji commented Jun 21, 2022

Re performance, the situation is a bit complex, here's the reasoning (benchmarking is available here):

  • EF Core needs to retrieve database-generated values (e.g. IDs) back after insertion, to assign them in the tracked CLR entity (so it can be further used). Allowing opting out of this is tracked by Do not track after SaveChanges() #9118.
  • Using a single INSERT statement with many row values (Insert_row_values_without_Output in the benchmark) is indeed very efficient, but there's no way to reliably get back the database-generated values; this is because SQL Server doesn't guarantee the ordering of rows returned from INSERT ... OUTPUT.
  • We can instead sending many batched INSERTS - each for a single row (BatchedInserts in the benchmark), but that's considerably slower than MERGE (Merge_with_Output in the benchmark).

In other words, given our constraints (which include getting back database-generated values), MERGE offers very significant perf advantages, and in the vast majority of cases has no issue. If there are indeed SQL Azure bugs, I'm hopeful that these can be fixed, as they should be.

The other general arguments against MERGE's reliability are usually against specific patterns of MERGEs (e.g. when you use MERGE for insert-or-update). We use MERGE in a very constrained way (to insert multiple rows), and as far as we know it's safe and reliable (it has been in use since the very first versions of EF Core).

@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants