Sproc fixes

src/EFCore.Relational/Diagnostics/RelationalEventId.cs

@@ -107,7 +107,8 @@ private enum Id
         BatchSmallerThanMinBatchSize,
         BatchExecutorFailedToRollbackToSavepoint,
         BatchExecutorFailedToReleaseSavepoint,
-        OptionalDependentWithAllNullPropertiesWarning
+        OptionalDependentWithAllNullPropertiesWarning,
+        UnexpectedTrailingResultSetWhenSaving,
     }
 
     private static readonly string _connectionPrefix = DbLoggerCategory.Database.Connection.Name + ".";
@@ -1001,4 +1002,16 @@ private static EventId MakeUpdateId(Id id)
     /// </remarks>
     public static readonly EventId OptionalDependentWithAllNullPropertiesWarning
         = MakeUpdateId(Id.OptionalDependentWithAllNullPropertiesWarning);
+
+    /// <summary>
+    ///     An unexpected trailing result set was found when reading the results of a SaveChanges operation; this may indicate that a stored
+    ///     procedure returned a result set without being configured for it in the EF model. Check your stored procedure definitions.
+    /// </summary>
+    /// <remarks>
+    ///     <para>
+    ///         This event is in the <see cref="DbLoggerCategory.Update" /> category.
+    ///     </para>
+    /// </remarks>
+    public static readonly EventId UnexpectedTrailingResultSetWhenSaving =
+        MakeUpdateId(Id.UnexpectedTrailingResultSetWhenSaving);
 }

src/EFCore.Relational/Diagnostics/RelationalLoggerExtensions.cs

@@ -3327,4 +3327,31 @@ private static string ColumnOrderIgnoredWarning(EventDefinitionBase definition,
         var p = (MigrationColumnOperationEventData)payload;
         return d.GenerateMessage((p.ColumnOperation.Table, p.ColumnOperation.Schema).FormatTable(), p.ColumnOperation.Name);
     }
+
+    /// <summary>
+    ///     Logs for the <see cref="RelationalEventId.UnexpectedTrailingResultSetWhenSaving" /> event.
+    /// </summary>
+    /// <param name="diagnostics">The diagnostics logger to use.</param>
+    public static void UnexpectedTrailingResultSetWhenSaving(this IDiagnosticsLogger<DbLoggerCategory.Update> diagnostics)
+    {
+        var definition = RelationalResources.LogUnexpectedTrailingResultSetWhenSaving(diagnostics);
+
+        if (diagnostics.ShouldLog(definition))
+        {
+            definition.Log(diagnostics);
+        }
+
+        if (diagnostics.NeedsEventData(definition, out var diagnosticSourceEnabled, out var simpleLogEnabled))
+        {
+            var eventData = new EventData(
+                definition,
+                static (definition, _) =>
+                {
+                    var d = (EventDefinition)definition;
+                    return d.GenerateMessage();
+                });
+
+            diagnostics.DispatchEventData(definition, eventData, diagnosticSourceEnabled, simpleLogEnabled);
+        }
+    }
 }

src/EFCore.Relational/Diagnostics/RelationalLoggingDefinitions.cs

@@ -663,4 +663,13 @@ public abstract class RelationalLoggingDefinitions : LoggingDefinitions
     /// </summary>
     [EntityFrameworkInternal]
     public EventDefinitionBase? LogExceptionDuringExecuteUpdate;
+
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    [EntityFrameworkInternal]
+    public EventDefinitionBase? LogUnexpectedTrailingResultSetWhenSaving;
 }

src/EFCore.Relational/Properties/RelationalStrings.resx

@@ -809,6 +809,10 @@
     <value>An error occurred using a transaction.</value>
     <comment>Error RelationalEventId.TransactionError</comment>
   </data>
+  <data name="LogUnexpectedTrailingResultSetWhenSaving" xml:space="preserve">
+    <value>An unexpected trailing result set was found when reading the results of a SaveChanges operation; this may indicate that a stored procedure returned a result set without being configured for it in the EF model. Check your stored procedure definitions.</value>
+    <comment>Warning CoreEventId.UnexpectedTrailingResultSetWhenSaving</comment>
+  </data>
   <data name="LogUnnamedIndexAllPropertiesNotToMappedToAnyTable" xml:space="preserve">
     <value>The unnamed index on the entity type '{entityType}' specifies properties {indexProperties}, but none of these properties are mapped to a column in any table. This index will not be created in the database.</value>
     <comment>Warning RelationalEventId.AllIndexPropertiesNotToMappedToAnyTable string string</comment>
@@ -855,6 +859,9 @@
   <data name="MissingParameterValue" xml:space="preserve">
     <value>No value was provided for the required parameter '{parameter}'.</value>
   </data>
