From 9d706b5c5ac58fde1579157d44bfd1753a9d4dc4 Mon Sep 17 00:00:00 2001 From: Shay Rojansky Date: Wed, 3 Aug 2022 13:49:28 +0200 Subject: [PATCH] Address review comments --- .../Storage/RelationalTypeMapping.cs | 9 +++++---- .../AffectedCountModificationCommandBatch.cs | 18 ++++++++++++------ .../Update/IReadOnlyModificationCommand.cs | 8 ++++---- .../Update/ModificationCommand.cs | 4 ++-- .../Internal/SqliteModelValidator.cs | 7 +++---- 5 files changed, 26 insertions(+), 20 deletions(-) diff --git a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs index 56be96e3841..5f90c7dfccc 100644 --- a/src/EFCore.Relational/Storage/RelationalTypeMapping.cs +++ b/src/EFCore.Relational/Storage/RelationalTypeMapping.cs @@ -496,9 +496,7 @@ public virtual DbParameter CreateParameter( parameter.Direction = direction; parameter.ParameterName = name; - var isInputParameter = direction is ParameterDirection.Input or ParameterDirection.InputOutput; - - if (isInputParameter) + if (direction.HasFlag(ParameterDirection.Input)) { value = NormalizeEnumValue(value); @@ -512,7 +510,10 @@ public virtual DbParameter CreateParameter( if (nullable.HasValue) { - Check.DebugAssert(nullable.Value || !isInputParameter || value != null, "Null value in a non-nullable input parameter"); + Check.DebugAssert(nullable.Value + || !direction.HasFlag(ParameterDirection.Input) + || value != null, + "Null value in a non-nullable input parameter"); parameter.IsNullable = nullable.Value; } diff --git a/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs b/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs index 93b6d983247..2469f04356a 100644 --- a/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/AffectedCountModificationCommandBatch.cs @@ -97,7 +97,7 @@ void HandleOutputParameters() { if (requiresResultPropagation) { - command.PropagateOutputParameters(reader); + command.PropagateOutputParameters(reader.DbCommand.Parameters); } // TODO: Rows affected via output parameter @@ -164,10 +164,9 @@ protected override async Task ConsumeAsync( // TODO: Maybe record if there's any sprocs in the batch, and if not, skip this while (true) { - if (command.StoredProcedure is not null) + if (resultSetMapping.HasFlag(ResultSetMapping.HasOutputParameters)) { - command.PropagateOutputParameters(reader); - // TODO: Rows affected via output parameter + HandleOutputParameters(); } if (commandIndex == lastHandledCommandIndex) @@ -175,15 +174,22 @@ protected override async Task ConsumeAsync( break; } - command = ModificationCommands[++commandIndex]; + resultSetMapping = CommandResultSet[++commandIndex]; + command = ModificationCommands[commandIndex]; + requiresResultPropagation = command.RequiresResultPropagation; } } else if (resultSetMapping.HasFlag(ResultSetMapping.HasOutputParameters)) { // Handle output parameters for commands which have no result rows. Commands with result rows are handled above. + HandleOutputParameters(); + } + + void HandleOutputParameters() + { if (requiresResultPropagation) { - command.PropagateOutputParameters(reader); + command.PropagateOutputParameters(reader.DbCommand.Parameters); } // TODO: Rows affected via output parameter diff --git a/src/EFCore.Relational/Update/IReadOnlyModificationCommand.cs b/src/EFCore.Relational/Update/IReadOnlyModificationCommand.cs index 09bdcfd16b4..aac721e5a2f 100644 --- a/src/EFCore.Relational/Update/IReadOnlyModificationCommand.cs +++ b/src/EFCore.Relational/Update/IReadOnlyModificationCommand.cs @@ -69,9 +69,9 @@ public interface IReadOnlyModificationCommand public void PropagateResults(RelationalDataReader relationalReader); /// - /// Reads output parameters returned from the database in the given and propagates them back to - /// into the appropriate from which the values can be propagated on to tracked entities. + /// Reads output parameters returned from the database in the given and propagates them back + /// to into the appropriate from which the values can be propagated on to tracked entities. /// - /// The relational reader containing the values read from the database. - public void PropagateOutputParameters(RelationalDataReader relationalReader); + /// The parameter collection from which to propagate output values. + public void PropagateOutputParameters(DbParameterCollection parameterCollection); } diff --git a/src/EFCore.Relational/Update/ModificationCommand.cs b/src/EFCore.Relational/Update/ModificationCommand.cs index 11eee3f58d5..bc5f773be16 100644 --- a/src/EFCore.Relational/Update/ModificationCommand.cs +++ b/src/EFCore.Relational/Update/ModificationCommand.cs @@ -500,7 +500,7 @@ public virtual void PropagateResults(RelationalDataReader relationalReader) } /// - public virtual void PropagateOutputParameters(RelationalDataReader relationalReader) + public virtual void PropagateOutputParameters(DbParameterCollection parameterCollection) { // Note that this call sets the value into a sidecar and will only commit to the actual entity // if SaveChanges is successful. @@ -528,7 +528,7 @@ public virtual void PropagateOutputParameters(RelationalDataReader relationalRea // Another option is to put the invariant name in the DbParameterCollection (without the prefix); the prefix isn't needed // there AFAIK (but should check this is OK across databases. var prefixedName = "@" + columnModification.ParameterName; - var dbParameter = relationalReader.DbCommand.Parameters[prefixedName]; + var dbParameter = parameterCollection[prefixedName]; columnModification.Value = dbParameter.Value; } } diff --git a/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs b/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs index 8185e670f57..5152b25318c 100644 --- a/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs +++ b/src/EFCore.Sqlite.Core/Infrastructure/Internal/SqliteModelValidator.cs @@ -85,11 +85,10 @@ protected virtual void ValidateNoStoredProcedures( { foreach (var entityType in model.GetEntityTypes()) { - if (entityType.GetInsertStoredProcedureMappings().Any() - || entityType.GetUpdateStoredProcedureMappings().Any() - || entityType.GetDeleteStoredProcedureMappings().Any()) + if (entityType.GetInsertStoredProcedure() is not null + || entityType.GetUpdateStoredProcedure() is not null + || entityType.GetDeleteStoredProcedure() is not null) { - // TODO: Make this throw and not warn (like other validations) since updates won't work throw new InvalidOperationException(SqliteStrings.StoredProceduresNotSupported(entityType.DisplayName())); } }