Efcore: Allow opt-out of rows affected check

[jsacapdev]

A trigger is created on the database to delete an row if it meets some condition.

An entity is added to the table that matches that condition.

The call to SaveChanges() throws.

Is this by design? Is there a workaround? (other than using a third party library)

Exception message:

"Database operation expected to affect 1 row(s) but actually affected 0 row(s). Data may have been modified or deleted since entities were loaded. See http://go.microsoft.com/fwlink/?LinkId=527962 for information on understanding and handling optimistic concurrency exceptions."

Stack trace:

"   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ThrowAggregateUpdateConcurrencyException(Int32 commandIndex, Int32 expectedRowsAffected, Int32 rowsAffected)\r\n   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.ConsumeResultSetWithPropagation(Int32 commandIndex, RelationalDataReader reader)\r\n   at Microsoft.EntityFrameworkCore.Update.AffectedCountModificationCommandBatch.Consume(RelationalDataReader reader)\r\n   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.Execute(IRelationalConnection connection)\r\n   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(Tuple`2 parameters)\r\n   at Microsoft.EntityFrameworkCore.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)\r\n   at Microsoft.EntityFrameworkCore.ExecutionStrategyExtensions.Execute[TState,TResult](IExecutionStrategy strategy, TState state, Func`2 operation)\r\n   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)\r\n   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IReadOnlyList`1 entries)\r\n   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IReadOnlyList`1 entriesToSave)\r\n   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)\r\n   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)\r\n   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()\r\n   at AlrightTriggerConsole.Program.Main(String[] args) in C:\\playground\\AlrightTrigger\\AlrightTriggerConsole\\Program.cs:line 30"

Steps to reproduce

1. Ran the program to create a Blog.

2. Ran the program to create a post.

3. Manually created the trigger.

4. Ran the program again to create a post.

```c#

using System;
using System.Collections.Generic;
using System.Linq;
using Microsoft.EntityFrameworkCore;

namespace AlrightTriggerConsole
{
class Program
{
static void Main(string[] args)
{
try
{
using (var db = new BloggingContext())
{
// if (!db.Blogs.Any())
// {
// db.Blogs.Add(new Blog { Url = "http://blogs.msdn.com/" });
// }

                var blog = db.Blogs.FirstOrDefault();

                if (blog.Posts == null)
                {
                    blog.Posts = new List<Post> { };
                }

                blog.Posts.Add(new Post { Content = "test1", Title = "test1" });

                var count = db.SaveChanges();
            }
        }
        catch (Exception ex)
        {
            Console.WriteLine(ex.Message);
        }
    }
}


public class BloggingContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }
    public DbSet<Post> Posts { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder.UseSqlServer("Server=localhost;Database=BlogDb;User Id=sa;Password=Password12345;");
    }
}

public class Blog
{
    public int BlogId { get; set; }
    public string Url { get; set; }

    public List<Post> Posts { get; set; }
}

public class Post
{
    public int PostId { get; set; }
    public string Title { get; set; }
    public string Content { get; set; }

    public int BlogId { get; set; }
    public Blog Blog { get; set; }
}

}

```T-SQL

CREATE TRIGGER RemoveTestPosts
ON [dbo].[Posts]
FOR INSERT
AS
DELETE FROM [dbo].[Posts] WHERE Content = 'test1'
GO

Further technical details

EF Core version:
Microsoft.AspNetCore.All 2.0.3
Microsoft.EntityFrameworkCore 2.0.1

Database Provider:
Microsoft.EntityFrameworkCore.SqlServer
SQL Server is running in docker

Operating system:
Visual Studio Code

Dotnet:
PS C:\playground\AlrightTrigger\AlrightTriggerConsole> dotnet --info
.NET Command Line Tools (2.0.3)

Product Information:
Version: 2.0.3
Commit SHA-1 hash: 12f0c7efcc

Runtime Environment:
OS Name: Windows
OS Version: 10.0.14393
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\2.0.3\

Microsoft .NET Core Shared Framework Host

Version : 2.0.3
Build : a9190d4a75f4a982ae4b4fa8d1a24526566c69df

[ajcvickers]

@jsacapdev EF Core currently expects the rows affected returned to match what is expected to happen in the database based on the model and conceptual operation being performed. In cases like this where a trigger is being used and the conceptual operation is being changed it would be useful to be able to opt out of this checking. For Adds specifically, see also #7038

[Suchiman]

@jsacapdev does it work if you follow the recommendation to add SET NOCOUNT ON in the trigger?
e.g.

CREATE TRIGGER RemoveTestPosts
ON [dbo].[Posts]
FOR INSERT
AS
SET NOCOUNT ON;
DELETE FROM [dbo].[Posts] WHERE Content = 'test1';
GO

[jsacapdev]

@Suchiman setting nocount on does not work

@ajcvickers would you be open to accepting a PR to get this fixed? I ask as you may need to consider the impact of the change required. If you are open to accepting a PR, we can start to discuss your thoughts on a design moving forward?

[ajcvickers]

@jsacapdev Yes, we would certainly consider a PR for this. I'll get back to you on a potential approach.

[ajcvickers]

We discussed this and came to the following initial conclusions:

• Any API that changes this must both cause the state manager to ignore the rows affected and also update the generated SQL to not collect the information
  
  
  
  • This is because collecting the rows affected can add both complexity and perf hits to the generated SQL and if we're not going to use the results, then this SQL should not be generated at all
  
  
• Issue #7038 is about doing this for just inserts, where we don't "use" rows affected anyway
  
  
  
  • This means the change can be made for inserts without any API surface--just never collect rows affected for inserts and never check it in the state manager
  
  
