diff --git a/src/EFCore.Relational/Storage/IRelationalCommandBuilder.cs b/src/EFCore.Relational/Storage/IRelationalCommandBuilder.cs index 5885ed35511..7a3da9c6924 100644 --- a/src/EFCore.Relational/Storage/IRelationalCommandBuilder.cs +++ b/src/EFCore.Relational/Storage/IRelationalCommandBuilder.cs @@ -30,6 +30,13 @@ public interface IRelationalCommandBuilder /// The same builder instance so that multiple calls can be chained. IRelationalCommandBuilder AddParameter(IRelationalParameter parameter); + /// + /// Removes the parameter with the given index from this command. + /// + /// The index of the parameter to be removed. + /// The same builder instance so that multiple calls can be chained. + IRelationalCommandBuilder RemoveParameterAt(int index); + /// /// The source for s to use. /// diff --git a/src/EFCore.Relational/Storage/RelationalCommandBuilder.cs b/src/EFCore.Relational/Storage/RelationalCommandBuilder.cs index 7f0f3084359..486184be4e5 100644 --- a/src/EFCore.Relational/Storage/RelationalCommandBuilder.cs +++ b/src/EFCore.Relational/Storage/RelationalCommandBuilder.cs @@ -3,19 +3,7 @@ namespace Microsoft.EntityFrameworkCore.Storage; -/// -/// -/// Builds a command to be executed against a relational database. -/// -/// -/// This type is typically used by database providers (and other extensions). It is generally -/// not used in application code. -/// -/// -/// -/// See Implementation of database providers and extensions -/// for more information and examples. -/// +/// public class RelationalCommandBuilder : IRelationalCommandBuilder { private readonly List _parameters = new(); @@ -42,17 +30,12 @@ public RelationalCommandBuilder( /// protected virtual RelationalCommandBuilderDependencies Dependencies { get; } - /// - /// The source for s to use. - /// + /// [Obsolete("Code trying to add parameter should add type mapped parameter using TypeMappingSource directly.")] public virtual IRelationalTypeMappingSource TypeMappingSource => Dependencies.TypeMappingSource; - /// - /// Creates the command. - /// - /// The newly created command. + /// public virtual IRelationalCommand Build() => new RelationalCommand(Dependencies, _commandTextBuilder.ToString(), Parameters); @@ -62,17 +45,11 @@ public virtual IRelationalCommand Build() public override string ToString() => _commandTextBuilder.ToString(); - /// - /// The collection of parameters. - /// + /// public virtual IReadOnlyList Parameters => _parameters; - /// - /// Adds the given parameter to this command. - /// - /// The parameter. - /// The same builder instance so that multiple calls can be chained. + /// public virtual IRelationalCommandBuilder AddParameter(IRelationalParameter parameter) { _parameters.Add(parameter); @@ -80,11 +57,15 @@ public virtual IRelationalCommandBuilder AddParameter(IRelationalParameter param return this; } - /// - /// Appends an object to the command text. - /// - /// The object to be written. - /// The same builder instance so that multiple calls can be chained. + /// + public virtual IRelationalCommandBuilder RemoveParameterAt(int index) + { + _parameters.RemoveAt(index); + + return this; + } + + /// public virtual IRelationalCommandBuilder Append(string value) { _commandTextBuilder.Append(value); @@ -92,10 +73,7 @@ public virtual IRelationalCommandBuilder Append(string value) return this; } - /// - /// Appends a blank line to the command text. - /// - /// The same builder instance so that multiple calls can be chained. + /// public virtual IRelationalCommandBuilder AppendLine() { _commandTextBuilder.AppendLine(); @@ -103,10 +81,7 @@ public virtual IRelationalCommandBuilder AppendLine() return this; } - /// - /// Increments the indent of subsequent lines. - /// - /// The same builder instance so that multiple calls can be chained. + /// public virtual IRelationalCommandBuilder IncrementIndent() { _commandTextBuilder.IncrementIndent(); @@ -114,10 +89,7 @@ public virtual IRelationalCommandBuilder IncrementIndent() return this; } - /// - /// Decrements the indent of subsequent lines. - /// - /// The same builder instance so that multiple calls can be chained. + /// public virtual IRelationalCommandBuilder DecrementIndent() { _commandTextBuilder.DecrementIndent(); @@ -125,9 +97,7 @@ public virtual IRelationalCommandBuilder DecrementIndent() return this; } - /// - /// Gets the length of the command text. - /// + /// public virtual int CommandTextLength => _commandTextBuilder.Length; } diff --git a/src/EFCore.Relational/Update/ColumnModification.cs b/src/EFCore.Relational/Update/ColumnModification.cs index c9da2313950..e6fec449e0f 100644 --- a/src/EFCore.Relational/Update/ColumnModification.cs +++ b/src/EFCore.Relational/Update/ColumnModification.cs @@ -55,100 +55,64 @@ public ColumnModification(in ColumnModificationParameters columnModificationPara UseParameter = _generateParameterName != null; } - /// - /// The that represents the entity that is being modified. - /// + /// public virtual IUpdateEntry? Entry { get; } - /// - /// The property that maps to the column. - /// + /// public virtual IProperty? Property { get; } - /// - /// The relational type mapping for the column. - /// + /// public virtual RelationalTypeMapping? TypeMapping { get; } - /// - /// A value indicating whether the column could contain a null value. - /// + /// public virtual bool? IsNullable { get; } - /// - /// Indicates whether a value must be read from the database for the column. - /// + /// public virtual bool IsRead { get; } - /// - /// Indicates whether a value must be written to the database for the column. - /// + /// public virtual bool IsWrite { get; } - /// - /// Indicates whether the column is used in the WHERE clause when updating. - /// + /// public virtual bool IsCondition { get; } - /// - /// Indicates whether the column is part of a primary or alternate key. - /// + /// public virtual bool IsKey { get; } - /// - /// Indicates whether the original value of the property must be passed as a parameter to the SQL. - /// + /// public virtual bool UseOriginalValueParameter => UseParameter && UseOriginalValue; - /// - /// Indicates whether the current value of the property must be passed as a parameter to the SQL. - /// + /// public virtual bool UseCurrentValueParameter => UseParameter && UseCurrentValue; - /// - /// Indicates whether the original value of the property should be used. - /// + /// public virtual bool UseOriginalValue => IsCondition; - /// - /// Indicates whether the current value of the property should be used. - /// + /// public virtual bool UseCurrentValue => IsWrite; - /// - /// Indicates whether the value of the property must be passed as a parameter to the SQL as opposed to being inlined. - /// + /// public virtual bool UseParameter { get; } - /// - /// The parameter name to use for the current value parameter (), if needed. - /// + /// public virtual string? ParameterName => _parameterName ??= UseCurrentValueParameter ? _generateParameterName!() : null; - /// - /// The parameter name to use for the original value parameter (), if needed. - /// + /// public virtual string? OriginalParameterName => _originalParameterName ??= UseOriginalValueParameter ? _generateParameterName!() : null; - /// - /// The name of the column. - /// + /// public virtual string ColumnName { get; } - /// - /// The database type of the column. - /// + /// public virtual string? ColumnType { get; } - /// - /// The original value of the property mapped to this column. - /// + /// public virtual object? OriginalValue => Entry == null ? _originalValue @@ -156,9 +120,7 @@ public virtual object? OriginalValue ? Entry.GetOriginalValue(Property!) : Entry.SharedIdentityEntry.GetOriginalValue(Property!); - /// - /// Gets or sets the current value of the property mapped to this column. - /// + /// public virtual object? Value { get => Entry == null @@ -186,10 +148,7 @@ public virtual object? Value } } - /// - /// Adds a modification affecting the same database value. - /// - /// The modification for the shared column. + /// public virtual void AddSharedColumnModification(IColumnModification modification) { Check.DebugAssert(Entry is not null, "Entry is not null"); @@ -258,4 +217,8 @@ public virtual void AddSharedColumnModification(IColumnModification modification _sharedColumnModifications.Add(modification); } + + /// + public virtual void ResetParameterNames() + => _parameterName = _originalParameterName = null; } diff --git a/src/EFCore.Relational/Update/IColumnModification.cs b/src/EFCore.Relational/Update/IColumnModification.cs index 0cee072bc33..41b5e9380ce 100644 --- a/src/EFCore.Relational/Update/IColumnModification.cs +++ b/src/EFCore.Relational/Update/IColumnModification.cs @@ -122,4 +122,9 @@ public interface IColumnModification /// /// The modification for the shared column. public void AddSharedColumnModification(IColumnModification modification); + + /// + /// Resets parameter names, so they can be regenerated if the command needs to be re-added to a new batch. + /// + public void ResetParameterNames(); } diff --git a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs index 7c5457b00fd..28e73094e27 100644 --- a/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs +++ b/src/EFCore.Relational/Update/Internal/CommandBatchPreparer.cs @@ -74,7 +74,7 @@ public CommandBatchPreparer(CommandBatchPreparerDependencies dependencies) continue; } - if (!batch.AddCommand(modificationCommand)) + if (!batch.TryAddCommand(modificationCommand)) { if (batch.ModificationCommands.Count == 1 || batch.ModificationCommands.Count >= _minBatchSize) @@ -144,7 +144,7 @@ private ModificationCommandBatch StartNewBatch( { parameterNameGenerator.Reset(); var batch = Dependencies.ModificationCommandBatchFactory.Create(); - batch.AddCommand(modificationCommand); + batch.TryAddCommand(modificationCommand); return batch; } diff --git a/src/EFCore.Relational/Update/ModificationCommandBatch.cs b/src/EFCore.Relational/Update/ModificationCommandBatch.cs index 152e665be45..3d97110eb52 100644 --- a/src/EFCore.Relational/Update/ModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/ModificationCommandBatch.cs @@ -24,14 +24,14 @@ public abstract class ModificationCommandBatch public abstract IReadOnlyList ModificationCommands { get; } /// - /// Adds the given insert/update/delete to the batch. + /// Attempts to adds the given insert/update/delete to the batch. /// /// The command to add. /// /// if the command was successfully added; if there was no /// room in the current batch to add the command and it must instead be added to a new batch. /// - public abstract bool AddCommand(IReadOnlyModificationCommand modificationCommand); + public abstract bool TryAddCommand(IReadOnlyModificationCommand modificationCommand); /// /// Indicates that no more commands will be added to this batch, and prepares it for execution. diff --git a/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs b/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs index 7ada76329fe..641d886d0ee 100644 --- a/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/ReaderModificationCommandBatch.cs @@ -22,8 +22,10 @@ namespace Microsoft.EntityFrameworkCore.Update; public abstract class ReaderModificationCommandBatch : ModificationCommandBatch { private readonly List _modificationCommands = new(); - private string? _finalCommandText; + private readonly int _batchHeaderLength; + private readonly List _pendingParameterNames = new(); private bool _requiresTransaction = true; + private int _sqlBuilderPosition, _commandResultSetCount, _resultsPositionalMappingEnabledLength; /// /// Creates a new instance. @@ -32,7 +34,12 @@ public abstract class ReaderModificationCommandBatch : ModificationCommandBatch protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies) { Dependencies = dependencies; - CachedCommandText = new StringBuilder(); + + RelationalCommandBuilder = dependencies.CommandBuilderFactory.Create(); + + UpdateSqlGenerator = dependencies.UpdateSqlGenerator; + UpdateSqlGenerator.AppendBatchHeader(SqlBuilder); + _batchHeaderLength = SqlBuilder.Length; } /// @@ -43,18 +50,22 @@ protected ReaderModificationCommandBatch(ModificationCommandBatchFactoryDependen /// /// The update SQL generator. /// - protected virtual IUpdateSqlGenerator UpdateSqlGenerator - => Dependencies.UpdateSqlGenerator; + protected virtual IUpdateSqlGenerator UpdateSqlGenerator { get; } + + /// + /// Gets the relational command builder for the commands in the batch. + /// + protected virtual IRelationalCommandBuilder RelationalCommandBuilder { get; } /// - /// Gets or sets the cached command text for the commands in the batch. + /// Gets the command text builder for the commands in the batch. /// - protected virtual StringBuilder CachedCommandText { get; set; } + protected virtual StringBuilder SqlBuilder { get; } = new(); /// - /// The ordinal of the last command for which command text was built. + /// Gets the parameter values for the commands in the batch. /// - protected virtual int LastCachedCommandIndex { get; set; } + protected virtual Dictionary ParameterValues { get; } = new(); /// /// The list of conceptual insert/update/delete s in the batch. @@ -75,66 +86,77 @@ public override IReadOnlyList ModificationCommands protected virtual BitArray? ResultsPositionalMappingEnabled { get; set; } /// - /// Adds the given insert/update/delete to the batch. + /// The store command generated from this batch when is called. /// - /// The command to add. - /// - /// if the command was successfully added; if there was no - /// room in the current batch to add the command and it must instead be added to a new batch. - /// - public override bool AddCommand(IReadOnlyModificationCommand modificationCommand) + protected virtual RawSqlCommand? StoreCommand { get; set; } + + /// + public override bool TryAddCommand(IReadOnlyModificationCommand modificationCommand) { - if (_finalCommandText is not null) + if (StoreCommand is not null) { throw new InvalidOperationException(RelationalStrings.ModificationCommandBatchAlreadyComplete); } - if (ModificationCommands.Count == 0) - { - ResetCommandText(); - } + _sqlBuilderPosition = SqlBuilder.Length; + _commandResultSetCount = CommandResultSet.Count; + _pendingParameterNames.Clear(); + _resultsPositionalMappingEnabledLength = ResultsPositionalMappingEnabled?.Length ?? 0; + + AddCommand(modificationCommand); + _modificationCommands.Add(modificationCommand); - if (!CanAddCommand(modificationCommand)) + // Check if the batch is still valid after having added the command (e.g. have we bypassed a maximum CommandText size?) + // A batch with only one command is always considered valid (otherwise we'd get an endless loop); allow the batch to fail + // server-side. + if (IsValid() || _modificationCommands.Count == 1) { - return false; + return true; } - _modificationCommands.Add(modificationCommand); - CommandResultSet.Add(ResultSetMapping.LastInResultSet); + RollbackLastCommand(); - if (!IsCommandTextValid()) + // The command's column modifications had their parameter names generated, that needs to be rolled back as well. + foreach (var columnModification in modificationCommand.ColumnModifications) { - ResetCommandText(); - _modificationCommands.RemoveAt(_modificationCommands.Count - 1); - CommandResultSet.RemoveAt(CommandResultSet.Count - 1); - return false; + columnModification.ResetParameterNames(); } - return true; + return false; } /// - /// Resets the builder to start building a new batch. + /// Rolls back the last command added. Used when adding a command caused the batch to become invalid (e.g. CommandText too long). /// - protected virtual void ResetCommandText() + protected virtual void RollbackLastCommand() { - CachedCommandText.Clear(); + _modificationCommands.RemoveAt(_modificationCommands.Count - 1); + + SqlBuilder.Length = _sqlBuilderPosition; - UpdateSqlGenerator.AppendBatchHeader(CachedCommandText); - _batchHeaderLength = CachedCommandText.Length; + while (CommandResultSet.Count > _commandResultSetCount) + { + CommandResultSet.RemoveAt(CommandResultSet.Count - 1); + } - SetRequiresTransaction(true); + if (ResultsPositionalMappingEnabled is not null) + { + ResultsPositionalMappingEnabled.Length = _resultsPositionalMappingEnabledLength; + } - LastCachedCommandIndex = -1; - } + foreach (var pendingParameterName in _pendingParameterNames) + { + ParameterValues.Remove(pendingParameterName); - private int _batchHeaderLength; + RelationalCommandBuilder.RemoveParameterAt(RelationalCommandBuilder.Parameters.Count - 1); + } + } /// /// Whether any SQL has already been added to the batch command text. /// - protected virtual bool IsCachedCommandTextEmpty - => CachedCommandText.Length == _batchHeaderLength; + protected virtual bool IsCommandTextEmpty + => SqlBuilder.Length == _batchHeaderLength; /// public override bool RequiresTransaction @@ -147,164 +169,136 @@ public override bool RequiresTransaction protected virtual void SetRequiresTransaction(bool requiresTransaction) => _requiresTransaction = requiresTransaction; - /// - /// Checks whether a new command can be added to the batch. - /// - /// The command to potentially add. - /// if the command can be added; otherwise. - protected abstract bool CanAddCommand(IReadOnlyModificationCommand modificationCommand); - /// /// Checks whether the command text is valid. /// /// if the command text is valid; otherwise. - protected abstract bool IsCommandTextValid(); - - /// - /// Processes all unprocessed commands in the batch, making sure their corresponding SQL is populated in - /// . - /// - protected virtual void UpdateCachedCommandText() - { - for (var i = LastCachedCommandIndex + 1; i < ModificationCommands.Count; i++) - { - UpdateCachedCommandText(i); - } - } + protected abstract bool IsValid(); /// - /// Updates the command text for the command at the given position in the - /// list. + /// Adds Updates the command text for the command at the given position in the list. /// - /// The position of the command to generate command text for. - protected virtual void UpdateCachedCommandText(int commandPosition) + /// The command to add. + protected virtual void AddCommand(IReadOnlyModificationCommand modificationCommand) { - var newModificationCommand = ModificationCommands[commandPosition]; - bool requiresTransaction; - switch (newModificationCommand.EntityState) + var commandPosition = CommandResultSet.Count; + + switch (modificationCommand.EntityState) { case EntityState.Added: - CommandResultSet[commandPosition] = + CommandResultSet.Add( UpdateSqlGenerator.AppendInsertOperation( - CachedCommandText, newModificationCommand, commandPosition, out requiresTransaction); + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; case EntityState.Modified: - CommandResultSet[commandPosition] = + CommandResultSet.Add( UpdateSqlGenerator.AppendUpdateOperation( - CachedCommandText, newModificationCommand, commandPosition, out requiresTransaction); + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; case EntityState.Deleted: - CommandResultSet[commandPosition] = + CommandResultSet.Add( UpdateSqlGenerator.AppendDeleteOperation( - CachedCommandText, newModificationCommand, commandPosition, out requiresTransaction); + SqlBuilder, modificationCommand, commandPosition, out requiresTransaction)); break; default: throw new InvalidOperationException( RelationalStrings.ModificationCommandInvalidEntityState( - newModificationCommand.Entries[0].EntityType, - newModificationCommand.EntityState)); + modificationCommand.Entries[0].EntityType, + modificationCommand.EntityState)); } - _requiresTransaction = commandPosition > 0 || requiresTransaction; + AddParameters(modificationCommand); - LastCachedCommandIndex = commandPosition; + _requiresTransaction = commandPosition > 0 || requiresTransaction; } - /// - /// Gets the total number of parameters needed for the batch. - /// - /// The total parameter count. - protected virtual int GetParameterCount() - => ModificationCommands.Sum(c => c.ColumnModifications.Count); - /// public override void Complete() { - UpdateCachedCommandText(); + if (StoreCommand is not null) + { + throw new InvalidOperationException(RelationalStrings.ModificationCommandBatchAlreadyComplete); + } // Some database have a mode where autocommit is off, and so executing a command outside of an explicit transaction implicitly // creates a new transaction (which needs to be explicitly committed). // The below is a hook for allowing providers to turn autocommit on, in case it's off. if (!RequiresTransaction) { - UpdateSqlGenerator.PrependEnsureAutocommit(CachedCommandText); + UpdateSqlGenerator.PrependEnsureAutocommit(SqlBuilder); } - _finalCommandText = CachedCommandText.ToString(); + RelationalCommandBuilder.Append(SqlBuilder.ToString()); + + StoreCommand = new RawSqlCommand(RelationalCommandBuilder.Build(), ParameterValues); } /// - /// Generates a for the batch. + /// Adds parameters for all column modifications in the given to the relational command + /// being built for this batch. /// - /// The command. - protected virtual RawSqlCommand CreateStoreCommand() + /// The modification command for which to add parameters. + protected virtual void AddParameters(IReadOnlyModificationCommand modificationCommand) { - Check.DebugAssert(_finalCommandText is not null, "_finalCommandText is not null, checked in Execute"); + foreach (var columnModification in modificationCommand.ColumnModifications) + { + AddParameter(columnModification); + } + } - var commandBuilder = Dependencies.CommandBuilderFactory - .Create() - .Append(_finalCommandText); + /// + /// Adds a parameter for the given to the relational command being built for this batch. + /// + /// The column modification for which to add parameters. + protected virtual void AddParameter(IColumnModification columnModification) + { + if (columnModification.UseCurrentValueParameter) + { + RelationalCommandBuilder.AddParameter( + columnModification.ParameterName, + Dependencies.SqlGenerationHelper.GenerateParameterName(columnModification.ParameterName), + columnModification.TypeMapping!, + columnModification.IsNullable); - var parameterValues = new Dictionary(GetParameterCount()); + ParameterValues.Add(columnModification.ParameterName, columnModification.Value); - // ReSharper disable once ForCanBeConvertedToForeach - for (var commandIndex = 0; commandIndex < ModificationCommands.Count; commandIndex++) - { - var command = ModificationCommands[commandIndex]; - // ReSharper disable once ForCanBeConvertedToForeach - for (var columnIndex = 0; columnIndex < command.ColumnModifications.Count; columnIndex++) - { - var columnModification = command.ColumnModifications[columnIndex]; - if (columnModification.UseCurrentValueParameter) - { - commandBuilder.AddParameter( - columnModification.ParameterName, - Dependencies.SqlGenerationHelper.GenerateParameterName(columnModification.ParameterName), - columnModification.TypeMapping!, - columnModification.IsNullable); - - parameterValues.Add(columnModification.ParameterName, columnModification.Value); - } - - if (columnModification.UseOriginalValueParameter) - { - commandBuilder.AddParameter( - columnModification.OriginalParameterName, - Dependencies.SqlGenerationHelper.GenerateParameterName(columnModification.OriginalParameterName), - columnModification.TypeMapping!, - columnModification.IsNullable); - - parameterValues.Add(columnModification.OriginalParameterName, columnModification.OriginalValue); - } - } + _pendingParameterNames.Add(columnModification.ParameterName); } - return new RawSqlCommand(commandBuilder.Build(), parameterValues); + if (columnModification.UseOriginalValueParameter) + { + RelationalCommandBuilder.AddParameter( + columnModification.OriginalParameterName, + Dependencies.SqlGenerationHelper.GenerateParameterName(columnModification.OriginalParameterName), + columnModification.TypeMapping!, + columnModification.IsNullable); + + ParameterValues.Add(columnModification.OriginalParameterName, columnModification.OriginalValue); + + _pendingParameterNames.Add(columnModification.OriginalParameterName); + } } /// - /// Executes the command generated by against a - /// database using the given connection. + /// Executes the command generated by this batch against a database using the given connection. /// /// The connection to the database to update. public override void Execute(IRelationalConnection connection) { - if (_finalCommandText is null) + if (StoreCommand is null) { throw new InvalidOperationException(RelationalStrings.ModificationCommandBatchNotComplete); } - var storeCommand = CreateStoreCommand(); - try { - using var dataReader = storeCommand.RelationalCommand.ExecuteReader( + using var dataReader = StoreCommand.RelationalCommand.ExecuteReader( new RelationalCommandParameterObject( connection, - storeCommand.ParameterValues, + StoreCommand.ParameterValues, null, Dependencies.CurrentContext.Context, Dependencies.Logger, CommandSource.SaveChanges)); @@ -320,8 +314,7 @@ public override void Execute(IRelationalConnection connection) } /// - /// Executes the command generated by against a - /// database using the given connection. + /// Executes the command generated by this batch against a database using the given connection. /// /// The connection to the database to update. /// A to observe while waiting for the task to complete. @@ -331,19 +324,17 @@ public override async Task ExecuteAsync( IRelationalConnection connection, CancellationToken cancellationToken = default) { - if (_finalCommandText is null) + if (StoreCommand is null) { throw new InvalidOperationException(RelationalStrings.ModificationCommandBatchNotComplete); } - var storeCommand = CreateStoreCommand(); - try { - var dataReader = await storeCommand.RelationalCommand.ExecuteReaderAsync( + var dataReader = await StoreCommand.RelationalCommand.ExecuteReaderAsync( new RelationalCommandParameterObject( connection, - storeCommand.ParameterValues, + StoreCommand.ParameterValues, null, Dependencies.CurrentContext.Context, Dependencies.Logger, CommandSource.SaveChanges), diff --git a/src/EFCore.Relational/Update/SingularModificationCommandBatch.cs b/src/EFCore.Relational/Update/SingularModificationCommandBatch.cs index 9af3e5e3704..e9763f2ab94 100644 --- a/src/EFCore.Relational/Update/SingularModificationCommandBatch.cs +++ b/src/EFCore.Relational/Update/SingularModificationCommandBatch.cs @@ -28,19 +28,11 @@ public SingularModificationCommandBatch(ModificationCommandBatchFactoryDependenc } /// - /// Only returns if the no command has already been added. - /// - /// The command to potentially add. - /// if no command has already been added. - protected override bool CanAddCommand(IReadOnlyModificationCommand modificationCommand) - => ModificationCommands.Count == 0; - - /// - /// Returns since only a single command is generated so the command text must be valid. + /// Returns only when the batch contains a single command. /// /// - /// + /// when the batch contains a single command, otherwise. /// - protected override bool IsCommandTextValid() - => true; + protected override bool IsValid() + => ModificationCommands.Count == 1; } diff --git a/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs b/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs index 2d2429f88c6..336e318c894 100644 --- a/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs +++ b/src/EFCore.SqlServer/Update/Internal/SqlServerModificationCommandBatch.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections; -using System.Text; using Microsoft.Data.SqlClient; using Microsoft.EntityFrameworkCore.SqlServer.Internal; @@ -20,10 +19,8 @@ public class SqlServerModificationCommandBatch : AffectedCountModificationComman private const int MaxScriptLength = 65536 * DefaultNetworkPacketSizeBytes / 2; private const int MaxParameterCount = 2100; private const int MaxRowCount = 1000; - private int _parameterCount = 1; // Implicit parameter for the command text private readonly int _maxBatchSize; - private readonly List _bulkInsertCommands = new(); - private int _commandsLeftToLengthCheck = 50; + private readonly List _pendingBulkInsertCommands = new(); /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -36,8 +33,7 @@ public SqlServerModificationCommandBatch( int? maxBatchSize) : base(dependencies) { - if (maxBatchSize.HasValue - && maxBatchSize.Value <= 0) + if (maxBatchSize is <= 0) { throw new ArgumentOutOfRangeException(nameof(maxBatchSize), RelationalStrings.InvalidMaxBatchSize(maxBatchSize.Value)); } @@ -60,77 +56,15 @@ public SqlServerModificationCommandBatch( /// 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. /// - protected override bool CanAddCommand(IReadOnlyModificationCommand modificationCommand) + protected override void RollbackLastCommand() { - if (ModificationCommands.Count >= _maxBatchSize) + if (_pendingBulkInsertCommands.Count > 0) { - return false; - } - - var additionalParameterCount = CountParameters(modificationCommand); - - if (_parameterCount + additionalParameterCount >= MaxParameterCount) - { - return false; - } - - _parameterCount += additionalParameterCount; - return true; - } - - /// - /// 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. - /// - protected override bool IsCommandTextValid() - { - if (--_commandsLeftToLengthCheck < 0) - { - UpdateCachedCommandText(); - var commandTextLength = CachedCommandText.Length; - if (commandTextLength >= MaxScriptLength) - { - return false; - } - - var averageCommandLength = commandTextLength / ModificationCommands.Count; - var expectedAdditionalCommandCapacity = (MaxScriptLength - commandTextLength) / averageCommandLength; - _commandsLeftToLengthCheck = Math.Max(1, expectedAdditionalCommandCapacity / 4); - } - - return true; - } - - /// - /// 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. - /// - protected override int GetParameterCount() - => _parameterCount; - - private static int CountParameters(IReadOnlyModificationCommand modificationCommand) - { - var parameterCount = 0; - // ReSharper disable once ForCanBeConvertedToForeach - for (var columnIndex = 0; columnIndex < modificationCommand.ColumnModifications.Count; columnIndex++) - { - var columnModification = modificationCommand.ColumnModifications[columnIndex]; - if (columnModification.UseCurrentValueParameter) - { - parameterCount++; - } - - if (columnModification.UseOriginalValueParameter) - { - parameterCount++; - } + _pendingBulkInsertCommands.RemoveAt(_pendingBulkInsertCommands.Count - 1); + return; } - return parameterCount; + base.RollbackLastCommand(); } /// @@ -139,24 +73,25 @@ private static int CountParameters(IReadOnlyModificationCommand modificationComm /// 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. /// - protected override void ResetCommandText() - { - base.ResetCommandText(); - - _bulkInsertCommands.Clear(); - } + protected override bool IsValid() + => ModificationCommands.Count <= _maxBatchSize + && SqlBuilder.Length < MaxScriptLength + // A single implicit parameter for the command text itself + && ParameterValues.Count + 1 < MaxParameterCount; - private void AppendBulkInsertCommandText(int lastIndex) + private void ApplyPendingBulkInsertCommands() { - if (_bulkInsertCommands.Count == 0) + if (_pendingBulkInsertCommands.Count == 0) { return; } - var wasCachedCommandTextEmpty = IsCachedCommandTextEmpty; + var commandPosition = CommandResultSet.Count; + + var wasCachedCommandTextEmpty = IsCommandTextEmpty; var resultSetMapping = UpdateSqlGenerator.AppendBulkInsertOperation( - CachedCommandText, _bulkInsertCommands, lastIndex - _bulkInsertCommands.Count, out var resultsContainPositionMapping, + SqlBuilder, _pendingBulkInsertCommands, commandPosition, out var resultsContainPositionMapping, out var requiresTransaction); SetRequiresTransaction(!wasCachedCommandTextEmpty || requiresTransaction); @@ -165,27 +100,29 @@ private void AppendBulkInsertCommandText(int lastIndex) { if (ResultsPositionalMappingEnabled is null) { - ResultsPositionalMappingEnabled = new BitArray(CommandResultSet.Count); + ResultsPositionalMappingEnabled = new BitArray(CommandResultSet.Count + _pendingBulkInsertCommands.Count); } else { - ResultsPositionalMappingEnabled.Length = CommandResultSet.Count; + ResultsPositionalMappingEnabled.Length = CommandResultSet.Count + _pendingBulkInsertCommands.Count; } - for (var i = lastIndex - _bulkInsertCommands.Count; i < lastIndex; i++) + for (var i = commandPosition; i < commandPosition + _pendingBulkInsertCommands.Count; i++) { ResultsPositionalMappingEnabled![i] = true; } } - for (var i = lastIndex - _bulkInsertCommands.Count; i < lastIndex; i++) + foreach (var pendingCommand in _pendingBulkInsertCommands) { - CommandResultSet[i] = resultSetMapping; + AddParameters(pendingCommand); + + CommandResultSet.Add(resultSetMapping); } if (resultSetMapping != ResultSetMapping.NoResultSet) { - CommandResultSet[lastIndex - 1] = ResultSetMapping.LastInResultSet; + CommandResultSet[^1] = ResultSetMapping.LastInResultSet; } } @@ -195,50 +132,33 @@ private void AppendBulkInsertCommandText(int lastIndex) /// 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. /// - protected override void UpdateCachedCommandText() - { - base.UpdateCachedCommandText(); - - AppendBulkInsertCommandText(ModificationCommands.Count); - } - - /// - /// 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. - /// - protected override void UpdateCachedCommandText(int commandPosition) + protected override void AddCommand(IReadOnlyModificationCommand modificationCommand) { - var newModificationCommand = ModificationCommands[commandPosition]; - - if (newModificationCommand.EntityState == EntityState.Added) + if (modificationCommand.EntityState == EntityState.Added) { - if (_bulkInsertCommands.Count > 0 - && !CanBeInsertedInSameStatement(_bulkInsertCommands[0], newModificationCommand)) + if (_pendingBulkInsertCommands.Count > 0 + && !CanBeInsertedInSameStatement(_pendingBulkInsertCommands[0], modificationCommand)) { // The new Add command cannot be added to the pending bulk insert commands (e.g. different table). // Write out the pending commands before starting a new pending chain. - AppendBulkInsertCommandText(commandPosition); - _bulkInsertCommands.Clear(); + ApplyPendingBulkInsertCommands(); + _pendingBulkInsertCommands.Clear(); } - _bulkInsertCommands.Add(newModificationCommand); - - LastCachedCommandIndex = commandPosition; + _pendingBulkInsertCommands.Add(modificationCommand); } else { // If we have any pending bulk insert commands, write them out before the next non-Add command - if (_bulkInsertCommands.Count > 0) + if (_pendingBulkInsertCommands.Count > 0) { // Note that we don't care about the transactionality of the bulk insert SQL, since there's the additional non-Add // command coming right afterwards, and so a transaction is required in any case. - AppendBulkInsertCommandText(commandPosition); - _bulkInsertCommands.Clear(); + ApplyPendingBulkInsertCommands(); + _pendingBulkInsertCommands.Clear(); } - base.UpdateCachedCommandText(commandPosition); + base.AddCommand(modificationCommand); } } @@ -252,6 +172,19 @@ private static bool CanBeInsertedInSameStatement( && firstCommand.ColumnModifications.Where(o => o.IsRead).Select(o => o.ColumnName).SequenceEqual( secondCommand.ColumnModifications.Where(o => o.IsRead).Select(o => o.ColumnName)); + /// + /// 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. + /// + public override void Complete() + { + ApplyPendingBulkInsertCommands(); + + base.Complete(); + } + /// /// 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 diff --git a/test/EFCore.Relational.Tests/TestUtilities/TestModificationCommandBatch.cs b/test/EFCore.Relational.Tests/TestUtilities/TestModificationCommandBatch.cs index 121e59c9c59..9d50183e4f0 100644 --- a/test/EFCore.Relational.Tests/TestUtilities/TestModificationCommandBatch.cs +++ b/test/EFCore.Relational.Tests/TestUtilities/TestModificationCommandBatch.cs @@ -15,6 +15,6 @@ public TestModificationCommandBatch( _maxBatchSize = maxBatchSize ?? 1; } - protected override bool CanAddCommand(IReadOnlyModificationCommand modificationCommand) - => ModificationCommands.Count < _maxBatchSize; + protected override bool IsValid() + => ModificationCommands.Count <= _maxBatchSize; } diff --git a/test/EFCore.Relational.Tests/Update/ReaderModificationCommandBatchTest.cs b/test/EFCore.Relational.Tests/Update/ReaderModificationCommandBatchTest.cs index f4848969fe0..00e251db6ee 100644 --- a/test/EFCore.Relational.Tests/Update/ReaderModificationCommandBatchTest.cs +++ b/test/EFCore.Relational.Tests/Update/ReaderModificationCommandBatchTest.cs @@ -15,55 +15,53 @@ namespace Microsoft.EntityFrameworkCore.Update; public class ReaderModificationCommandBatchTest { [ConditionalFact] - public void AddCommand_adds_command_if_possible() + public void AddCommand_adds_command_if_batch_is_valid() { - var command = CreateModificationCommand("T1", null, true, columnModifications: null); + var command1 = CreateModificationCommand("T1", null, true, columnModifications: null); + var command2 = CreateModificationCommand("T2", null, true, columnModifications: null); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); - batch.ShouldAddCommand = true; - batch.ShouldValidateSql = true; - - batch.AddCommand(command); + batch.ShouldBeValid = true; + Assert.True(batch.TryAddCommand(command1)); + Assert.True(batch.TryAddCommand(command2)); batch.Complete(); - Assert.Equal(2, batch.ModificationCommands.Count); - Assert.Same(command, batch.ModificationCommands[0]); - Assert.Equal("..", batch.CommandText); - } - - [ConditionalFact] - public void AddCommand_does_not_add_command_if_not_possible() - { - var command = CreateModificationCommand("T1", null, true, columnModifications: null); + Assert.Collection(batch.ModificationCommands, + m => Assert.Same(command1, m), + m => Assert.Same(command2, m)); - var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); - batch.ShouldAddCommand = false; - batch.ShouldValidateSql = true; + Assert.Equal(@"UPDATE ""T1"" SET ; +SELECT provider_specific_rowcount(); - batch.AddCommand(command); - batch.Complete(); +UPDATE ""T2"" SET ; +SELECT provider_specific_rowcount(); - Assert.Equal(1, batch.ModificationCommands.Count); - Assert.Equal(".", batch.CommandText); +", + batch.CommandText, + ignoreLineEndingDifferences: true); } [ConditionalFact] - public void AddCommand_does_not_add_command_if_resulting_sql_is_invalid() + public void AddCommand_does_not_add_command_batch_is_invalid() { - var command = CreateModificationCommand("T1", null, true, columnModifications: null); + var command1 = CreateModificationCommand("T1", null, true, columnModifications: null); + var command2 = CreateModificationCommand("T2", null, true, columnModifications: null); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); - batch.ShouldAddCommand = true; - batch.ShouldValidateSql = false; + Assert.True(batch.TryAddCommand(command1)); + batch.ShouldBeValid = false; - batch.AddCommand(command); + Assert.False(batch.TryAddCommand(command2)); batch.Complete(); - Assert.Equal(1, batch.ModificationCommands.Count); - Assert.Equal(".", batch.CommandText); + Assert.Same(command1, Assert.Single(batch.ModificationCommands)); + + Assert.Equal(@"UPDATE ""T1"" SET ; +SELECT provider_specific_rowcount(); + +", + batch.CommandText, + ignoreLineEndingDifferences: true); } [ConditionalFact] @@ -74,16 +72,12 @@ public void UpdateCommandText_compiles_inserts() var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); command.AddEntry(entry, true); - var fakeSqlGenerator = new FakeSqlGenerator( - RelationalTestHelpers.Instance.CreateContextServices().GetRequiredService()); - var batch = new ModificationCommandBatchFake(fakeSqlGenerator); - batch.AddCommand(command); + var batch = new ModificationCommandBatchFake(); + batch.TryAddCommand(command); batch.Complete(); - batch.UpdateCachedCommandTextBase(0); - - Assert.Equal(1, fakeSqlGenerator.AppendBatchHeaderCalls); - Assert.Equal(1, fakeSqlGenerator.AppendInsertOperationCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendBatchHeaderCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendInsertOperationCalls); } [ConditionalFact] @@ -94,16 +88,12 @@ public void UpdateCommandText_compiles_updates() var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); command.AddEntry(entry, true); - var fakeSqlGenerator = new FakeSqlGenerator( - RelationalTestHelpers.Instance.CreateContextServices().GetRequiredService()); - var batch = new ModificationCommandBatchFake(fakeSqlGenerator); - batch.AddCommand(command); - - batch.UpdateCachedCommandTextBase(0); + var batch = new ModificationCommandBatchFake(); + batch.TryAddCommand(command); batch.Complete(); - Assert.Equal(1, fakeSqlGenerator.AppendBatchHeaderCalls); - Assert.Equal(1, fakeSqlGenerator.AppendUpdateOperationCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendBatchHeaderCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendUpdateOperationCalls); } [ConditionalFact] @@ -114,16 +104,12 @@ public void UpdateCommandText_compiles_deletes() var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); command.AddEntry(entry, true); - var fakeSqlGenerator = new FakeSqlGenerator( - RelationalTestHelpers.Instance.CreateContextServices().GetRequiredService()); - var batch = new ModificationCommandBatchFake(fakeSqlGenerator); - batch.AddCommand(command); - - batch.UpdateCachedCommandTextBase(0); + var batch = new ModificationCommandBatchFake(); + batch.TryAddCommand(command); batch.Complete(); - Assert.Equal(1, fakeSqlGenerator.AppendBatchHeaderCalls); - Assert.Equal(1, fakeSqlGenerator.AppendDeleteOperationCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendBatchHeaderCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendDeleteOperationCalls); } [ConditionalFact] @@ -131,25 +117,25 @@ public void UpdateCommandText_compiles_multiple_commands() { var entry = CreateEntry(EntityState.Added); - var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); - command.AddEntry(entry, true); + var parameterNameGenerator = new ParameterNameGenerator(); + var command1 = CreateModificationCommand("T1", null, parameterNameGenerator.GenerateNext, true, null); + command1.AddEntry(entry, true); + var command2 = CreateModificationCommand("T1", null, parameterNameGenerator.GenerateNext, true, null); + command2.AddEntry(entry, true); - var fakeSqlGenerator = new FakeSqlGenerator( - RelationalTestHelpers.Instance.CreateContextServices().GetRequiredService()); - var batch = new ModificationCommandBatchFake(fakeSqlGenerator); - batch.AddCommand(command); - batch.AddCommand(command); + var batch = new ModificationCommandBatchFake(); + batch.TryAddCommand(command1); + batch.TryAddCommand(command2); batch.Complete(); - Assert.Equal("..", batch.CommandText); - - Assert.Equal(1, fakeSqlGenerator.AppendBatchHeaderCalls); + Assert.Equal(1, batch.FakeSqlGenerator.AppendBatchHeaderCalls); + Assert.Equal(2, batch.FakeSqlGenerator.AppendInsertOperationCalls); } [ConditionalFact] public async Task ExecuteAsync_executes_batch_commands_and_consumes_reader() { - var entry = CreateEntry(EntityState.Added); + var entry = CreateEntry(EntityState.Added, generateKeyValues: true); var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); command.AddEntry(entry, true); @@ -159,7 +145,7 @@ public async Task ExecuteAsync_executes_batch_commands_and_consumes_reader() var connection = CreateConnection(dbDataReader); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); await batch.ExecuteAsync(connection); @@ -182,7 +168,7 @@ public async Task ExecuteAsync_saves_store_generated_values() new[] { "Col1" }, new List { new object[] { 42 } })); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); await batch.ExecuteAsync(connection); @@ -206,7 +192,7 @@ public async Task ExecuteAsync_saves_store_generated_values_on_non_key_columns() new[] { "Col1", "Col2" }, new List { new object[] { 42, "FortyTwo" } })); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); await batch.ExecuteAsync(connection); @@ -219,7 +205,7 @@ public async Task ExecuteAsync_saves_store_generated_values_on_non_key_columns() public async Task ExecuteAsync_saves_store_generated_values_when_updating() { var entry = CreateEntry( - EntityState.Modified, generateKeyValues: true, computeNonKeyValue: true); + EntityState.Modified, generateKeyValues: true, overrideKeyValues: true, computeNonKeyValue: true); var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); command.AddEntry(entry, true); @@ -229,7 +215,7 @@ public async Task ExecuteAsync_saves_store_generated_values_when_updating() new[] { "Col2" }, new List { new object[] { "FortyTwo" } })); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); await batch.ExecuteAsync(connection); @@ -253,7 +239,7 @@ public async Task Exception_not_thrown_for_more_than_one_row_returned_for_single new List { new object[] { 42 }, new object[] { 43 } })); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); await batch.ExecuteAsync(connection); @@ -266,7 +252,7 @@ public async Task Exception_not_thrown_for_more_than_one_row_returned_for_single [InlineData(true)] public async Task Exception_thrown_if_rows_returned_for_command_without_store_generated_values_is_not_1(bool async) { - var entry = CreateEntry(EntityState.Added); + var entry = CreateEntry(EntityState.Modified); var command = CreateModificationCommand("T1", null, new ParameterNameGenerator().GenerateNext, true, null); command.AddEntry(entry, true); @@ -276,7 +262,7 @@ public async Task Exception_thrown_if_rows_returned_for_command_without_store_ge new[] { "Col1" }, new List { new object[] { 42 } })); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); var exception = async @@ -301,7 +287,7 @@ public async Task Exception_thrown_if_no_rows_returned_for_command_with_store_ge CreateFakeDataReader(new[] { "Col1" }, new List())); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); var exception = async @@ -329,7 +315,7 @@ public async Task DbException_is_wrapped_with_DbUpdateException(bool async) executeReader: (c, b) => throw originalException)); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); var actualException = async @@ -357,7 +343,7 @@ public async Task OperationCanceledException_is_not_wrapped_with_DbUpdateExcepti executeReader: (c, b) => throw originalException)); var batch = new ModificationCommandBatchFake(); - batch.AddCommand(command); + batch.TryAddCommand(command); batch.Complete(); var actualException = async @@ -377,7 +363,7 @@ public void CreateStoreCommand_creates_parameters_for_each_ModificationCommand() var batch = new ModificationCommandBatchFake(); var parameterNameGenerator = new ParameterNameGenerator(); - batch.AddCommand( + batch.TryAddCommand( CreateModificationCommand( "T1", null, @@ -393,7 +379,7 @@ public void CreateStoreCommand_creates_parameters_for_each_ModificationCommand() false, true, false, false, true) })); - batch.AddCommand( + batch.TryAddCommand( CreateModificationCommand( "T1", null, @@ -411,7 +397,7 @@ public void CreateStoreCommand_creates_parameters_for_each_ModificationCommand() batch.Complete(); - var storeCommand = batch.CreateStoreCommandBase(); + var storeCommand = batch.StoreCommand; Assert.Equal(2, storeCommand.RelationalCommand.Parameters.Count); Assert.Equal("p0", storeCommand.RelationalCommand.Parameters[0].InvariantName); @@ -431,7 +417,7 @@ public void PopulateParameters_creates_parameter_for_write_ModificationCommand() var batch = new ModificationCommandBatchFake(); var parameterNameGenerator = new ParameterNameGenerator(); - batch.AddCommand( + batch.TryAddCommand( CreateModificationCommand( "T1", null, @@ -450,7 +436,7 @@ public void PopulateParameters_creates_parameter_for_write_ModificationCommand() batch.Complete(); - var storeCommand = batch.CreateStoreCommandBase(); + var storeCommand = batch.StoreCommand; Assert.Equal(1, storeCommand.RelationalCommand.Parameters.Count); Assert.Equal("p0", storeCommand.RelationalCommand.Parameters[0].InvariantName); @@ -468,7 +454,7 @@ public void PopulateParameters_creates_parameter_for_condition_ModificationComma var batch = new ModificationCommandBatchFake(); var parameterNameGenerator = new ParameterNameGenerator(); - batch.AddCommand( + batch.TryAddCommand( CreateModificationCommand( "T1", null, @@ -487,7 +473,7 @@ public void PopulateParameters_creates_parameter_for_condition_ModificationComma batch.Complete(); - var storeCommand = batch.CreateStoreCommandBase(); + var storeCommand = batch.StoreCommand; Assert.Equal(1, storeCommand.RelationalCommand.Parameters.Count); Assert.Equal("p0", storeCommand.RelationalCommand.Parameters[0].InvariantName); @@ -505,7 +491,7 @@ public void PopulateParameters_creates_parameters_for_write_and_condition_Modifi var batch = new ModificationCommandBatchFake(); var parameterNameGenerator = new ParameterNameGenerator(); - batch.AddCommand( + batch.TryAddCommand( CreateModificationCommand( "T1", null, @@ -524,7 +510,7 @@ public void PopulateParameters_creates_parameters_for_write_and_condition_Modifi batch.Complete(); - var storeCommand = batch.CreateStoreCommandBase(); + var storeCommand = batch.StoreCommand; Assert.Equal(2, storeCommand.RelationalCommand.Parameters.Count); Assert.Equal("p0", storeCommand.RelationalCommand.Parameters[0].InvariantName); @@ -544,7 +530,7 @@ public void PopulateParameters_does_not_create_parameter_for_read_ModificationCo var batch = new ModificationCommandBatchFake(); var parameterNameGenerator = new ParameterNameGenerator(); - batch.AddCommand( + batch.TryAddCommand( CreateModificationCommand( "T1", null, @@ -563,7 +549,7 @@ public void PopulateParameters_does_not_create_parameter_for_read_ModificationCo batch.Complete(); - var storeCommand = batch.CreateStoreCommandBase(); + var storeCommand = batch.StoreCommand; Assert.Equal(0, storeCommand.RelationalCommand.Parameters.Count); } @@ -597,12 +583,17 @@ private static IModel BuildModel(bool generateKeyValues, bool computeNonKeyValue private static InternalEntityEntry CreateEntry( EntityState entityState, bool generateKeyValues = false, + bool overrideKeyValues = false, bool computeNonKeyValue = false) { var model = BuildModel(generateKeyValues, computeNonKeyValue); return RelationalTestHelpers.Instance.CreateInternalEntry( - model, entityState, new T1 { Id = 1, Name = computeNonKeyValue ? null : "Test" }); + model, entityState, new T1 + { + Id = overrideKeyValues ? 1 : default, + Name = computeNonKeyValue ? null : "Test" + }); } private static FakeDbDataReader CreateFakeDataReader(string[] columnNames = null, IList results = null) @@ -615,12 +606,14 @@ private static FakeDbDataReader CreateFakeDataReader(string[] columnNames = null private class ModificationCommandBatchFake : AffectedCountModificationCommandBatch { - public ModificationCommandBatchFake( - IUpdateSqlGenerator sqlGenerator = null) + private readonly FakeSqlGenerator _fakeSqlGenerator; + + public ModificationCommandBatchFake(IUpdateSqlGenerator sqlGenerator = null) : base(CreateDependencies(sqlGenerator)) { - ShouldAddCommand = true; - ShouldValidateSql = true; + ShouldBeValid = true; + + _fakeSqlGenerator = Dependencies.UpdateSqlGenerator as FakeSqlGenerator; } private static ModificationCommandBatchFactoryDependencies CreateDependencies( @@ -632,6 +625,10 @@ private static ModificationCommandBatchFactoryDependencies CreateDependencies( var logger = new FakeRelationalCommandDiagnosticsLogger(); + sqlGenerator ??= new FakeSqlGenerator( + RelationalTestHelpers.Instance.CreateContextServices() + .GetRequiredService()); + return new ModificationCommandBatchFactoryDependencies( new RelationalCommandBuilderFactory( new RelationalCommandBuilderDependencies( @@ -639,10 +636,7 @@ private static ModificationCommandBatchFactoryDependencies CreateDependencies( new ExceptionDetector())), new RelationalSqlGenerationHelper( new RelationalSqlGenerationHelperDependencies()), - sqlGenerator - ?? new FakeSqlGenerator( - RelationalTestHelpers.Instance.CreateContextServices() - .GetRequiredService()), + sqlGenerator, new TypedRelationalValueBufferFactoryFactory( new RelationalValueBufferFactoryDependencies( typeMappingSource, @@ -651,27 +645,20 @@ private static ModificationCommandBatchFactoryDependencies CreateDependencies( logger); } - public string CommandText - => CachedCommandText.ToString(); - public bool ShouldAddCommand { get; set; } - - protected override bool CanAddCommand(IReadOnlyModificationCommand modificationCommand) - => ShouldAddCommand; - - public bool ShouldValidateSql { get; set; } + public string CommandText + => SqlBuilder.ToString(); - protected override bool IsCommandTextValid() - => ShouldValidateSql; + public bool ShouldBeValid { get; set; } - protected override void UpdateCachedCommandText(int commandIndex) - => CachedCommandText.Append("."); + protected override bool IsValid() + => ShouldBeValid; - public void UpdateCachedCommandTextBase(int commandIndex) - => base.UpdateCachedCommandText(commandIndex); + public new RawSqlCommand StoreCommand + => base.StoreCommand; - public RawSqlCommand CreateStoreCommandBase() - => CreateStoreCommand(); + public FakeSqlGenerator FakeSqlGenerator + => _fakeSqlGenerator ?? throw new InvalidOperationException("Not using FakeSqlGenerator"); } private class FakeDbContext : DbContext diff --git a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestRelationalCommandBuilderFactory.cs b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestRelationalCommandBuilderFactory.cs index a1144359e67..0a9b2685713 100644 --- a/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestRelationalCommandBuilderFactory.cs +++ b/test/EFCore.SqlServer.FunctionalTests/TestUtilities/TestRelationalCommandBuilderFactory.cs @@ -40,6 +40,13 @@ public IRelationalCommandBuilder AddParameter(IRelationalParameter parameter) return this; } + public IRelationalCommandBuilder RemoveParameterAt(int index) + { + _parameters.RemoveAt(index); + + return this; + } + [Obsolete("Code trying to add parameter should add type mapped parameter using TypeMappingSource directly.")] public IRelationalTypeMappingSource TypeMappingSource => Dependencies.TypeMappingSource; diff --git a/test/EFCore.SqlServer.Tests/SqlServerDatabaseCreatorTest.cs b/test/EFCore.SqlServer.Tests/SqlServerDatabaseCreatorTest.cs index fb625df70a0..a5a9ed19b75 100644 --- a/test/EFCore.SqlServer.Tests/SqlServerDatabaseCreatorTest.cs +++ b/test/EFCore.SqlServer.Tests/SqlServerDatabaseCreatorTest.cs @@ -184,6 +184,13 @@ public IRelationalCommandBuilder AddParameter(IRelationalParameter parameter) return this; } + public IRelationalCommandBuilder RemoveParameterAt(int index) + { + _parameters.RemoveAt(index); + + return this; + } + public IRelationalTypeMappingSource TypeMappingSource => null; diff --git a/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchFactoryTest.cs b/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchFactoryTest.cs index bfd00295b34..e172e013bbc 100644 --- a/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchFactoryTest.cs +++ b/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchFactoryTest.cs @@ -45,8 +45,8 @@ public void Uses_MaxBatchSize_specified_in_SqlServerOptionsExtension() var batch = factory.Create(); - Assert.True(batch.AddCommand(CreateModificationCommand("T1", null, false))); - Assert.False(batch.AddCommand(CreateModificationCommand("T1", null, false))); + Assert.True(batch.TryAddCommand(CreateModificationCommand("T1", null, false))); + Assert.False(batch.TryAddCommand(CreateModificationCommand("T1", null, false))); } [ConditionalFact] @@ -83,8 +83,8 @@ public void MaxBatchSize_is_optional() var batch = factory.Create(); - Assert.True(batch.AddCommand(CreateModificationCommand("T1", null, false))); - Assert.True(batch.AddCommand(CreateModificationCommand("T1", null, false))); + Assert.True(batch.TryAddCommand(CreateModificationCommand("T1", null, false))); + Assert.True(batch.TryAddCommand(CreateModificationCommand("T1", null, false))); } private class FakeDbContext : DbContext diff --git a/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs b/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs index adc2169c793..606372df527 100644 --- a/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs +++ b/test/EFCore.SqlServer.Tests/Update/SqlServerModificationCommandBatchTest.cs @@ -14,13 +14,58 @@ public class SqlServerModificationCommandBatchTest [ConditionalFact] public void AddCommand_returns_false_when_max_batch_size_is_reached() { - var typeMapper = new SqlServerTypeMappingSource( - TestServiceFactory.Instance.Create(), - TestServiceFactory.Instance.Create()); + var batch = CreateBatch(maxBatchSize: 1); - var logger = new FakeRelationalCommandDiagnosticsLogger(); + var firstCommand = CreateModificationCommand("T1", null, false); + Assert.True(batch.TryAddCommand(firstCommand)); + Assert.False(batch.TryAddCommand(CreateModificationCommand("T1", null, false))); + + Assert.Same(firstCommand, Assert.Single(batch.ModificationCommands)); + } + + [ConditionalFact] + public void AddCommand_returns_false_when_max_parameters_are_reached() + { + var typeMapper = CreateTypeMappingSource(); + var intMapping = typeMapper.FindMapping(typeof(int)); + var paramIndex = 0; - var batch = new SqlServerModificationCommandBatch( + var batch = CreateBatch(); + + var command = CreateModificationCommand("T1", null, false); + for (var i = 0; i < 2098; i++) + { + command.AddColumnModification(CreateModificationParameters("col" + i)); + } + Assert.True(batch.TryAddCommand(command)); + + var secondCommand = CreateModificationCommand("T2", null, false); + secondCommand.AddColumnModification(CreateModificationParameters("col")); + Assert.False(batch.TryAddCommand(secondCommand)); + Assert.Same(command, Assert.Single(batch.ModificationCommands)); + Assert.Equal(2098, batch.ParameterValues.Count); + + ColumnModificationParameters CreateModificationParameters(string columnName) + => new() + { + ColumnName = columnName, + ColumnType = "integer", + TypeMapping = intMapping, + IsWrite = true, + OriginalValue = 8, + GenerateParameterName = () => "p" + paramIndex++ + }; + } + + private class FakeDbContext : DbContext + { + } + + private static TestSqlServerModificationCommandBatch CreateBatch(int? maxBatchSize = null) + { + var typeMapper = CreateTypeMappingSource(); + + return new TestSqlServerModificationCommandBatch( new ModificationCommandBatchFactoryDependencies( new RelationalCommandBuilderFactory( new RelationalCommandBuilderDependencies( @@ -37,32 +82,30 @@ public void AddCommand_returns_false_when_max_batch_size_is_reached() new RelationalValueBufferFactoryDependencies( typeMapper, new CoreSingletonOptions())), new CurrentDbContext(new FakeDbContext()), - logger), - 1); - - Assert.True( - batch.AddCommand( - CreateModificationCommand("T1", null, false))); - Assert.False( - batch.AddCommand( - CreateModificationCommand("T1", null, false))); + new FakeRelationalCommandDiagnosticsLogger()), + maxBatchSize); } - private class FakeDbContext : DbContext - { - } + private static SqlServerTypeMappingSource CreateTypeMappingSource() + => new( + TestServiceFactory.Instance.Create(), + TestServiceFactory.Instance.Create()); private static IModificationCommand CreateModificationCommand( string name, string schema, bool sensitiveLoggingEnabled) - { - var modificationCommandParameters = new ModificationCommandParameters( - name, schema, sensitiveLoggingEnabled); + => new ModificationCommandFactory().CreateModificationCommand( + new ModificationCommandParameters(name, schema, sensitiveLoggingEnabled)); - var modificationCommand = new ModificationCommandFactory().CreateModificationCommand( - modificationCommandParameters); + private class TestSqlServerModificationCommandBatch : SqlServerModificationCommandBatch + { + public TestSqlServerModificationCommandBatch(ModificationCommandBatchFactoryDependencies dependencies, int? maxBatchSize) + : base(dependencies, maxBatchSize) + { + } - return modificationCommand; + public new Dictionary ParameterValues + => base.ParameterValues; } }