+  <data name="MissingResultSetWhenSaving" xml:space="preserve">
+    <value>A result set was was missing when reading the results of a SaveChanges operation; this may indicate that a stored procedure was configured to return results in the EF model, but did not. Check your stored procedure definitions.</value>
+  </data>
   <data name="ModificationCommandBatchAlreadyComplete" xml:space="preserve">
     <value>Cannot add commands to a completed ModificationCommandBatch.</value>
   </data>

src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs

@@ -53,7 +53,7 @@ protected override void Consume(RelationalDataReader reader)
                 {
                     if (onResultSet == false)
                     {
-                        Check.DebugFail("Missing a result set");
+                        throw new InvalidOperationException(RelationalStrings.MissingResultSetWhenSaving);
                     }
 
                     var lastHandledCommandIndex = command.RequiresResultPropagation
@@ -75,12 +75,15 @@ protected override void Consume(RelationalDataReader reader)
                 }
             }
 
-            Debug.Assert(onResultSet != true, "Unexpected result set found at end");
+            if (onResultSet == true)
+            {
+                Dependencies.UpdateLogger.UnexpectedTrailingResultSetWhenSaving();
+            }
+
+            reader.DbDataReader.Close();
 
             if (hasOutputParameters)
             {
-                reader.DbDataReader.Close();
-
                 var parameterCounter = 0;
                 IReadOnlyModificationCommand command;
 
@@ -133,7 +136,7 @@ protected override void Consume(RelationalDataReader reader)
             throw new DbUpdateException(
                 RelationalStrings.UpdateStoreException,
                 ex,
-                ModificationCommands[commandIndex].Entries);
+                ModificationCommands[commandIndex < ModificationCommands.Count ? commandIndex : ModificationCommands.Count - 1].Entries);
         }
     }
 
@@ -168,7 +171,7 @@ protected override async Task ConsumeAsync(
                 {
                     if (onResultSet == false)
                     {
-                        Check.DebugFail("Missing a result set");
+                        throw new InvalidOperationException(RelationalStrings.MissingResultSetWhenSaving);
                     }
 
                     var lastHandledCommandIndex = command.RequiresResultPropagation
@@ -190,12 +193,15 @@ protected override async Task ConsumeAsync(
                 }
             }
 
-            Debug.Assert(onResultSet != true, "Unexpected result set found at end");
+            if (onResultSet == true)
+            {
+                Dependencies.UpdateLogger.UnexpectedTrailingResultSetWhenSaving();
+            }
+
+            await reader.DbDataReader.CloseAsync().ConfigureAwait(false);
 
             if (hasOutputParameters)
             {
-                await reader.DbDataReader.CloseAsync().ConfigureAwait(false);
-
                 var parameterCounter = 0;
                 IReadOnlyModificationCommand command;
 
@@ -249,7 +255,7 @@ await ThrowAggregateUpdateConcurrencyExceptionAsync(
             throw new DbUpdateException(
                 RelationalStrings.UpdateStoreException,
                 ex,
-                ModificationCommands[commandIndex].Entries);
+                ModificationCommands[commandIndex < ModificationCommands.Count ? commandIndex : ModificationCommands.Count - 1].Entries);
         }
     }
 

src/EFCore.Relational/Update/ModificationCommand.cs

@@ -866,6 +866,11 @@ public virtual void PropagateResults(RelationalDataReader relationalReader)
                     break;
 
                 case IStoreStoredProcedureResultColumn resultColumn:
+                    if (ReferenceEquals(RowsAffectedColumn, resultColumn))
+                    {
+                        continue;
+                    }
+
                     // For stored procedure result sets, we need to get the column ordering from metadata.
                     readerIndex = resultColumn.Position;
 #if DEBUG

test/EFCore.Relational.Specification.Tests/TestModels/StoredProcedureUpdateModel/StoredProcedureUpdateContext.cs

@@ -22,6 +22,9 @@ public DbSet<EntityWithAdditionalProperty> WithTwoResultColumns
     public DbSet<EntityWithAdditionalProperty> WithOutputParameterAndResultColumn
         => Set<EntityWithAdditionalProperty>(nameof(WithOutputParameterAndResultColumn));
 
+    public DbSet<EntityWithAdditionalProperty> WithOutputParameterAndRowsAffectedResultColumn
+        => Set<EntityWithAdditionalProperty>(nameof(WithOutputParameterAndRowsAffectedResultColumn));
+
     public DbSet<EntityWithTwoAdditionalProperties> WithOutputParameterAndResultColumnAndResultValue
         => Set<EntityWithTwoAdditionalProperties>(nameof(WithOutputParameterAndResultColumnAndResultValue));
 

test/EFCore.Relational.Specification.Tests/Update/StoredProcedureUpdateFixtureBase.cs

@@ -61,6 +61,21 @@ protected override void OnModelCreating(ModelBuilder modelBuilder, DbContext con
                         .HasResultColumn(w => w.AdditionalProperty));
             });
 