• For updates and deletes, we need to decide on an API surface. Possibilities:
  
  
  
  • Configured in the model for per-entity type or for the whole model
  
  
  • A context-bound option that disables it for all entities in any model
  
  
  • A flag to SaveChanges that can turn the behavior on and off
  
  
  • Some combination of the above
  
  

As of now, we haven't settled on the API questions, so marking this issue as needs-design.

@jsacapdev If you only need this for inserts, then I would suggest tackling #7038 since this doesn't require API changes. If you need this for updates and deletes as well, then we're going to have to do more design work, and this may take some time. Also, we are surprised that using SET NOCOUNT ON does not work--can you provide any more details on why this doesn't work?

[jsacapdev]

@ajcvickers What I have found is that with both triggers (with and without NOCOUNT) I dont get a PostId returned when either query is generated and executed on the database. So the logic in ConsumeResultSetWithPropagation() that checks for a populated data reader fails and it throws back with the same behaviour.

SET NOCOUNT ON; INSERT INTO [Posts] ([BlogId], [Content], [Title]) VALUES (1, 'test1', 'test1'); SELECT [PostId] FROM [Posts] WHERE @@ROWCOUNT = 1 AND [PostId] = scope_identity();

[ajcvickers]

@jsacapdev Can you explain more about what the scenarios are for using the triggers? (If, for example, the result of doing the insert is that the record doesn't exist after the insert, then this is going to be very hard for EF to reason about. However, it seems likely that the real reason for using the trigger is something different, so we'd like to understand more.

[jsacapdev]

@ajcvickers if i am honest, its not a very general use case, and its just as simple as understanding why once a triggered has been fired do i get an exception throw back. i appreciate your time looking into this and the information you have provided. and i have learnt a little bit more about the product.

[justin1291]

@ajcvickers I am having the same issue with my "UPSERT" trigger. Weirdly enough, my error

An unhandled exception occurred while processing the request.

DbUpdateConcurrencyException: Database operation expected to affect 1 row(s) but actually affected 0 row(s). Data may have been modified or deleted since entities 

Odd note, inserting test data from VS code returns 1 row affected, with the trigger catching and updating it.

Trigger code:

IF EXISTS (SELECT * FROM sysobjects WHERE TYPE = 'TR'
     AND NAME = 'VISITOR_IF_EXISTS_UPDATE_TIME')
     DROP TRIGGER VISITOR_IF_EXISTS_UPDATE_TIME
GO
CREATE TRIGGER VISITOR_IF_EXISTS_UPDATE_TIME
   ON Visitor
   INSTEAD OF INSERT
AS 
BEGIN
IF NOT EXISTS (SELECT V.IP_ADDRESS from Visitor V, INSERTED I
 WHERE V.IP_ADDRESS = I.IP_ADDRESS)
    INSERT INTO VISITOR (IP_ADDRESS, COUNTRY_NAME, REGION_NAME, CITY, LATITUDE, LONGITUDE)
SELECT IP_ADDRESS, COUNTRY_NAME, REGION_NAME, CITY, LATITUDE, LONGITUDE
 FROM INSERTED
ELSE
UPDATE Visitor
    SET Visitor.UPDATE_DATE = (SYSDATETIME()),
    Visitor.UPDATE_COUNT = V.UPDATE_COUNT + 1
  FROM Visitor V, INSERTED I
WHERE V.IP_ADDRESS = I.IP_ADDRESS
END

[ajcvickers]

@AndriySvyryd @divega Should we document what the update pipeline expects back for the various cases? For example, if EF is doing an insert in batch mode and there are store-generated values coming back.

[saluce65]

The issue I'm having with INSTEAD OF INSERT triggers is that Entity Framework is batching the insert statement with a select statement using scope_identity() ... since the INSTEAD OF trigger has its own scope, the select statement fails.

Perhaps an option to have inserts use @@IDENTITY instead of scope_identity() would allow users to use INSTEAD OF triggers without resorting to returning result sets, which is deprecated in SQL Server per https://docs.microsoft.com/en-us/sql/t-sql/statements/create-trigger-transact-sql?view=sql-server-2017#returning-results. That option could be added to the SqlServerDbContextOptionsBuilder class.

[PetrMotlicek]

The issue I'm having with INSTEAD OF INSERT triggers is that Entity Framework is batching the insert statement with a select statement using scope_identity() ... since the INSTEAD OF trigger has its own scope, the select statement fails.

Perhaps an option to have inserts use @@IDENTITY instead of scope_identity() would allow users to use INSTEAD OF triggers without resorting to returning result sets, which is deprecated in SQL Server per https://docs.microsoft.com/en-us/sql/t-sql/statements/create-trigger-transact-sql?view=sql-server-2017#returning-results. That option could be added to the SqlServerDbContextOptionsBuilder class.

I have INSTEAD OF INSERT trigger too - just collecting some third-party ad-hoc data and in the instead-of-trigger having logic avoiding duplications via unique key doing actually a data merge - insert or update.

In fact there is no transactional concurrency - third-party data are collected just-in-time - storing only needed ones, not PRE-IMPORTING all the third-party entities before running an application.

I thought using ALTERNATE (composed) KEY would be the way, but not: there is still

WHERE @@ROWCOUNT = 1 AND [Id] = scope_identity();

@@ROWCOUNT is 1, but scope_identity is NULL.

May be using alternate key in the way:

WHERE @@ROWCOUNT = 1 AND ([Id] = scope_identity() OR (ALTERNATE_KEY_COLUMN1 = xyz AND ALTERNATE_KEY_COLUMN2 = 123456789))

could be easy to implement?