Fix ExecuteUpdate returning -1 on open connections by setting NOCOUNT OFF

[Abde1rahman1]

• I've read the guidelines for contributing and seen the walkthrough
• I've posted a comment on an issue with a detailed description of how I am planning to contribute and got approval from a member of the team
• The code builds and tests pass locally (also verified by our automated build checks)
• Commit messages follow this format:

Summary of the changes

When ExecuteUpdate is called on an already open connection that had a previous operation (like an Insert with identity) which enabled SET NOCOUNT ON, the subsequent ExecuteUpdate returns -1 instead of the actual rows affected. This PR prepends SET NOCOUNT OFF to the generated SQL to ensure the row count is always returned correctly.

Fixes #37062

• Tests for the changes have been added (for bug fixes / features)
• Code follows the same patterns and style as existing code in this repo

[roji]

I'm not sure how this relates to #35535 - that issue doesn't mention ExecuteUpdate and doesn't seem to be about NOCOUNT specifically, but rather about triggers.

Can you show exactly how this fixes #35535 via a minimal repro? If it's unrelated to #35535, please open a separate issue with a minimal repro that shows what this fixes. On that note, I don't see any test in this PR that specifically shows what it fixes.

[Abde1rahman1]

Sorry, I mentioned the wrong issue

[roji]

@Abde1rahman1 no need to close the PR just because you referenced the wrong issue - you can just edit the PR and reference the correct one.

[Abde1rahman1]

I updated the issue number in PR description

[Copilot]

Pull request overview

This PR addresses SQL Server ExecuteUpdate/ExecuteDelete returning -1 affected rows when an already-open connection has SET NOCOUNT ON enabled from a previous operation, by prepending SET NOCOUNT OFF; to the generated DML SQL.

Changes:

• Prepend SET NOCOUNT OFF; to SQL generated for SQL Server UpdateExpression/DeleteExpression (used by ExecuteUpdate/ExecuteDelete).
• Update many SQL-baseline assertions in SQL Server functional tests to include the new SET NOCOUNT OFF; line.
• Add a new SQL Server functional test intended to validate correct affected-row counts on an open connection.

Reviewed changes

Copilot reviewed 44 out of 44 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs	Prepends SET NOCOUNT OFF; before generated UPDATE/DELETE SQL.
test/EFCore.SqlServer.FunctionalTests/Types/Temporal/SqlServerTimeSpanTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Temporal/SqlServerTimeOnlyTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Temporal/SqlServerDateTimeTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Temporal/SqlServerDateTimeOffsetTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Temporal/SqlServerDateTime2TypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Temporal/SqlServerDateOnlyTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerShortTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerLongTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerIntTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerFloatTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerDoubleTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerDecimalTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Numeric/SqlServerByteTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Miscellaneous/SqlServerStringTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Miscellaneous/SqlServerGuidTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Miscellaneous/SqlServerByteArrayTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Miscellaneous/SqlServerBoolTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryPolygonTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryPointTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryMultiPolygonTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryMultiPointTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryMultiLineStringTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryLineStringTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geometry/SqlServerGeometryCollectionTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyPolygonTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyPointTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyMultiPolygonTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyMultiPointTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyMultiLineStringTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyLineStringTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/Types/Geography/SqlServerGeographyCollectionTypeTest.cs	Updates SQL baselines to expect SET NOCOUNT OFF; before UPDATE.
test/EFCore.SqlServer.FunctionalTests/TableSplittingSqlServerTest.cs	Updates ExecuteUpdate SQL baseline to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/Query/PrecompiledQuerySqlServerTest.cs	Updates terminating ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/Query/Associations/ComplexTableSplitting/ComplexTableSplittingBulkUpdateSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/Query/Associations/ComplexJson/ComplexJsonBulkUpdateSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs	Updates many baselines to include SET NOCOUNT OFF; and adds a new regression test.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NonSharedModelBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/Inheritance/TPTInheritanceBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/Inheritance/TPTFiltersInheritanceBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/Inheritance/TPHInheritanceBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/Inheritance/TPHFiltersInheritanceBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/Inheritance/TPCInheritanceBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.
test/EFCore.SqlServer.FunctionalTests/BulkUpdates/Inheritance/TPCFiltersInheritanceBulkUpdatesSqlServerTest.cs	Updates ExecuteUpdate/Delete SQL baselines to include SET NOCOUNT OFF;.

Comments suppressed due to low confidence (2)

src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs:84

• Prepending SET NOCOUNT OFF changes the session-level NOCOUNT setting for the underlying connection and will remain in effect after ExecuteUpdate/ExecuteDelete completes. Since EF Core also sets SET NOCOUNT ON for SaveChanges batches, this can cause the connection’s NOCOUNT state to flip depending on the last EF operation; consider whether this behavior change is acceptable, or whether the NOCOUNT change should be scoped/mitigated (e.g. alternative way of obtaining rows-affected that doesn't require changing session state).

        Sql.Append("SET NOCOUNT OFF").AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);

test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs:1756

