Fixes to stored procedure update mapping

* Don't attempt to propagate rows affected result columns.
* Warn if an unexpected trailing result set is found.
* Throw an informative message if a result set is missing.

Fixes dotnet#28803

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;