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

Instead of insert triggers & inserts with WHERE @@ROWCOUNT = 1 AND [IdentityColumnName] = scope_identity(); #35508

Closed
lukenuttall opened this issue Jan 22, 2025 · 4 comments

Comments

@lukenuttall
Copy link

Question

The problem

We have an existing old database with lots of instead of insert triggers and we are having trouble adding multiple entities in a single batch on EF 8 unless we set the batch size to 1 or call SaveChanges() and round tripping to the db more than we should. The desired outcome for the code with the issues is to eager load, updating tracked entities and inserting new ones, then calling SaveChanges() once.

The instead of insert triggers have been modified to output the identities being created and concurrency tokens, but the SELECT ... FROM [TableName] WHERE @@ROWCOUNT = 1 AND [[IdentityColumnName]] = scope_identity(); is generating an empty result set as scope_identity() is null since the trigger is running in another scope. When EF gets to the first empty result set it gives up and throws a concurrency error, so it works with one record since the trigger outputted what EF needs, but not 2 or 3 records. If I try to insert 4 records though, the insert gets turned into a merge into with output into, which works great since output into works fine if a table has triggers.

I can workaround it by setting the MaxBatchSize to 1, since the first EF picks up the first record set that the trigger generated, but that doesn't feel like a great way to do things.

Questions

How can I force inserts to particular tables to either:

  1. Use single inserts with output into when I have 1 - 3 records, or
  2. Omit the select with scope_identity(), or
  3. Always trigger the merge into with output into, or
  4. Force ef to always use output into even with single insertions?

Looking at https://github.com/dotnet/efcore/blob/main/src/EFCore.SqlServer/Update/Internal/SqlServerUpdateSqlGenerator.cs#L326 it seems that MergeIntoMinimumThreshold could be configurable or I could add something like builder.ToTable(tableName, builder => builder.UseSqlOutputIntoClause(true)); as opposed UseSqlOutputClause, which doesn't work if the table has triggers.

Is there another way to do this or is it 100% a bad idea?

EF Generated SQL (table names/columns redacted)

EF is generating (with

SET NOCOUNT ON;

INSERT INTO [TableName] (....)
VALUES(....)
SELECT [IdentityColumnName], [ConcurrencyToken]
FROM [TableName]
WHERE @@RowCount = 1 AND [IdentityColumnName] = scope_identity();

INSERT INTO [TableName] (....)
VALUES(....)
SELECT [IdentityColumnName], [ConcurrencyToken]
FROM [TableName]
WHERE @@RowCount = 1 AND [IdentityColumnName] = scope_identity();

Re-running the sql captured via SQLProfiler

Running the sql in data studio I get:
Result set from trigger
Empty result set from the scope_identity() select
Result set from trigger
Empty result set from the scope_identity() select
Result set with an int EF has declared and is returning (presumably for tracking/tracing/mars or something I don't understand)

Image

Your code

Stack traces


Verbose output


EF Core version

8.0.11

Database provider

No response

Target framework

8.0.404

Operating system

Windows 11

IDE

Rider

@lukenuttall lukenuttall changed the title Instead of insert triggers & WHERE @@ROWCOUNT = 1 AND [MembershipPlanIdentity] = scope_identity(); Instead of insert triggers & inserts with WHERE @@ROWCOUNT = 1 AND [IdentityColumnName] = scope_identity(); Jan 22, 2025
@AndriySvyryd
Copy link
Member

You could call builder.ToTable(tableName, builder => builder.UseSqlOutputClause(false));

@AndriySvyryd
Copy link
Member

I think that the long-term solution for this will be #10443

@lukenuttall
Copy link
Author

@AndriySvyryd It's not that I want to opt out of optimistic concurrency. Setting using SqlOutputClause(false) doesn't really help, since the issue for an instead of insert trigger is that the selection of the generated values doesn't work as scope_identity() returns null as the insert happens in the trigger's scope. We are already marking all the tables with triggers using the BlankTriggerAddingConvention reported elsewhere which works for inserting a single row as we are currently returning the actually inserted row from the trigger.

It seems this can just work for instead of insert triggers though if we use an output into clause to capture the generated values , which is what happens if we insert MergeIntoMinimumThreshold records, or if we use select with output into.

Is there another mechanism with an extension point that controls what gets put into a batch?

@AndriySvyryd
Copy link
Member

I see. We don't currently expose anything that could be of help, #35535 is tracking that. You could replace the SqlServerModificationCommandBatchFactory service with your own implementation, but that is not a supported scenario and will likely break with future releases.

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

2 participants