• Method name ends with _Final, which is inconsistent with the surrounding test naming patterns and makes the intent unclear. Please rename to follow the existing convention (describe the scenario/expected behavior without suffixes like _Final).

    public virtual async Task ExecuteUpdate_after_empty_insert_on_open_connection_returns_correct_rows_affected_Final()
    {

---

You can also share your feedback on Copilot code review. Take the survey.

[roji]

@Abde1rahman1 as always please ensure tests are fully running and all Copilot review comments are addressed; at that point, please request a review from me.

[Abde1rahman1]

@roji, I’ve addressed all the Copilot review comments and updated the tests.

Regarding the NOCOUNT session state concern, I have implemented your previous suggestion of prepending SET NOCOUNT OFF and confirmed it's working across the suite.

However, I have an alternative approach if you’d like to avoid altering the connection's session state entirely: we could append SELECT @@ROWCOUNT at the end of the batch instead. This would keep the session state clean and avoid any 'flipping' issues in the connection pool.

Let me know if you’d like me to stick with the current fix or switch to the @@ROWCOUNT alternative

[Copilot]

Pull request overview

Copilot reviewed 44 out of 44 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs:84

• Prepending SET NOCOUNT OFF; changes the session-level NOCOUNT setting for the user-provided connection and it is not restored. This can alter behavior for subsequent commands on the same open connection (outside EF). Consider preserving the prior NOCOUNT state (e.g., via @@OPTIONS) and restoring it after the DML statement, or otherwise scoping the change so it doesn't leak beyond the ExecuteDelete command.

        Sql.Append("SET NOCOUNT OFF").AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);

src/EFCore.SqlServer/Query/Internal/SqlServerQuerySqlGenerator.cs:293

• Prepending SET NOCOUNT OFF; changes the session-level NOCOUNT setting for the user-provided connection and it is not restored. This can alter behavior for subsequent commands on the same open connection (outside EF). Consider preserving the prior NOCOUNT state (e.g., via @@OPTIONS) and restoring it after the DML statement, or otherwise scoping the change so it doesn't leak beyond the ExecuteUpdate command.

        Sql.Append("SET NOCOUNT OFF").AppendLine(Dependencies.SqlGenerationHelper.StatementTerminator);

test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs:1756

• The new test method name ends with _Final, which is inconsistent with the surrounding naming pattern and looks like a temporary suffix. Also, the name mentions an "empty insert" but the test doesn't actually perform an empty entity insert; it manually sets NOCOUNT ON. Consider renaming to reflect the actual scenario being validated (open connection with NOCOUNT ON affecting ExecuteUpdate rows affected).

    [ConditionalFact]
    public virtual async Task ExecuteUpdate_after_empty_insert_on_open_connection_returns_correct_rows_affected_Final()
    {

test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs:1788

• There are blank lines with trailing whitespace (e.g. after customerId) and the next method declaration starts immediately after the closing brace of this method. Please remove trailing whitespace and keep a blank line between method declarations to match the project's formatting conventions.

        var customerId = "FIXED";
        
        await context.Database.ExecuteSqlRawAsync($"DELETE FROM [Orders] WHERE [CustomerID] = '{customerId}'");
        await context.Database.ExecuteSqlRawAsync($"DELETE FROM [Customers] WHERE [CustomerID] = '{customerId}'");
        
        await context.Database.ExecuteSqlRawAsync(
            $"INSERT INTO [Customers] ([CustomerID], [CompanyName], [ContactName]) VALUES ('{customerId}', 'Test Corp', 'Owner')");

        await context.Database.ExecuteSqlRawAsync(
            $"INSERT INTO [Orders] ([CustomerID], [OrderDate]) VALUES ('{customerId}', GETDATE())");

        await context.Database.ExecuteSqlRawAsync("SET NOCOUNT ON;");
        var affected = await context.Customers
            .Where(c => c.CustomerID == customerId)
            .ExecuteUpdateAsync(setters => setters.SetProperty(c => c.City, "Cairo"));
        Assert.Equal(1, affected);
        await context.Database.ExecuteSqlRawAsync("SET NOCOUNT OFF;");

        await context.Database.ExecuteSqlRawAsync($"DELETE FROM [Orders] WHERE [CustomerID] = '{customerId}'");
        await context.Database.ExecuteSqlRawAsync($"DELETE FROM [Customers] WHERE [CustomerID] = '{customerId}'");
    }
    public override async Task Update_with_select_mixed_entity_scalar_anonymous_projection(bool async)

test/EFCore.SqlServer.FunctionalTests/BulkUpdates/NorthwindBulkUpdatesSqlServerTest.cs:1771

• These ExecuteSqlRawAsync calls embed customerId directly into SQL text. Even though this is a test, using parameters (e.g. {0} placeholders or SqlParameter) avoids quoting/escaping pitfalls and keeps the pattern consistent with safe SQL execution elsewhere.

        await context.Database.ExecuteSqlRawAsync($"DELETE FROM [Orders] WHERE [CustomerID] = '{customerId}'");
        await context.Database.ExecuteSqlRawAsync($"DELETE FROM [Customers] WHERE [CustomerID] = '{customerId}'");
        

---

You can also share your feedback on Copilot code review. Take the survey.

[roji]

However, I have an alternative approach if you’d like to avoid altering the connection's session state entirely: we could append SELECT @@rowcount at the end of the batch instead. This would keep the session state clean and avoid any 'flipping' issues in the connection pool.

Can you explain how you think this would be better, or how it compares to the current approach? I'm not quite sure whether SELECT @@ROWCOUNT yields the exact same behavior - IIRC it would create a new resultset that needs to be read, as opposed to the current approach where the rowcount is simply returned by SqlClient when calling ExecuteNonQuery(); that seems like it would be heavier.

[Abde1rahman1]

@roji You're absolutely right. What if we wrap the generated SQL? We can keep SET NOCOUNT OFF at the beginning so the rowcount is reported, but append SET NOCOUNT ON at the very end of the batch. This ensures the connection is returned to the pool in its original state while keeping the execution light, as you suggested.