+        modelBuilder.SharedTypeEntity<EntityWithAdditionalProperty>(
+            nameof(StoredProcedureUpdateContext.WithOutputParameterAndRowsAffectedResultColumn),
+            b =>
+            {
+                b.Property(w => w.AdditionalProperty).HasComputedColumnSql("8");
+
+                b.UpdateUsingStoredProcedure(
+                    nameof(StoredProcedureUpdateContext.WithOutputParameterAndRowsAffectedResultColumn) + "_Update",
+                    spb => spb
+                        .HasParameter(w => w.Id)
+                        .HasParameter(w => w.Name)
+                        .HasParameter(w => w.AdditionalProperty, pb => pb.IsOutput())
+                        .HasRowsAffectedResultColumn());
+            });
+
         modelBuilder.SharedTypeEntity<EntityWithAdditionalProperty>(nameof(StoredProcedureUpdateContext.WithTwoOutputParameters))
             .UpdateUsingStoredProcedure(
                 nameof(StoredProcedureUpdateContext.WithTwoOutputParameters) + "_Update", spb => spb

test/EFCore.Relational.Specification.Tests/Update/StoredProcedureUpdateTestBase.cs

@@ -152,6 +152,31 @@ public virtual async Task Update_partial(bool async)
         }
     }
 
+    [ConditionalTheory]
+    [MemberData(nameof(IsAsyncData))]
+    public virtual async Task Update_with_output_parameter_and_rows_affected_result_column(bool async)
+    {
+        await using var context = CreateContext();
+
+        var entity = new EntityWithAdditionalProperty { Name = "Foo" };
+        context.WithOutputParameterAndRowsAffectedResultColumn.Add(entity);
+        await context.SaveChangesAsync();
+
+        entity.Name = "Updated";
+
+        ClearLog();
+
+        await SaveChanges(context, async);
+
+        using (Fixture.TestSqlLoggerFactory.SuspendRecordingEvents())
+        {
+            var actual = await context.WithOutputParameterAndRowsAffectedResultColumn.SingleAsync(w => w.Id == entity.Id);
+
+            Assert.Equal("Updated", actual.Name);
+            Assert.Equal(8, actual.AdditionalProperty);
+        }
+    }
+
     [ConditionalTheory]
     [MemberData(nameof(IsAsyncData))]
     public virtual async Task Delete(bool async)

test/EFCore.SqlServer.FunctionalTests/Update/StoredProcedureUpdateSqlServerTest.cs

@@ -115,6 +115,19 @@ public override async Task Update_partial(bool async)
 EXEC [WithTwoOutputParameters_Update] @p0, @p1, @p2;");
     }
 
+    public override async Task Update_with_output_parameter_and_rows_affected_result_column(bool async)
+    {
+        await base.Update_with_output_parameter_and_rows_affected_result_column(async);
+
+        AssertSql(
+            @"@p0='1'
+@p1='Updated' (Size = 4000)
+@p2=NULL (Nullable = false) (Direction = Output) (DbType = Int32)
+
+SET NOCOUNT ON;
+EXEC [WithOutputParameterAndRowsAffectedResultColumn_Update] @p0, @p1, @p2 OUTPUT;");
+    }
+
     public override async Task Delete(bool async)
     {
         await base.Delete(async);
@@ -384,6 +397,7 @@ public override void CleanData()
 TRUNCATE TABLE [WithOriginalAndCurrentValueOnNonConcurrencyToken];
 TRUNCATE TABLE [WithOutputParameter];
 TRUNCATE TABLE [WithOutputParameterAndResultColumn];
+TRUNCATE TABLE [WithOutputParameterAndRowsAffectedResultColumn];
 TRUNCATE TABLE [WithOutputParameterAndResultColumnAndResultValue];
 TRUNCATE TABLE [WithResultColumn];
 TRUNCATE TABLE [WithTwoResultColumns];
@@ -458,10 +472,19 @@ AS BEGIN
 
 GO
 
+CREATE PROCEDURE WithOutputParameterAndRowsAffectedResultColumn_Update(@Id int, @Name varchar(max), @AdditionalProperty int OUT)
+AS BEGIN
+    UPDATE [WithOutputParameterAndRowsAffectedResultColumn] SET [Name] = @Name, @AdditionalProperty = [AdditionalProperty] WHERE [Id] = @Id;
+    SELECT @@ROWCOUNT;
+END;
+
+GO
+
 CREATE PROCEDURE WithTwoOutputParameters_Update(@Id int, @Name varchar(max), @AdditionalProperty int)
 AS UPDATE [WithTwoOutputParameters] SET [Name] = @Name, [AdditionalProperty] = @AdditionalProperty WHERE [Id] = @id;
 
 GO
+
 CREATE PROCEDURE WithRowsAffectedParameter_Update(@Id int, @Name varchar(max), @RowsAffected int OUT)
 AS BEGIN
     UPDATE [WithRowsAffectedParameter] SET [Name] = @Name WHERE [Id] = @Id;