From 4a2a40c2a053cfd6f0385d77103bce8db3530b1c Mon Sep 17 00:00:00 2001 From: maca88 Date: Thu, 1 Mar 2018 21:12:55 +0100 Subject: [PATCH 01/12] Added batching support for insert/update/delete statements on PostgreSQL --- src/NHibernate.Test/Ado/BatcherFixture.cs | 6 + .../Async/Ado/BatcherFixture.cs | 6 + .../TypesTest/AbstractDateTimeTypeFixture.cs | 11 +- .../TypesTest/AbstractDateTimeTypeFixture.cs | 11 +- src/NHibernate/AdoNet/AbstractBatcher.cs | 12 + .../AdoNet/PostgreSQLClientBatchingBatcher.cs | 263 ++++++++++++++++++ .../PostgreSQLClientBatchingBatcherFactory.cs | 12 + .../AdoNet/PostgreSQLClientBatchingBatcher.cs | 121 ++++++++ src/NHibernate/Driver/DriverBase.cs | 22 +- 9 files changed, 456 insertions(+), 8 deletions(-) create mode 100644 src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs create mode 100644 src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs create mode 100644 src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs diff --git a/src/NHibernate.Test/Ado/BatcherFixture.cs b/src/NHibernate.Test/Ado/BatcherFixture.cs index 4d7e8107673..d7e617c454c 100644 --- a/src/NHibernate.Test/Ado/BatcherFixture.cs +++ b/src/NHibernate.Test/Ado/BatcherFixture.cs @@ -59,6 +59,7 @@ private void FillDb() { s.Save(new VerySimple {Id = 1, Name = "Fabio", Weight = 119.5}); s.Save(new VerySimple {Id = 2, Name = "Fiamma", Weight = 9.8}); + s.Save(new VerySimple {Id = 3, Name = "Roberto", Weight = 98.8 }); tx.Commit(); } } @@ -74,11 +75,14 @@ public void OneRoundTripUpdate() { var vs1 = s.Get(1); var vs2 = s.Get(2); + var vs3 = s.Get(3); vs1.Weight -= 10; vs2.Weight -= 1; + vs3.Weight -= 5; Sfi.Statistics.Clear(); s.Update(vs1); s.Update(vs2); + s.Update(vs3); tx.Commit(); } @@ -154,9 +158,11 @@ public void OneRoundTripDelete() { var vs1 = s.Get(1); var vs2 = s.Get(2); + var vs3 = s.Get(3); Sfi.Statistics.Clear(); s.Delete(vs1); s.Delete(vs2); + s.Delete(vs3); tx.Commit(); } diff --git a/src/NHibernate.Test/Async/Ado/BatcherFixture.cs b/src/NHibernate.Test/Async/Ado/BatcherFixture.cs index 20c3cdc04fd..db11dd2450b 100644 --- a/src/NHibernate.Test/Async/Ado/BatcherFixture.cs +++ b/src/NHibernate.Test/Async/Ado/BatcherFixture.cs @@ -71,6 +71,7 @@ public async Task OneRoundTripInsertsAsync() { await (s.SaveAsync(new VerySimple {Id = 1, Name = "Fabio", Weight = 119.5}, cancellationToken)); await (s.SaveAsync(new VerySimple {Id = 2, Name = "Fiamma", Weight = 9.8}, cancellationToken)); + await (s.SaveAsync(new VerySimple {Id = 3, Name = "Roberto", Weight = 98.8 }, cancellationToken)); await (tx.CommitAsync(cancellationToken)); } } @@ -86,11 +87,14 @@ public async Task OneRoundTripUpdateAsync() { var vs1 = await (s.GetAsync(1)); var vs2 = await (s.GetAsync(2)); + var vs3 = await (s.GetAsync(3)); vs1.Weight -= 10; vs2.Weight -= 1; + vs3.Weight -= 5; Sfi.Statistics.Clear(); await (s.UpdateAsync(vs1)); await (s.UpdateAsync(vs2)); + await (s.UpdateAsync(vs3)); await (tx.CommitAsync()); } @@ -166,9 +170,11 @@ public async Task OneRoundTripDeleteAsync() { var vs1 = await (s.GetAsync(1)); var vs2 = await (s.GetAsync(2)); + var vs3 = await (s.GetAsync(3)); Sfi.Statistics.Clear(); await (s.DeleteAsync(vs1)); await (s.DeleteAsync(vs2)); + await (s.DeleteAsync(vs3)); await (tx.CommitAsync()); } diff --git a/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs b/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs index 5cb1c03a231..20a21363a5d 100644 --- a/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs +++ b/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs @@ -14,6 +14,7 @@ using System.Data.Common; using System.Linq; using System.Reflection; +using NHibernate.AdoNet; using NHibernate.Cfg; using NHibernate.Driver; using NHibernate.Engine; @@ -246,8 +247,16 @@ public virtual async Task SaveUseExpectedSqlTypeAsync() await (t.CommitAsync()); } + var expected = 3; + // PostgreSQL batcher uses IDriver.GenerateCommand method to create the batching command, + // so the expected result will be doubled as GenerateCommand calls IDriver.GenerateParameter + // for each parameter. + if (Sfi.Settings.BatcherFactory is PostgreSQLClientBatchingBatcherFactory) + { + expected *= 2; + } // 2 properties + revision - AssertSqlType(driver, 3, true); + AssertSqlType(driver, expected, true); } [Test] diff --git a/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs b/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs index 75fc976284e..c24886e92bc 100644 --- a/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs +++ b/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs @@ -4,6 +4,7 @@ using System.Data.Common; using System.Linq; using System.Reflection; +using NHibernate.AdoNet; using NHibernate.Cfg; using NHibernate.Driver; using NHibernate.Engine; @@ -277,8 +278,16 @@ public virtual void SaveUseExpectedSqlType() t.Commit(); } + var expected = 3; + // PostgreSQL batcher uses IDriver.GenerateCommand method to create the batching command, + // so the expected result will be doubled as GenerateCommand calls IDriver.GenerateParameter + // for each parameter. + if (Sfi.Settings.BatcherFactory is PostgreSQLClientBatchingBatcherFactory) + { + expected *= 2; + } // 2 properties + revision - AssertSqlType(driver, 3, true); + AssertSqlType(driver, expected, true); } [Test] diff --git a/src/NHibernate/AdoNet/AbstractBatcher.cs b/src/NHibernate/AdoNet/AbstractBatcher.cs index f31862b6662..ed893cd387b 100644 --- a/src/NHibernate/AdoNet/AbstractBatcher.cs +++ b/src/NHibernate/AdoNet/AbstractBatcher.cs @@ -67,6 +67,18 @@ protected DbCommand CurrentCommand get { return _batchCommand; } } + /// + /// Gets the current that is contained for this Batch + /// + /// The current . + protected SqlString CurrentCommandSql => _batchCommandSql; + + /// + /// Gets the current parameters that are contained for this Batch + /// + /// The current . + protected SqlType[] CurrentCommandParameterTypes => _batchCommandParameterTypes; + public DbCommand Generate(CommandType type, SqlString sqlString, SqlType[] parameterTypes) { SqlString sql = GetSQL(sqlString); diff --git a/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs b/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs new file mode 100644 index 00000000000..141a25a9f42 --- /dev/null +++ b/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs @@ -0,0 +1,263 @@ +using System; +using System.Collections.Generic; +using System.Data; +using System.Data.Common; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using NHibernate.AdoNet.Util; +using NHibernate.Driver; +using NHibernate.Exceptions; +using NHibernate.SqlCommand; +using NHibernate.SqlTypes; +using NHibernate.Util; + +namespace NHibernate.AdoNet +{ + /// + /// Batcher for PostgreSQL that will batch UPDATE/INSERT/DELETE commands. + /// + public partial class PostgreSQLClientBatchingBatcher : AbstractBatcher + { + private int _totalExpectedRowsAffected; + private PostgreSQLCommandSet _currentBatch; + private StringBuilder _currentBatchCommandsLog; + + public PostgreSQLClientBatchingBatcher(ConnectionManager connectionManager, IInterceptor interceptor) + : base(connectionManager, interceptor) + { + BatchSize = Factory.Settings.AdoBatchSize; + _currentBatch = CreateConfiguredBatch(); + + //we always create this, because we need to deal with a scenario in which + //the user change the logging configuration at runtime. Trying to put this + //behind an if(log.IsDebugEnabled) will cause a null reference exception + //at that point. + _currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:"); + } + + public sealed override int BatchSize { get; set; } + + protected override int CountOfStatementsInCurrentBatch => _currentBatch.CountOfCommands; + + public override void AddToBatch(IExpectation expectation) + { + _totalExpectedRowsAffected += expectation.ExpectedRowCount; + var batchUpdate = CurrentCommand; + Driver.AdjustCommand(batchUpdate); + string lineWithParameters = null; + var sqlStatementLogger = Factory.Settings.SqlStatementLogger; + if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) + { + lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchUpdate); + var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); + lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); + _currentBatchCommandsLog.Append("command ") + .Append(_currentBatch.CountOfCommands) + .Append(":") + .AppendLine(lineWithParameters); + } + if (Log.IsDebugEnabled()) + { + Log.Debug("Adding to batch:{0}", lineWithParameters); + } + _currentBatch.Append(CurrentCommand.Parameters); + + if (_currentBatch.CountOfCommands >= BatchSize) + { + ExecuteBatchWithTiming(batchUpdate); + } + } + + protected override void DoExecuteBatch(DbCommand ps) + { + try + { + Log.Debug("Executing batch"); + CheckReaders(); + if (Factory.Settings.SqlStatementLogger.IsDebugEnabled) + { + Factory.Settings.SqlStatementLogger.LogBatchCommand(_currentBatchCommandsLog.ToString()); + } + + int rowsAffected; + try + { + rowsAffected = _currentBatch.ExecuteNonQuery(); + } + catch (DbException e) + { + throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command."); + } + + Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected); + } + finally + { + ClearCurrentBatch(); + } + } + + private PostgreSQLCommandSet CreateConfiguredBatch() + { + return new PostgreSQLCommandSet(this); + } + + private void ClearCurrentBatch() + { + _currentBatch.Dispose(); + _totalExpectedRowsAffected = 0; + _currentBatch = CreateConfiguredBatch(); + + if (Factory.Settings.SqlStatementLogger.IsDebugEnabled) + { + _currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:"); + } + } + + public override void CloseCommands() + { + base.CloseCommands(); + + try + { + ClearCurrentBatch(); + } + catch (Exception e) + { + // Prevent exceptions when clearing the batch from hiding any original exception + // (We do not know here if this batch closing occurs after a failure or not.) + Log.Warn(e, "Exception clearing batch"); + } + } + + protected override void Dispose(bool isDisposing) + { + base.Dispose(isDisposing); + // Prevent exceptions when closing the batch from hiding any original exception + // (We do not know here if this batch closing occurs after a failure or not.) + try + { + _currentBatch.Dispose(); + } + catch (Exception e) + { + Log.Warn(e, "Exception closing batcher"); + } + } + + private partial class PostgreSQLCommandSet : IDisposable + { + private int _currentParameterIndex; + private DbCommand _batchCommand; + private readonly PostgreSQLClientBatchingBatcher _batcher; + + public PostgreSQLCommandSet(PostgreSQLClientBatchingBatcher batcher) + { + _batcher = batcher; + } + + public int CountOfCommands { get; private set; } + + public void Append(DbParameterCollection parameters) + { + if (_batchCommand == null) + { + _batchCommand = _batcher.Driver.GenerateCommand( + _batcher.CurrentCommand.CommandType, + _batcher.CurrentCommandSql, + _batcher.CurrentCommandParameterTypes); + UpdateCommandParameters(_batchCommand, parameters); + _currentParameterIndex = parameters.Count; + } + else + { + // We need to create a new command with different parameter names to avoid duplicates + var command = _batcher.Driver.GenerateCommand( + _batcher.CurrentCommand.CommandType, + PrepareSqlString(_batcher.CurrentCommandSql), + _batcher.CurrentCommandParameterTypes); + UpdateCommandParameters(command, parameters); + _batchCommand.CommandText += ";" + command.CommandText; + foreach (DbParameter parameter in command.Parameters) + { + _batchCommand.Parameters.Add(CopyParameter(_batchCommand, parameter)); + } + command.Dispose(); + } + CountOfCommands++; + } + + public int ExecuteNonQuery() + { + if (_batchCommand == null) + { + return 0; + } + // Npgsql will correctly prepare a multi SQL statement even if the parameter names are different + // for each statement. Npgsql internally parses the query and omits parameter names when comparing two queries + // in order to prevent having multiple prepared statements for the same query. + _batcher.Prepare(_batchCommand); + return _batchCommand.ExecuteNonQuery(); + } + + public void Dispose() + { + _batchCommand?.Dispose(); + _batchCommand = null; + _currentParameterIndex = 0; + CountOfCommands = 0; + } + + private void UpdateCommandParameters(DbCommand command, DbParameterCollection parameters) + { + for (var i = 0; i < parameters.Count; i++) + { + var parameter = parameters[i]; + var cmdParam = command.Parameters[i]; + cmdParam.Value = parameter.Value; + cmdParam.Direction = parameter.Direction; + cmdParam.Precision = parameter.Precision; + cmdParam.Scale = parameter.Scale; + cmdParam.Size = parameter.Size; + } + } + + private DbParameter CopyParameter(DbCommand command, DbParameter parameter) + { + var copy = command.CreateParameter(); + copy.DbType = parameter.DbType; + copy.IsNullable = parameter.IsNullable; + copy.ParameterName = parameter.ParameterName; + copy.Value = parameter.Value; + copy.Direction = parameter.Direction; + copy.Precision = parameter.Precision; + copy.Scale = parameter.Scale; + copy.Size = parameter.Size; + copy.SourceVersion = parameter.SourceVersion; + copy.SourceColumn = parameter.SourceColumn; + copy.SourceColumnNullMapping = parameter.SourceColumnNullMapping; + return copy; + } + + private SqlString PrepareSqlString(SqlString sql) + { + if (_batchCommand == null) + { + return sql; + } + sql = sql.Copy(); + foreach (var part in sql) + { + if (part is Parameter param) + { + param.ParameterPosition = _currentParameterIndex++; + } + } + return sql; + } + } + } +} diff --git a/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs b/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs new file mode 100644 index 00000000000..0aabe773782 --- /dev/null +++ b/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs @@ -0,0 +1,12 @@ +using NHibernate.Engine; + +namespace NHibernate.AdoNet +{ + public class PostgreSQLClientBatchingBatcherFactory: IBatcherFactory + { + public virtual IBatcher CreateBatcher(ConnectionManager connectionManager, IInterceptor interceptor) + { + return new PostgreSQLClientBatchingBatcher(connectionManager, interceptor); + } + } +} diff --git a/src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs b/src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs new file mode 100644 index 00000000000..902099bb8c8 --- /dev/null +++ b/src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs @@ -0,0 +1,121 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System; +using System.Collections.Generic; +using System.Data; +using System.Data.Common; +using System.Linq; +using System.Text; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using NHibernate.AdoNet.Util; +using NHibernate.Driver; +using NHibernate.Exceptions; +using NHibernate.SqlCommand; +using NHibernate.SqlTypes; +using NHibernate.Util; + +namespace NHibernate.AdoNet +{ + public partial class PostgreSQLClientBatchingBatcher : AbstractBatcher + { + + public override Task AddToBatchAsync(IExpectation expectation, CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); + } + try + { + _totalExpectedRowsAffected += expectation.ExpectedRowCount; + var batchUpdate = CurrentCommand; + Driver.AdjustCommand(batchUpdate); + string lineWithParameters = null; + var sqlStatementLogger = Factory.Settings.SqlStatementLogger; + if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) + { + lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchUpdate); + var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); + lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); + _currentBatchCommandsLog.Append("command ") + .Append(_currentBatch.CountOfCommands) + .Append(":") + .AppendLine(lineWithParameters); + } + if (Log.IsDebugEnabled()) + { + Log.Debug("Adding to batch:{0}", lineWithParameters); + } + _currentBatch.Append(CurrentCommand.Parameters); + + if (_currentBatch.CountOfCommands >= BatchSize) + { + return ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken); + } + return Task.CompletedTask; + } + catch (Exception ex) + { + return Task.FromException(ex); + } + } + + protected override async Task DoExecuteBatchAsync(DbCommand ps, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + try + { + Log.Debug("Executing batch"); + await (CheckReadersAsync(cancellationToken)).ConfigureAwait(false); + if (Factory.Settings.SqlStatementLogger.IsDebugEnabled) + { + Factory.Settings.SqlStatementLogger.LogBatchCommand(_currentBatchCommandsLog.ToString()); + } + + int rowsAffected; + try + { + rowsAffected = await (_currentBatch.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false); + } + catch (DbException e) + { + throw ADOExceptionHelper.Convert(Factory.SQLExceptionConverter, e, "could not execute batch command."); + } + + Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, rowsAffected); + } + finally + { + ClearCurrentBatch(); + } + } + + private partial class PostgreSQLCommandSet : IDisposable + { + + public async Task ExecuteNonQueryAsync(CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + if (_batchCommand == null) + { + return 0; + } + // Npgsql will correctly prepare a multi SQL statement even if the parameter names are different + // for each statement. Npgsql internally parses the query and omits parameter names when comparing two queries + // in order to prevent having multiple prepared statements for the same query. + await (_batcher.PrepareAsync(_batchCommand, cancellationToken)).ConfigureAwait(false); + return await (_batchCommand.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false); + } + } + } +} diff --git a/src/NHibernate/Driver/DriverBase.cs b/src/NHibernate/Driver/DriverBase.cs index c647ec0ff82..1a858d650aa 100644 --- a/src/NHibernate/Driver/DriverBase.cs +++ b/src/NHibernate/Driver/DriverBase.cs @@ -133,7 +133,7 @@ public virtual DbCommand GenerateCommand(CommandType type, SqlString sqlString, SetCommandTimeout(cmd); SetCommandText(cmd, sqlString); - SetCommandParameters(cmd, parameterTypes); + SetCommandParameters(cmd, parameterTypes, sqlString.GetParameters().ToArray()); return cmd; } @@ -156,9 +156,9 @@ protected virtual void SetCommandTimeout(DbCommand cmd) } } - private static string ToParameterName(int index) + private static string ToParameterName(int index, string prefix = null) { - return "p" + index; + return string.IsNullOrEmpty(prefix) ? "p" + index : prefix + index; } string ISqlParameterFormatter.GetParameterName(int index) @@ -178,13 +178,23 @@ protected virtual SqlStringFormatter GetSqlStringFormatter() return new SqlStringFormatter(this, ";"); } - private void SetCommandParameters(DbCommand cmd, SqlType[] sqlTypes) + private void SetCommandParameters(DbCommand cmd, SqlType[] sqlTypes, Parameter[] parameters) { - for (int i = 0; i < sqlTypes.Length; i++) + var usedParameters = new HashSet(); + for (var i = 0; i < sqlTypes.Length; i++) { - string paramName = ToParameterName(i); + var parameter = parameters.Length > i ? parameters[i] : null; + var position = parameter?.ParameterPosition ?? i; + // Use a different prefix when two or more parameters have the same position in + // order to prevert a name collision. When this happens, only one parameter will + // be used while the others are added to prevent an ArgumentOutOfRangeException + // being thrown when NamedParameterSpecification.Bind method is called. + var paramName = usedParameters.Contains(position) + ? ToParameterName(i, "fp") + : ToParameterName(position); var dbParam = GenerateParameter(cmd, paramName, sqlTypes[i]); cmd.Parameters.Add(dbParam); + usedParameters.Add(position); } } From 1014e35c39c8e9b968c41b15c682a1cf3bd19566 Mon Sep 17 00:00:00 2001 From: maca88 Date: Sat, 3 Mar 2018 14:03:14 +0100 Subject: [PATCH 02/12] Added an automatic flush when the max number of parameters are reached and renamed the batcher to a more universal name --- .../Ado/GenericBatchingBatcherFixture.cs | 149 ++++++++++++++++ .../Ado/GenericBatchingBatcherFixture.cs | 162 ++++++++++++++++++ .../TypesTest/AbstractDateTimeTypeFixture.cs | 4 +- .../TypesTest/AbstractDateTimeTypeFixture.cs | 4 +- ...ngBatcher.cs => GenericBatchingBatcher.cs} | 107 ++++++------ .../AdoNet/GenericBatchingBatcherFactory.cs | 19 ++ .../PostgreSQLClientBatchingBatcherFactory.cs | 12 -- ...ngBatcher.cs => GenericBatchingBatcher.cs} | 73 ++++---- 8 files changed, 417 insertions(+), 113 deletions(-) create mode 100644 src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs create mode 100644 src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs rename src/NHibernate/AdoNet/{PostgreSQLClientBatchingBatcher.cs => GenericBatchingBatcher.cs} (74%) create mode 100644 src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs delete mode 100644 src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs rename src/NHibernate/Async/AdoNet/{PostgreSQLClientBatchingBatcher.cs => GenericBatchingBatcher.cs} (59%) diff --git a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs new file mode 100644 index 00000000000..0721de20865 --- /dev/null +++ b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs @@ -0,0 +1,149 @@ +using System; +using System.Collections; +using System.Linq; +using log4net; +using NHibernate.AdoNet; +using NHibernate.Cfg; +using NHibernate.Dialect; +using NUnit.Framework; +using Environment = NHibernate.Cfg.Environment; + +namespace NHibernate.Test.Ado +{ + [TestFixture] + public class GenericBatchingBatcherFixture : TestCase + { + protected override string MappingsAssembly => "NHibernate.Test"; + + protected override IList Mappings => new[] {"Ado.VerySimple.hbm.xml"}; + + protected override void Configure(Configuration configuration) + { + configuration.SetProperty(Environment.BatchStrategy, typeof(GenericBatchingBatcherFactory).AssemblyQualifiedName); + configuration.SetProperty(Environment.GenerateStatistics, "true"); + configuration.SetProperty(Environment.BatchSize, "1000"); + } + + [Test] + public void MassiveInsertUpdateDeleteTest() + { + var totalRecords = 1000; + BatchInsert(totalRecords); + BatchUpdate(totalRecords); + BatchDelete(totalRecords); + + DbShoudBeEmpty(); + } + + [Test] + public void BatchSizeTest() + { + using (var sqlLog = new SqlLogSpy()) + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + s.SetBatchSize(5); + for (var i = 0; i < 20; i++) + { + s.Save(new VerySimple { Id = 1 + i, Name = $"Fabio{i}", Weight = 1.45 + i }); + } + tx.Commit(); + + var log = sqlLog.GetWholeLog(); + Assert.That(4, Is.EqualTo(FindAllOccurrences(log, "Batch commands:"))); + } + Cleanup(); + } + + private void BatchInsert(int totalRecords) + { + Sfi.Statistics.Clear(); + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + for (var i = 0; i < totalRecords; i++) + { + s.Save(new VerySimple {Id = 1 + i, Name = $"Fabio{i}", Weight = 1.45 + i}); + } + tx.Commit(); + } + Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); + } + + public void BatchUpdate(int totalRecords) + { + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + var items = s.Query().ToList(); + Assert.That(items.Count, Is.EqualTo(totalRecords)); + + foreach (var item in items) + { + item.Weight += 5; + s.Update(item); + } + + Sfi.Statistics.Clear(); + tx.Commit(); + } + Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); + } + + public void BatchDelete(int totalRecords) + { + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + var items = s.Query().ToList(); + Assert.That(items.Count, Is.EqualTo(totalRecords)); + + foreach (var item in items) + { + s.Delete(item); + } + + Sfi.Statistics.Clear(); + tx.Commit(); + } + Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); + } + + private void DbShoudBeEmpty() + { + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + var items = s.Query().ToList(); + Assert.That(items.Count, Is.EqualTo(0)); + + tx.Commit(); + } + } + + private void Cleanup() + { + using (var s = Sfi.OpenSession()) + using (s.BeginTransaction()) + { + s.CreateQuery("delete from VerySimple").ExecuteUpdate(); + s.Transaction.Commit(); + } + } + + private int FindAllOccurrences(string source, string substring) + { + if (source == null) + { + return 0; + } + int n = 0, count = 0; + while ((n = source.IndexOf(substring, n, StringComparison.InvariantCulture)) != -1) + { + n += substring.Length; + ++count; + } + return count; + } + } +} diff --git a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs new file mode 100644 index 00000000000..bb163fde4a2 --- /dev/null +++ b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs @@ -0,0 +1,162 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System; +using System.Collections; +using System.Linq; +using log4net; +using NHibernate.AdoNet; +using NHibernate.Cfg; +using NHibernate.Dialect; +using NUnit.Framework; +using Environment = NHibernate.Cfg.Environment; +using NHibernate.Linq; + +namespace NHibernate.Test.Ado +{ + using System.Threading.Tasks; + using System.Threading; + [TestFixture] + public class GenericBatchingBatcherFixtureAsync : TestCase + { + protected override string MappingsAssembly => "NHibernate.Test"; + + protected override IList Mappings => new[] {"Ado.VerySimple.hbm.xml"}; + + protected override void Configure(Configuration configuration) + { + configuration.SetProperty(Environment.BatchStrategy, typeof(GenericBatchingBatcherFactory).AssemblyQualifiedName); + configuration.SetProperty(Environment.GenerateStatistics, "true"); + configuration.SetProperty(Environment.BatchSize, "1000"); + } + + [Test] + public async Task MassiveInsertUpdateDeleteTestAsync() + { + var totalRecords = 1000; + await (BatchInsertAsync(totalRecords)); + await (BatchUpdateAsync(totalRecords)); + await (BatchDeleteAsync(totalRecords)); + + await (DbShoudBeEmptyAsync()); + } + + [Test] + public async Task BatchSizeTestAsync() + { + using (var sqlLog = new SqlLogSpy()) + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + s.SetBatchSize(5); + for (var i = 0; i < 20; i++) + { + await (s.SaveAsync(new VerySimple { Id = 1 + i, Name = $"Fabio{i}", Weight = 1.45 + i })); + } + await (tx.CommitAsync()); + + var log = sqlLog.GetWholeLog(); + Assert.That(4, Is.EqualTo(FindAllOccurrences(log, "Batch commands:"))); + } + await (CleanupAsync()); + } + + private async Task BatchInsertAsync(int totalRecords, CancellationToken cancellationToken = default(CancellationToken)) + { + Sfi.Statistics.Clear(); + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + for (var i = 0; i < totalRecords; i++) + { + await (s.SaveAsync(new VerySimple {Id = 1 + i, Name = $"Fabio{i}", Weight = 1.45 + i}, cancellationToken)); + } + await (tx.CommitAsync(cancellationToken)); + } + Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); + } + + public async Task BatchUpdateAsync(int totalRecords, CancellationToken cancellationToken = default(CancellationToken)) + { + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + var items = await (s.Query().ToListAsync(cancellationToken)); + Assert.That(items.Count, Is.EqualTo(totalRecords)); + + foreach (var item in items) + { + item.Weight += 5; + await (s.UpdateAsync(item, cancellationToken)); + } + + Sfi.Statistics.Clear(); + await (tx.CommitAsync(cancellationToken)); + } + Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); + } + + public async Task BatchDeleteAsync(int totalRecords, CancellationToken cancellationToken = default(CancellationToken)) + { + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + var items = await (s.Query().ToListAsync(cancellationToken)); + Assert.That(items.Count, Is.EqualTo(totalRecords)); + + foreach (var item in items) + { + await (s.DeleteAsync(item, cancellationToken)); + } + + Sfi.Statistics.Clear(); + await (tx.CommitAsync(cancellationToken)); + } + Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); + } + + private async Task DbShoudBeEmptyAsync(CancellationToken cancellationToken = default(CancellationToken)) + { + using (var s = Sfi.OpenSession()) + using (var tx = s.BeginTransaction()) + { + var items = await (s.Query().ToListAsync(cancellationToken)); + Assert.That(items.Count, Is.EqualTo(0)); + + await (tx.CommitAsync(cancellationToken)); + } + } + + private async Task CleanupAsync(CancellationToken cancellationToken = default(CancellationToken)) + { + using (var s = Sfi.OpenSession()) + using (s.BeginTransaction()) + { + await (s.CreateQuery("delete from VerySimple").ExecuteUpdateAsync(cancellationToken)); + await (s.Transaction.CommitAsync(cancellationToken)); + } + } + + private int FindAllOccurrences(string source, string substring) + { + if (source == null) + { + return 0; + } + int n = 0, count = 0; + while ((n = source.IndexOf(substring, n, StringComparison.InvariantCulture)) != -1) + { + n += substring.Length; + ++count; + } + return count; + } + } +} diff --git a/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs b/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs index 20a21363a5d..8995595b6bf 100644 --- a/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs +++ b/src/NHibernate.Test/Async/TypesTest/AbstractDateTimeTypeFixture.cs @@ -248,10 +248,10 @@ public virtual async Task SaveUseExpectedSqlTypeAsync() } var expected = 3; - // PostgreSQL batcher uses IDriver.GenerateCommand method to create the batching command, + // GenericBatchingBatcher uses IDriver.GenerateCommand method to create the batching command, // so the expected result will be doubled as GenerateCommand calls IDriver.GenerateParameter // for each parameter. - if (Sfi.Settings.BatcherFactory is PostgreSQLClientBatchingBatcherFactory) + if (Sfi.Settings.BatcherFactory is GenericBatchingBatcherFactory) { expected *= 2; } diff --git a/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs b/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs index c24886e92bc..ff54e4fab02 100644 --- a/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs +++ b/src/NHibernate.Test/TypesTest/AbstractDateTimeTypeFixture.cs @@ -279,10 +279,10 @@ public virtual void SaveUseExpectedSqlType() } var expected = 3; - // PostgreSQL batcher uses IDriver.GenerateCommand method to create the batching command, + // GenericBatchingBatcher uses IDriver.GenerateCommand method to create the batching command, // so the expected result will be doubled as GenerateCommand calls IDriver.GenerateParameter // for each parameter. - if (Sfi.Settings.BatcherFactory is PostgreSQLClientBatchingBatcherFactory) + if (Sfi.Settings.BatcherFactory is GenericBatchingBatcherFactory) { expected *= 2; } diff --git a/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs similarity index 74% rename from src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs rename to src/NHibernate/AdoNet/GenericBatchingBatcher.cs index 141a25a9f42..65a8bf801d8 100644 --- a/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -1,51 +1,57 @@ using System; -using System.Collections.Generic; -using System.Data; using System.Data.Common; -using System.Linq; using System.Text; -using System.Text.RegularExpressions; -using System.Threading; -using System.Threading.Tasks; using NHibernate.AdoNet.Util; -using NHibernate.Driver; +using NHibernate.Dialect; using NHibernate.Exceptions; using NHibernate.SqlCommand; -using NHibernate.SqlTypes; -using NHibernate.Util; namespace NHibernate.AdoNet { /// - /// Batcher for PostgreSQL that will batch UPDATE/INSERT/DELETE commands. + /// A generic batcher that will batch UPDATE/INSERT/DELETE commands by concatenating them with a semicolon. + /// Use this batcher only if there are no dedicated batchers in the given environment. /// - public partial class PostgreSQLClientBatchingBatcher : AbstractBatcher + public partial class GenericBatchingBatcher : AbstractBatcher { + private readonly int _maxNumberOfParameters = int.MaxValue; + private readonly BatchingCommandSet _currentBatch; private int _totalExpectedRowsAffected; - private PostgreSQLCommandSet _currentBatch; private StringBuilder _currentBatchCommandsLog; - public PostgreSQLClientBatchingBatcher(ConnectionManager connectionManager, IInterceptor interceptor) + public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor interceptor, char statementTerminator) : base(connectionManager, interceptor) { BatchSize = Factory.Settings.AdoBatchSize; - _currentBatch = CreateConfiguredBatch(); - - //we always create this, because we need to deal with a scenario in which - //the user change the logging configuration at runtime. Trying to put this - //behind an if(log.IsDebugEnabled) will cause a null reference exception - //at that point. + StatementTerminator = statementTerminator; + _currentBatch = new BatchingCommandSet(this); + // On Sql Server there is a limit of 2100 parameters, but 2 are reserved for sp_executesql, so + // we are able to use up to 2098 parameters. The same applies for sp_prepexec. + if (Factory.Dialect is MsSql2000Dialect) + { + _maxNumberOfParameters = 2098; + } + // We always create this, because we need to deal with a scenario in which + // the user change the logging configuration at runtime. Trying to put this + // behind an if(log.IsDebugEnabled) will cause a null reference exception + // at that point. _currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:"); } + public char StatementTerminator { get; } + public sealed override int BatchSize { get; set; } protected override int CountOfStatementsInCurrentBatch => _currentBatch.CountOfCommands; public override void AddToBatch(IExpectation expectation) { - _totalExpectedRowsAffected += expectation.ExpectedRowCount; var batchUpdate = CurrentCommand; + if (_currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) + { + ExecuteBatchWithTiming(batchUpdate); + } + _totalExpectedRowsAffected += expectation.ExpectedRowCount; Driver.AdjustCommand(batchUpdate); string lineWithParameters = null; var sqlStatementLogger = Factory.Settings.SqlStatementLogger; @@ -63,6 +69,7 @@ public override void AddToBatch(IExpectation expectation) { Log.Debug("Adding to batch:{0}", lineWithParameters); } + _currentBatch.Append(CurrentCommand.Parameters); if (_currentBatch.CountOfCommands >= BatchSize) @@ -73,6 +80,11 @@ public override void AddToBatch(IExpectation expectation) protected override void DoExecuteBatch(DbCommand ps) { + if (_currentBatch.CountOfCommands == 0) + { + Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0); + return; + } try { Log.Debug("Executing batch"); @@ -100,16 +112,10 @@ protected override void DoExecuteBatch(DbCommand ps) } } - private PostgreSQLCommandSet CreateConfiguredBatch() - { - return new PostgreSQLCommandSet(this); - } - private void ClearCurrentBatch() { - _currentBatch.Dispose(); + _currentBatch.Clear(); _totalExpectedRowsAffected = 0; - _currentBatch = CreateConfiguredBatch(); if (Factory.Settings.SqlStatementLogger.IsDebugEnabled) { @@ -120,47 +126,29 @@ private void ClearCurrentBatch() public override void CloseCommands() { base.CloseCommands(); - - try - { - ClearCurrentBatch(); - } - catch (Exception e) - { - // Prevent exceptions when clearing the batch from hiding any original exception - // (We do not know here if this batch closing occurs after a failure or not.) - Log.Warn(e, "Exception clearing batch"); - } + ClearCurrentBatch(); } protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - // Prevent exceptions when closing the batch from hiding any original exception - // (We do not know here if this batch closing occurs after a failure or not.) - try - { - _currentBatch.Dispose(); - } - catch (Exception e) - { - Log.Warn(e, "Exception closing batcher"); - } + _currentBatch.Dispose(); } - private partial class PostgreSQLCommandSet : IDisposable + private partial class BatchingCommandSet : IDisposable { - private int _currentParameterIndex; private DbCommand _batchCommand; - private readonly PostgreSQLClientBatchingBatcher _batcher; + private readonly GenericBatchingBatcher _batcher; - public PostgreSQLCommandSet(PostgreSQLClientBatchingBatcher batcher) + public BatchingCommandSet(GenericBatchingBatcher batcher) { _batcher = batcher; } public int CountOfCommands { get; private set; } + public int CountOfParameters { get; private set; } + public void Append(DbParameterCollection parameters) { if (_batchCommand == null) @@ -170,7 +158,7 @@ public void Append(DbParameterCollection parameters) _batcher.CurrentCommandSql, _batcher.CurrentCommandParameterTypes); UpdateCommandParameters(_batchCommand, parameters); - _currentParameterIndex = parameters.Count; + CountOfParameters = parameters.Count; } else { @@ -180,7 +168,7 @@ public void Append(DbParameterCollection parameters) PrepareSqlString(_batcher.CurrentCommandSql), _batcher.CurrentCommandParameterTypes); UpdateCommandParameters(command, parameters); - _batchCommand.CommandText += ";" + command.CommandText; + _batchCommand.CommandText += $"{_batcher.StatementTerminator}{command.CommandText}"; foreach (DbParameter parameter in command.Parameters) { _batchCommand.Parameters.Add(CopyParameter(_batchCommand, parameter)); @@ -203,14 +191,19 @@ public int ExecuteNonQuery() return _batchCommand.ExecuteNonQuery(); } - public void Dispose() + public void Clear() { _batchCommand?.Dispose(); _batchCommand = null; - _currentParameterIndex = 0; + CountOfParameters = 0; CountOfCommands = 0; } + public void Dispose() + { + Clear(); + } + private void UpdateCommandParameters(DbCommand command, DbParameterCollection parameters) { for (var i = 0; i < parameters.Count; i++) @@ -253,7 +246,7 @@ private SqlString PrepareSqlString(SqlString sql) { if (part is Parameter param) { - param.ParameterPosition = _currentParameterIndex++; + param.ParameterPosition = CountOfParameters++; } } return sql; diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs b/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs new file mode 100644 index 00000000000..4b6638acc33 --- /dev/null +++ b/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs @@ -0,0 +1,19 @@ +using NHibernate.Engine; + +namespace NHibernate.AdoNet +{ + public class GenericBatchingBatcherFactory: IBatcherFactory + { + private char _statementTerminator = ';'; + + public virtual IBatcher CreateBatcher(ConnectionManager connectionManager, IInterceptor interceptor) + { + return new GenericBatchingBatcher(connectionManager, interceptor, _statementTerminator); + } + + public void SetStatementTerminator(char value) + { + _statementTerminator = value; + } + } +} diff --git a/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs b/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs deleted file mode 100644 index 0aabe773782..00000000000 --- a/src/NHibernate/AdoNet/PostgreSQLClientBatchingBatcherFactory.cs +++ /dev/null @@ -1,12 +0,0 @@ -using NHibernate.Engine; - -namespace NHibernate.AdoNet -{ - public class PostgreSQLClientBatchingBatcherFactory: IBatcherFactory - { - public virtual IBatcher CreateBatcher(ConnectionManager connectionManager, IInterceptor interceptor) - { - return new PostgreSQLClientBatchingBatcher(connectionManager, interceptor); - } - } -} diff --git a/src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs similarity index 59% rename from src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs rename to src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs index 902099bb8c8..2cd71b57d56 100644 --- a/src/NHibernate/Async/AdoNet/PostgreSQLClientBatchingBatcher.cs +++ b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs @@ -9,70 +9,63 @@ using System; -using System.Collections.Generic; -using System.Data; using System.Data.Common; -using System.Linq; using System.Text; -using System.Text.RegularExpressions; -using System.Threading; -using System.Threading.Tasks; using NHibernate.AdoNet.Util; -using NHibernate.Driver; +using NHibernate.Dialect; using NHibernate.Exceptions; using NHibernate.SqlCommand; -using NHibernate.SqlTypes; -using NHibernate.Util; namespace NHibernate.AdoNet { - public partial class PostgreSQLClientBatchingBatcher : AbstractBatcher + using System.Threading.Tasks; + using System.Threading; + public partial class GenericBatchingBatcher : AbstractBatcher { - public override Task AddToBatchAsync(IExpectation expectation, CancellationToken cancellationToken) + public override async Task AddToBatchAsync(IExpectation expectation, CancellationToken cancellationToken) { - if (cancellationToken.IsCancellationRequested) + cancellationToken.ThrowIfCancellationRequested(); + var batchUpdate = CurrentCommand; + if (_currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) { - return Task.FromCanceled(cancellationToken); + await (ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken)).ConfigureAwait(false); } - try + _totalExpectedRowsAffected += expectation.ExpectedRowCount; + Driver.AdjustCommand(batchUpdate); + string lineWithParameters = null; + var sqlStatementLogger = Factory.Settings.SqlStatementLogger; + if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) { - _totalExpectedRowsAffected += expectation.ExpectedRowCount; - var batchUpdate = CurrentCommand; - Driver.AdjustCommand(batchUpdate); - string lineWithParameters = null; - var sqlStatementLogger = Factory.Settings.SqlStatementLogger; - if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) - { - lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchUpdate); - var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); - lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); - _currentBatchCommandsLog.Append("command ") + lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchUpdate); + var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); + lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); + _currentBatchCommandsLog.Append("command ") .Append(_currentBatch.CountOfCommands) .Append(":") .AppendLine(lineWithParameters); - } - if (Log.IsDebugEnabled()) - { - Log.Debug("Adding to batch:{0}", lineWithParameters); - } - _currentBatch.Append(CurrentCommand.Parameters); - - if (_currentBatch.CountOfCommands >= BatchSize) - { - return ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken); - } - return Task.CompletedTask; } - catch (Exception ex) + if (Log.IsDebugEnabled()) { - return Task.FromException(ex); + Log.Debug("Adding to batch:{0}", lineWithParameters); + } + + _currentBatch.Append(CurrentCommand.Parameters); + + if (_currentBatch.CountOfCommands >= BatchSize) + { + await (ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken)).ConfigureAwait(false); } } protected override async Task DoExecuteBatchAsync(DbCommand ps, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); + if (_currentBatch.CountOfCommands == 0) + { + Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0); + return; + } try { Log.Debug("Executing batch"); @@ -100,7 +93,7 @@ protected override async Task DoExecuteBatchAsync(DbCommand ps, CancellationToke } } - private partial class PostgreSQLCommandSet : IDisposable + private partial class BatchingCommandSet : IDisposable { public async Task ExecuteNonQueryAsync(CancellationToken cancellationToken) From 2375c6d082b8452856c7a9d0318d894a9b42904f Mon Sep 17 00:00:00 2001 From: maca88 Date: Sat, 3 Mar 2018 23:13:19 +0100 Subject: [PATCH 03/12] Removed unsupported clients for the generic batcher from tests --- .../Ado/GenericBatchingBatcherFixture.cs | 8 +++++++- .../Async/Ado/GenericBatchingBatcherFixture.cs | 8 +++++++- src/NHibernate/AdoNet/GenericBatchingBatcher.cs | 10 ++++++---- src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs | 3 --- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs index 0721de20865..964286b86c4 100644 --- a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs +++ b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs @@ -1,7 +1,6 @@ using System; using System.Collections; using System.Linq; -using log4net; using NHibernate.AdoNet; using NHibernate.Cfg; using NHibernate.Dialect; @@ -24,6 +23,13 @@ protected override void Configure(Configuration configuration) configuration.SetProperty(Environment.BatchSize, "1000"); } + protected override bool AppliesTo(Dialect.Dialect dialect) + { + return !(dialect is FirebirdDialect) && + !(dialect is Oracle8iDialect) && + !(dialect is MsSqlCeDialect); + } + [Test] public void MassiveInsertUpdateDeleteTest() { diff --git a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs index bb163fde4a2..7a4ecbc38ff 100644 --- a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs +++ b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs @@ -11,7 +11,6 @@ using System; using System.Collections; using System.Linq; -using log4net; using NHibernate.AdoNet; using NHibernate.Cfg; using NHibernate.Dialect; @@ -37,6 +36,13 @@ protected override void Configure(Configuration configuration) configuration.SetProperty(Environment.BatchSize, "1000"); } + protected override bool AppliesTo(Dialect.Dialect dialect) + { + return !(dialect is FirebirdDialect) && + !(dialect is Oracle8iDialect) && + !(dialect is MsSqlCeDialect); + } + [Test] public async Task MassiveInsertUpdateDeleteTestAsync() { diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index 65a8bf801d8..20aa512835b 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -10,7 +10,12 @@ namespace NHibernate.AdoNet { /// /// A generic batcher that will batch UPDATE/INSERT/DELETE commands by concatenating them with a semicolon. - /// Use this batcher only if there are no dedicated batchers in the given environment. + /// Use this batcher only if there are no dedicated batchers in the given environment. Unfortunately some + /// database clients do not support concatenating commands with a semicolon. Here are the known clients + /// that do not work with this batcher: + /// - FirebirdSql.Data.FirebirdClient + /// - Oracle.ManagedDataAccess + /// - System.Data.SqlServerCe /// public partial class GenericBatchingBatcher : AbstractBatcher { @@ -184,9 +189,6 @@ public int ExecuteNonQuery() { return 0; } - // Npgsql will correctly prepare a multi SQL statement even if the parameter names are different - // for each statement. Npgsql internally parses the query and omits parameter names when comparing two queries - // in order to prevent having multiple prepared statements for the same query. _batcher.Prepare(_batchCommand); return _batchCommand.ExecuteNonQuery(); } diff --git a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs index 2cd71b57d56..4a7351f4e3f 100644 --- a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs @@ -103,9 +103,6 @@ public async Task ExecuteNonQueryAsync(CancellationToken cancellationToken) { return 0; } - // Npgsql will correctly prepare a multi SQL statement even if the parameter names are different - // for each statement. Npgsql internally parses the query and omits parameter names when comparing two queries - // in order to prevent having multiple prepared statements for the same query. await (_batcher.PrepareAsync(_batchCommand, cancellationToken)).ConfigureAwait(false); return await (_batchCommand.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false); } From 2fd9868095b95354c531cc8d5e4555e24ed0979e Mon Sep 17 00:00:00 2001 From: maca88 Date: Sun, 4 Mar 2018 11:19:04 +0100 Subject: [PATCH 04/12] Fixed the max number of parameters so that we will never exceed the limit when sp_prepexec is used. --- src/NHibernate/AdoNet/GenericBatchingBatcher.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index 20aa512835b..b10f1568f40 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -31,10 +31,12 @@ public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor StatementTerminator = statementTerminator; _currentBatch = new BatchingCommandSet(this); // On Sql Server there is a limit of 2100 parameters, but 2 are reserved for sp_executesql, so - // we are able to use up to 2098 parameters. The same applies for sp_prepexec. + // we are able to use up to 2098 parameters. When sp_prepexec is used for preparing and executing + // statements then one more parameter is used. Set the max number of parameters to 2097 in order + // to ensure that we will never exceed the limit. if (Factory.Dialect is MsSql2000Dialect) { - _maxNumberOfParameters = 2098; + _maxNumberOfParameters = 2097; } // We always create this, because we need to deal with a scenario in which // the user change the logging configuration at runtime. Trying to put this From eb10f86f8c4817e7ec1d6698745006465126b7df Mon Sep 17 00:00:00 2001 From: maca88 Date: Sun, 4 Mar 2018 12:42:01 +0100 Subject: [PATCH 05/12] Reuse parameters instead of copying them --- .../AdoNet/GenericBatchingBatcher.cs | 23 ++++--------------- 1 file changed, 4 insertions(+), 19 deletions(-) diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index b10f1568f40..db0618ab94f 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -176,9 +176,11 @@ public void Append(DbParameterCollection parameters) _batcher.CurrentCommandParameterTypes); UpdateCommandParameters(command, parameters); _batchCommand.CommandText += $"{_batcher.StatementTerminator}{command.CommandText}"; - foreach (DbParameter parameter in command.Parameters) + while (command.Parameters.Count > 0) { - _batchCommand.Parameters.Add(CopyParameter(_batchCommand, parameter)); + var pram = command.Parameters[0]; + command.Parameters.RemoveAt(0); + _batchCommand.Parameters.Add(pram); } command.Dispose(); } @@ -222,23 +224,6 @@ private void UpdateCommandParameters(DbCommand command, DbParameterCollection pa } } - private DbParameter CopyParameter(DbCommand command, DbParameter parameter) - { - var copy = command.CreateParameter(); - copy.DbType = parameter.DbType; - copy.IsNullable = parameter.IsNullable; - copy.ParameterName = parameter.ParameterName; - copy.Value = parameter.Value; - copy.Direction = parameter.Direction; - copy.Precision = parameter.Precision; - copy.Scale = parameter.Scale; - copy.Size = parameter.Size; - copy.SourceVersion = parameter.SourceVersion; - copy.SourceColumn = parameter.SourceColumn; - copy.SourceColumnNullMapping = parameter.SourceColumnNullMapping; - return copy; - } - private SqlString PrepareSqlString(SqlString sql) { if (_batchCommand == null) From 48e7c010289f76137f5f1616704f03fcafcdbc8b Mon Sep 17 00:00:00 2001 From: maca88 Date: Tue, 6 Mar 2018 20:35:21 +0100 Subject: [PATCH 06/12] Added MaxNumberOfParameters to Dialect --- src/NHibernate/AdoNet/GenericBatchingBatcher.cs | 15 +++++---------- src/NHibernate/Dialect/Dialect.cs | 5 +++++ src/NHibernate/Dialect/MsSql2000Dialect.cs | 7 +++++++ 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index db0618ab94f..e36dc8a2998 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -19,7 +19,7 @@ namespace NHibernate.AdoNet /// public partial class GenericBatchingBatcher : AbstractBatcher { - private readonly int _maxNumberOfParameters = int.MaxValue; + private readonly int? _maxNumberOfParameters; private readonly BatchingCommandSet _currentBatch; private int _totalExpectedRowsAffected; private StringBuilder _currentBatchCommandsLog; @@ -30,14 +30,8 @@ public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor BatchSize = Factory.Settings.AdoBatchSize; StatementTerminator = statementTerminator; _currentBatch = new BatchingCommandSet(this); - // On Sql Server there is a limit of 2100 parameters, but 2 are reserved for sp_executesql, so - // we are able to use up to 2098 parameters. When sp_prepexec is used for preparing and executing - // statements then one more parameter is used. Set the max number of parameters to 2097 in order - // to ensure that we will never exceed the limit. - if (Factory.Dialect is MsSql2000Dialect) - { - _maxNumberOfParameters = 2097; - } + _maxNumberOfParameters = Factory.Dialect.MaxNumberOfParameters; + // We always create this, because we need to deal with a scenario in which // the user change the logging configuration at runtime. Trying to put this // behind an if(log.IsDebugEnabled) will cause a null reference exception @@ -54,7 +48,8 @@ public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor public override void AddToBatch(IExpectation expectation) { var batchUpdate = CurrentCommand; - if (_currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) + if (_maxNumberOfParameters.HasValue && + _currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) { ExecuteBatchWithTiming(batchUpdate); } diff --git a/src/NHibernate/Dialect/Dialect.cs b/src/NHibernate/Dialect/Dialect.cs index 53a31c43e15..f42c8545ef1 100644 --- a/src/NHibernate/Dialect/Dialect.cs +++ b/src/NHibernate/Dialect/Dialect.cs @@ -2446,6 +2446,11 @@ public virtual string LowercaseFunction /// public virtual int MaxAliasLength => 18; + /// + /// The maximum number of parameters allowed in a query. + /// + public virtual int? MaxNumberOfParameters { get; } = null; + /// /// The syntax used to add a column to a table. Note this is deprecated /// diff --git a/src/NHibernate/Dialect/MsSql2000Dialect.cs b/src/NHibernate/Dialect/MsSql2000Dialect.cs index 050d65e142d..6ec913a05f4 100644 --- a/src/NHibernate/Dialect/MsSql2000Dialect.cs +++ b/src/NHibernate/Dialect/MsSql2000Dialect.cs @@ -716,6 +716,13 @@ public override bool SupportsSqlBatches /// public override int MaxAliasLength => 30; + /// + /// On SQL Server there is a limit of 2100 parameters, but two are reserved for sp_executesql + /// and three for sp_prepexec (used when preparing is enabled). Set the number to 2097 + /// as the worst case scenario. + /// + public override int? MaxNumberOfParameters => 2097; + #region Overridden informational metadata public override bool SupportsEmptyInList => false; From 9ff621cf6bf2e66a576fbb0f067cc1303742405d Mon Sep 17 00:00:00 2001 From: maca88 Date: Tue, 6 Mar 2018 22:22:36 +0100 Subject: [PATCH 07/12] Redesigned the BatchingCommandSet for better performance --- .../AdoNet/GenericBatchingBatcher.cs | 111 ++++++++++-------- .../Async/AdoNet/GenericBatchingBatcher.cs | 27 ++++- 2 files changed, 83 insertions(+), 55 deletions(-) diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index e36dc8a2998..178b450ed67 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -1,8 +1,9 @@ using System; +using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Text; using NHibernate.AdoNet.Util; -using NHibernate.Dialect; using NHibernate.Exceptions; using NHibernate.SqlCommand; @@ -139,92 +140,88 @@ protected override void Dispose(bool isDisposing) private partial class BatchingCommandSet : IDisposable { - private DbCommand _batchCommand; private readonly GenericBatchingBatcher _batcher; + private readonly SqlStringBuilder _sql = new SqlStringBuilder(); + private readonly List _sqlTypes = new List(); + private readonly List _parameters = new List(); + private CommandType _commandType; + + private class BatchParameter + { + public ParameterDirection Direction { get; set; } + + public byte Precision { get; set; } + + public byte Scale { get; set; } + + public int Size { get; set; } + + public object Value { get; set; } + } public BatchingCommandSet(GenericBatchingBatcher batcher) { _batcher = batcher; } - + public int CountOfCommands { get; private set; } public int CountOfParameters { get; private set; } public void Append(DbParameterCollection parameters) { - if (_batchCommand == null) + if (CountOfCommands > 0) { - _batchCommand = _batcher.Driver.GenerateCommand( - _batcher.CurrentCommand.CommandType, - _batcher.CurrentCommandSql, - _batcher.CurrentCommandParameterTypes); - UpdateCommandParameters(_batchCommand, parameters); - CountOfParameters = parameters.Count; + _sql.Add(_batcher.StatementTerminator.ToString()); } else { - // We need to create a new command with different parameter names to avoid duplicates - var command = _batcher.Driver.GenerateCommand( - _batcher.CurrentCommand.CommandType, - PrepareSqlString(_batcher.CurrentCommandSql), - _batcher.CurrentCommandParameterTypes); - UpdateCommandParameters(command, parameters); - _batchCommand.CommandText += $"{_batcher.StatementTerminator}{command.CommandText}"; - while (command.Parameters.Count > 0) + _commandType = _batcher.CurrentCommand.CommandType; + } + _sql.Add(PrepareSqlString(_batcher.CurrentCommandSql)); + _sqlTypes.AddRange(_batcher.CurrentCommandParameterTypes); + + foreach (DbParameter parameter in parameters) + { + _parameters.Add(new BatchParameter { - var pram = command.Parameters[0]; - command.Parameters.RemoveAt(0); - _batchCommand.Parameters.Add(pram); - } - command.Dispose(); + Direction = parameter.Direction, + Precision = parameter.Precision, + Scale = parameter.Scale, + Size = parameter.Size, + Value = parameter.Value + }); } CountOfCommands++; } public int ExecuteNonQuery() { - if (_batchCommand == null) + if (CountOfCommands == 0) { return 0; } - _batcher.Prepare(_batchCommand); - return _batchCommand.ExecuteNonQuery(); - } - - public void Clear() - { - _batchCommand?.Dispose(); - _batchCommand = null; - CountOfParameters = 0; - CountOfCommands = 0; - } - - public void Dispose() - { - Clear(); - } - - private void UpdateCommandParameters(DbCommand command, DbParameterCollection parameters) - { - for (var i = 0; i < parameters.Count; i++) + var batcherCommand = _batcher.Driver.GenerateCommand( + _commandType, + _sql.ToSqlString(), + _sqlTypes.ToArray() + ); + for (var i = 0; i < _parameters.Count; i++) { - var parameter = parameters[i]; - var cmdParam = command.Parameters[i]; + var parameter = _parameters[i]; + var cmdParam = batcherCommand.Parameters[i]; cmdParam.Value = parameter.Value; cmdParam.Direction = parameter.Direction; cmdParam.Precision = parameter.Precision; cmdParam.Scale = parameter.Scale; cmdParam.Size = parameter.Size; } + _batcher.Prepare(batcherCommand); + return batcherCommand.ExecuteNonQuery(); } private SqlString PrepareSqlString(SqlString sql) { - if (_batchCommand == null) - { - return sql; - } sql = sql.Copy(); foreach (var part in sql) { @@ -235,6 +232,20 @@ private SqlString PrepareSqlString(SqlString sql) } return sql; } + + public void Clear() + { + CountOfParameters = 0; + CountOfCommands = 0; + _sql.Clear(); + _sqlTypes.Clear(); + _parameters.Clear(); + } + + public void Dispose() + { + Clear(); + } } } } diff --git a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs index 4a7351f4e3f..c6c6be6706c 100644 --- a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs @@ -9,10 +9,11 @@ using System; +using System.Collections.Generic; +using System.Data; using System.Data.Common; using System.Text; using NHibernate.AdoNet.Util; -using NHibernate.Dialect; using NHibernate.Exceptions; using NHibernate.SqlCommand; @@ -27,7 +28,8 @@ public override async Task AddToBatchAsync(IExpectation expectation, Cancellatio { cancellationToken.ThrowIfCancellationRequested(); var batchUpdate = CurrentCommand; - if (_currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) + if (_maxNumberOfParameters.HasValue && + _currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) { await (ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken)).ConfigureAwait(false); } @@ -99,12 +101,27 @@ private partial class BatchingCommandSet : IDisposable public async Task ExecuteNonQueryAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - if (_batchCommand == null) + if (CountOfCommands == 0) { return 0; } - await (_batcher.PrepareAsync(_batchCommand, cancellationToken)).ConfigureAwait(false); - return await (_batchCommand.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false); + var batcherCommand = _batcher.Driver.GenerateCommand( + _commandType, + _sql.ToSqlString(), + _sqlTypes.ToArray() + ); + for (var i = 0; i < _parameters.Count; i++) + { + var parameter = _parameters[i]; + var cmdParam = batcherCommand.Parameters[i]; + cmdParam.Value = parameter.Value; + cmdParam.Direction = parameter.Direction; + cmdParam.Precision = parameter.Precision; + cmdParam.Scale = parameter.Scale; + cmdParam.Size = parameter.Size; + } + await (_batcher.PrepareAsync(batcherCommand, cancellationToken)).ConfigureAwait(false); + return await (batcherCommand.ExecuteNonQueryAsync(cancellationToken)).ConfigureAwait(false); } } } From 11b45e78f250e9970e9fd020b93e5c432b24dbc5 Mon Sep 17 00:00:00 2001 From: maca88 Date: Wed, 7 Mar 2018 20:46:20 +0100 Subject: [PATCH 08/12] Reverted the changes in DriverBase and simplified BatchingCommandSet --- .../AdoNet/GenericBatchingBatcher.cs | 26 ++++--------------- .../Async/AdoNet/GenericBatchingBatcher.cs | 2 +- src/NHibernate/Driver/DriverBase.cs | 22 +++++----------- 3 files changed, 12 insertions(+), 38 deletions(-) diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index 178b450ed67..b31998af077 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -135,10 +135,10 @@ public override void CloseCommands() protected override void Dispose(bool isDisposing) { base.Dispose(isDisposing); - _currentBatch.Dispose(); + _currentBatch.Clear(); } - private partial class BatchingCommandSet : IDisposable + private partial class BatchingCommandSet { private readonly GenericBatchingBatcher _batcher; private readonly SqlStringBuilder _sql = new SqlStringBuilder(); @@ -178,7 +178,8 @@ public void Append(DbParameterCollection parameters) { _commandType = _batcher.CurrentCommand.CommandType; } - _sql.Add(PrepareSqlString(_batcher.CurrentCommandSql)); + + _sql.Add(_batcher.CurrentCommandSql.Copy()); _sqlTypes.AddRange(_batcher.CurrentCommandParameterTypes); foreach (DbParameter parameter in parameters) @@ -193,6 +194,7 @@ public void Append(DbParameterCollection parameters) }); } CountOfCommands++; + CountOfParameters += parameters.Count; } public int ExecuteNonQuery() @@ -220,19 +222,6 @@ public int ExecuteNonQuery() return batcherCommand.ExecuteNonQuery(); } - private SqlString PrepareSqlString(SqlString sql) - { - sql = sql.Copy(); - foreach (var part in sql) - { - if (part is Parameter param) - { - param.ParameterPosition = CountOfParameters++; - } - } - return sql; - } - public void Clear() { CountOfParameters = 0; @@ -241,11 +230,6 @@ public void Clear() _sqlTypes.Clear(); _parameters.Clear(); } - - public void Dispose() - { - Clear(); - } } } } diff --git a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs index c6c6be6706c..8d1bcab437b 100644 --- a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs @@ -95,7 +95,7 @@ protected override async Task DoExecuteBatchAsync(DbCommand ps, CancellationToke } } - private partial class BatchingCommandSet : IDisposable + private partial class BatchingCommandSet { public async Task ExecuteNonQueryAsync(CancellationToken cancellationToken) diff --git a/src/NHibernate/Driver/DriverBase.cs b/src/NHibernate/Driver/DriverBase.cs index 1a858d650aa..c647ec0ff82 100644 --- a/src/NHibernate/Driver/DriverBase.cs +++ b/src/NHibernate/Driver/DriverBase.cs @@ -133,7 +133,7 @@ public virtual DbCommand GenerateCommand(CommandType type, SqlString sqlString, SetCommandTimeout(cmd); SetCommandText(cmd, sqlString); - SetCommandParameters(cmd, parameterTypes, sqlString.GetParameters().ToArray()); + SetCommandParameters(cmd, parameterTypes); return cmd; } @@ -156,9 +156,9 @@ protected virtual void SetCommandTimeout(DbCommand cmd) } } - private static string ToParameterName(int index, string prefix = null) + private static string ToParameterName(int index) { - return string.IsNullOrEmpty(prefix) ? "p" + index : prefix + index; + return "p" + index; } string ISqlParameterFormatter.GetParameterName(int index) @@ -178,23 +178,13 @@ protected virtual SqlStringFormatter GetSqlStringFormatter() return new SqlStringFormatter(this, ";"); } - private void SetCommandParameters(DbCommand cmd, SqlType[] sqlTypes, Parameter[] parameters) + private void SetCommandParameters(DbCommand cmd, SqlType[] sqlTypes) { - var usedParameters = new HashSet(); - for (var i = 0; i < sqlTypes.Length; i++) + for (int i = 0; i < sqlTypes.Length; i++) { - var parameter = parameters.Length > i ? parameters[i] : null; - var position = parameter?.ParameterPosition ?? i; - // Use a different prefix when two or more parameters have the same position in - // order to prevert a name collision. When this happens, only one parameter will - // be used while the others are added to prevent an ArgumentOutOfRangeException - // being thrown when NamedParameterSpecification.Bind method is called. - var paramName = usedParameters.Contains(position) - ? ToParameterName(i, "fp") - : ToParameterName(position); + string paramName = ToParameterName(i); var dbParam = GenerateParameter(cmd, paramName, sqlTypes[i]); cmd.Parameters.Add(dbParam); - usedParameters.Add(position); } } From e228b2b126ab7e87ca1e517c266a8f37a6600dd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Thu, 8 Mar 2018 17:19:31 +0100 Subject: [PATCH 09/12] Add a performance check of the generic batcher. --- .../Ado/GenericBatchingBatcherFixture.cs | 36 +++++++++++++++++++ .../Ado/GenericBatchingBatcherFixture.cs | 36 +++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs index 964286b86c4..d3c104d2bcd 100644 --- a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs +++ b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Diagnostics; using System.Linq; using NHibernate.AdoNet; using NHibernate.Cfg; @@ -61,6 +62,41 @@ public void BatchSizeTest() Cleanup(); } + // Demonstrates a 50% performance gain with SQL-Server, around 40% for PostgreSQL, + // around 15% for MySql, but around 200% performance loss for SQLite. + // (Tested with databases on same machine for all cases.) + [Theory, Explicit("This is a performance test, to be checked manually.")] + public void MassivePerformanceTest(bool batched) + { + if (batched) + { + // Bring down batch size to a reasonnable value, otherwise performances are worsen. + cfg.SetProperty(Environment.BatchSize, "50"); + } + else + { + cfg.SetProperty(Environment.BatchStrategy, typeof(NonBatchingBatcherFactory).AssemblyQualifiedName); + cfg.Properties.Remove(Environment.BatchSize); + } + RebuildSessionFactory(); + + try + { + // Warm up + MassiveInsertUpdateDeleteTest(); + + var chrono = new Stopwatch(); + chrono.Start(); + MassiveInsertUpdateDeleteTest(); + Console.WriteLine($"Elapsed time: {chrono.Elapsed}"); + } + finally + { + Configure(cfg); + RebuildSessionFactory(); + } + } + private void BatchInsert(int totalRecords) { Sfi.Statistics.Clear(); diff --git a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs index 7a4ecbc38ff..0dd412cf594 100644 --- a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs +++ b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs @@ -10,6 +10,7 @@ using System; using System.Collections; +using System.Diagnostics; using System.Linq; using NHibernate.AdoNet; using NHibernate.Cfg; @@ -74,6 +75,41 @@ public async Task BatchSizeTestAsync() await (CleanupAsync()); } + // Demonstrates a 50% performance gain with SQL-Server, around 40% for PostgreSQL, + // around 15% for MySql, but around 200% performance loss for SQLite. + // (Tested with databases on same machine for all cases.) + [Theory, Explicit("This is a performance test, to be checked manually.")] + public async Task MassivePerformanceTestAsync(bool batched) + { + if (batched) + { + // Bring down batch size to a reasonnable value, otherwise performances are worsen. + cfg.SetProperty(Environment.BatchSize, "50"); + } + else + { + cfg.SetProperty(Environment.BatchStrategy, typeof(NonBatchingBatcherFactory).AssemblyQualifiedName); + cfg.Properties.Remove(Environment.BatchSize); + } + RebuildSessionFactory(); + + try + { + // Warm up + await (MassiveInsertUpdateDeleteTestAsync()); + + var chrono = new Stopwatch(); + chrono.Start(); + await (MassiveInsertUpdateDeleteTestAsync()); + Console.WriteLine($"Elapsed time: {chrono.Elapsed}"); + } + finally + { + Configure(cfg); + RebuildSessionFactory(); + } + } + private async Task BatchInsertAsync(int totalRecords, CancellationToken cancellationToken = default(CancellationToken)) { Sfi.Statistics.Clear(); From aca8e3961dd778d0b7a3c297d37f0541cf040f31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= Date: Thu, 8 Mar 2018 17:39:10 +0100 Subject: [PATCH 10/12] Set the generic batcher as default batcher for drivers benefiting of it. --- .../AdoNet/IEmbeddedBatcherFactoryProvider.cs | 16 ++++++++-------- src/NHibernate/Driver/NpgsqlDriver.cs | 5 ++++- src/NHibernate/Driver/SqlClientDriver.cs | 6 +++++- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/src/NHibernate/AdoNet/IEmbeddedBatcherFactoryProvider.cs b/src/NHibernate/AdoNet/IEmbeddedBatcherFactoryProvider.cs index f01b2048e85..42d96215163 100644 --- a/src/NHibernate/AdoNet/IEmbeddedBatcherFactoryProvider.cs +++ b/src/NHibernate/AdoNet/IEmbeddedBatcherFactoryProvider.cs @@ -1,18 +1,18 @@ namespace NHibernate.AdoNet { /// - /// Provide the class of according to the configuration - /// and the capabilities of the driver. + /// Provides a default class. /// /// - /// By default, .Net doesn't have any batching capabilities, drivers that does have - /// batching support. - /// The BatcherFactory trough session-factory configuration section. - /// This interface was added in NHibernate for backdraw compatibility to have the ability - /// to specify a default for a specific . + /// This interface allows to specify a default for a specific + /// . The configuration setting + /// takes precedence over BatcherFactoryClass. /// public interface IEmbeddedBatcherFactoryProvider { + /// + /// The class type. + /// System.Type BatcherFactoryClass { get;} } -} \ No newline at end of file +} diff --git a/src/NHibernate/Driver/NpgsqlDriver.cs b/src/NHibernate/Driver/NpgsqlDriver.cs index 1f0701c30ad..5bc725494c9 100644 --- a/src/NHibernate/Driver/NpgsqlDriver.cs +++ b/src/NHibernate/Driver/NpgsqlDriver.cs @@ -1,5 +1,6 @@ using System.Data; using System.Data.Common; +using NHibernate.AdoNet; namespace NHibernate.Driver { @@ -24,7 +25,7 @@ namespace NHibernate.Driver /// http://pgfoundry.org/projects/npgsql. ///

/// - public class NpgsqlDriver : ReflectionBasedDriver + public class NpgsqlDriver : ReflectionBasedDriver, IEmbeddedBatcherFactoryProvider { /// /// Initializes a new instance of the class. @@ -92,5 +93,7 @@ protected override void InitializeParameter(DbParameter dbParam, string name, Sq public override bool RequiresTimeSpanForTime => (DriverVersion?.Major ?? 3) >= 3; public override bool HasDelayedDistributedTransactionCompletion => true; + + System.Type IEmbeddedBatcherFactoryProvider.BatcherFactoryClass => typeof(GenericBatchingBatcherFactory); } } diff --git a/src/NHibernate/Driver/SqlClientDriver.cs b/src/NHibernate/Driver/SqlClientDriver.cs index b9849ae7b59..328de551fdf 100644 --- a/src/NHibernate/Driver/SqlClientDriver.cs +++ b/src/NHibernate/Driver/SqlClientDriver.cs @@ -2,7 +2,9 @@ using System.Collections.Generic; using System.Data; using System.Data.Common; +#if NETFX using System.Data.SqlClient; +#endif using NHibernate.AdoNet; using NHibernate.Engine; using NHibernate.SqlTypes; @@ -16,7 +18,7 @@ public class SqlClientDriver #if NETFX : DriverBase, IEmbeddedBatcherFactoryProvider #else - : ReflectionBasedDriver + : ReflectionBasedDriver, IEmbeddedBatcherFactoryProvider #endif { public const int MaxSizeForAnsiClob = 2147483647; // int.MaxValue @@ -50,6 +52,8 @@ public SqlClientDriver() : base("System.Data.SqlClient", "System.Data.SqlClient.SqlConnection", "System.Data.SqlClient.SqlCommand") { } + + System.Type IEmbeddedBatcherFactoryProvider.BatcherFactoryClass => typeof(GenericBatchingBatcherFactory); #else /// /// Creates an uninitialized object for From ed97fa9d16a03cd1d0304734528afb4a912fd8e9 Mon Sep 17 00:00:00 2001 From: maca88 Date: Thu, 8 Mar 2018 17:44:17 +0100 Subject: [PATCH 11/12] Assert corrected and used lambda for getter --- src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs | 2 +- src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs | 2 +- src/NHibernate/Dialect/Dialect.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs index 964286b86c4..ea68c24a1a3 100644 --- a/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs +++ b/src/NHibernate.Test/Ado/GenericBatchingBatcherFixture.cs @@ -56,7 +56,7 @@ public void BatchSizeTest() tx.Commit(); var log = sqlLog.GetWholeLog(); - Assert.That(4, Is.EqualTo(FindAllOccurrences(log, "Batch commands:"))); + Assert.That(FindAllOccurrences(log, "Batch commands:"), Is.EqualTo(4)); } Cleanup(); } diff --git a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs index 7a4ecbc38ff..3a79e042e8f 100644 --- a/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs +++ b/src/NHibernate.Test/Async/Ado/GenericBatchingBatcherFixture.cs @@ -69,7 +69,7 @@ public async Task BatchSizeTestAsync() await (tx.CommitAsync()); var log = sqlLog.GetWholeLog(); - Assert.That(4, Is.EqualTo(FindAllOccurrences(log, "Batch commands:"))); + Assert.That(FindAllOccurrences(log, "Batch commands:"), Is.EqualTo(4)); } await (CleanupAsync()); } diff --git a/src/NHibernate/Dialect/Dialect.cs b/src/NHibernate/Dialect/Dialect.cs index f42c8545ef1..1b637174718 100644 --- a/src/NHibernate/Dialect/Dialect.cs +++ b/src/NHibernate/Dialect/Dialect.cs @@ -2449,7 +2449,7 @@ public virtual string LowercaseFunction /// /// The maximum number of parameters allowed in a query. /// - public virtual int? MaxNumberOfParameters { get; } = null; + public virtual int? MaxNumberOfParameters => null; /// /// The syntax used to add a column to a table. Note this is deprecated From 9232cbb9c67af8ec7cbdb95700cc551eba9e96fa Mon Sep 17 00:00:00 2001 From: maca88 Date: Fri, 9 Mar 2018 17:06:08 +0100 Subject: [PATCH 12/12] Moved statement terminator setting into dialect and done some minor refactoring. --- .../AdoNet/GenericBatchingBatcher.cs | 63 ++++++++++--------- .../AdoNet/GenericBatchingBatcherFactory.cs | 9 +-- .../Async/AdoNet/GenericBatchingBatcher.cs | 30 +++------ src/NHibernate/Dialect/Dialect.cs | 5 ++ .../Driver/BasicResultSetsCommand.cs | 4 +- 5 files changed, 49 insertions(+), 62 deletions(-) diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs index b31998af077..26074117c4a 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcher.cs @@ -25,12 +25,11 @@ public partial class GenericBatchingBatcher : AbstractBatcher private int _totalExpectedRowsAffected; private StringBuilder _currentBatchCommandsLog; - public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor interceptor, char statementTerminator) + public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor interceptor) : base(connectionManager, interceptor) { BatchSize = Factory.Settings.AdoBatchSize; - StatementTerminator = statementTerminator; - _currentBatch = new BatchingCommandSet(this); + _currentBatch = new BatchingCommandSet(this, Factory.Dialect.StatementTerminator); _maxNumberOfParameters = Factory.Dialect.MaxNumberOfParameters; // We always create this, because we need to deal with a scenario in which @@ -40,44 +39,26 @@ public GenericBatchingBatcher(ConnectionManager connectionManager, IInterceptor _currentBatchCommandsLog = new StringBuilder().AppendLine("Batch commands:"); } - public char StatementTerminator { get; } - public sealed override int BatchSize { get; set; } protected override int CountOfStatementsInCurrentBatch => _currentBatch.CountOfCommands; public override void AddToBatch(IExpectation expectation) { - var batchUpdate = CurrentCommand; + var batchCommand = CurrentCommand; if (_maxNumberOfParameters.HasValue && - _currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) + _currentBatch.CountOfParameters + batchCommand.Parameters.Count > _maxNumberOfParameters) { - ExecuteBatchWithTiming(batchUpdate); + ExecuteBatchWithTiming(batchCommand); } _totalExpectedRowsAffected += expectation.ExpectedRowCount; - Driver.AdjustCommand(batchUpdate); - string lineWithParameters = null; - var sqlStatementLogger = Factory.Settings.SqlStatementLogger; - if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) - { - lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchUpdate); - var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); - lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); - _currentBatchCommandsLog.Append("command ") - .Append(_currentBatch.CountOfCommands) - .Append(":") - .AppendLine(lineWithParameters); - } - if (Log.IsDebugEnabled()) - { - Log.Debug("Adding to batch:{0}", lineWithParameters); - } - - _currentBatch.Append(CurrentCommand.Parameters); + Driver.AdjustCommand(batchCommand); + LogBatchCommand(batchCommand); + _currentBatch.Append(batchCommand.Parameters); if (_currentBatch.CountOfCommands >= BatchSize) { - ExecuteBatchWithTiming(batchUpdate); + ExecuteBatchWithTiming(batchCommand); } } @@ -115,6 +96,26 @@ protected override void DoExecuteBatch(DbCommand ps) } } + private void LogBatchCommand(DbCommand batchCommand) + { + string lineWithParameters = null; + var sqlStatementLogger = Factory.Settings.SqlStatementLogger; + if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) + { + lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchCommand); + var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); + lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); + _currentBatchCommandsLog.Append("command ") + .Append(_currentBatch.CountOfCommands) + .Append(":") + .AppendLine(lineWithParameters); + } + if (Log.IsDebugEnabled()) + { + Log.Debug("Adding to batch:{0}", lineWithParameters); + } + } + private void ClearCurrentBatch() { _currentBatch.Clear(); @@ -140,6 +141,7 @@ protected override void Dispose(bool isDisposing) private partial class BatchingCommandSet { + private readonly string _statementTerminator; private readonly GenericBatchingBatcher _batcher; private readonly SqlStringBuilder _sql = new SqlStringBuilder(); private readonly List _sqlTypes = new List(); @@ -159,9 +161,10 @@ private class BatchParameter public object Value { get; set; } } - public BatchingCommandSet(GenericBatchingBatcher batcher) + public BatchingCommandSet(GenericBatchingBatcher batcher, char statementTerminator) { _batcher = batcher; + _statementTerminator = statementTerminator.ToString(); } public int CountOfCommands { get; private set; } @@ -172,7 +175,7 @@ public void Append(DbParameterCollection parameters) { if (CountOfCommands > 0) { - _sql.Add(_batcher.StatementTerminator.ToString()); + _sql.Add(_statementTerminator); } else { diff --git a/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs b/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs index 4b6638acc33..69a127e2ec1 100644 --- a/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs +++ b/src/NHibernate/AdoNet/GenericBatchingBatcherFactory.cs @@ -4,16 +4,9 @@ namespace NHibernate.AdoNet { public class GenericBatchingBatcherFactory: IBatcherFactory { - private char _statementTerminator = ';'; - public virtual IBatcher CreateBatcher(ConnectionManager connectionManager, IInterceptor interceptor) { - return new GenericBatchingBatcher(connectionManager, interceptor, _statementTerminator); - } - - public void SetStatementTerminator(char value) - { - _statementTerminator = value; + return new GenericBatchingBatcher(connectionManager, interceptor); } } } diff --git a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs index 8d1bcab437b..47ebe2ed12c 100644 --- a/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs +++ b/src/NHibernate/Async/AdoNet/GenericBatchingBatcher.cs @@ -27,36 +27,20 @@ public partial class GenericBatchingBatcher : AbstractBatcher public override async Task AddToBatchAsync(IExpectation expectation, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - var batchUpdate = CurrentCommand; + var batchCommand = CurrentCommand; if (_maxNumberOfParameters.HasValue && - _currentBatch.CountOfParameters + CurrentCommand.Parameters.Count > _maxNumberOfParameters) + _currentBatch.CountOfParameters + batchCommand.Parameters.Count > _maxNumberOfParameters) { - await (ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken)).ConfigureAwait(false); + await (ExecuteBatchWithTimingAsync(batchCommand, cancellationToken)).ConfigureAwait(false); } _totalExpectedRowsAffected += expectation.ExpectedRowCount; - Driver.AdjustCommand(batchUpdate); - string lineWithParameters = null; - var sqlStatementLogger = Factory.Settings.SqlStatementLogger; - if (sqlStatementLogger.IsDebugEnabled || Log.IsDebugEnabled()) - { - lineWithParameters = sqlStatementLogger.GetCommandLineWithParameters(batchUpdate); - var formatStyle = sqlStatementLogger.DetermineActualStyle(FormatStyle.Basic); - lineWithParameters = formatStyle.Formatter.Format(lineWithParameters); - _currentBatchCommandsLog.Append("command ") - .Append(_currentBatch.CountOfCommands) - .Append(":") - .AppendLine(lineWithParameters); - } - if (Log.IsDebugEnabled()) - { - Log.Debug("Adding to batch:{0}", lineWithParameters); - } - - _currentBatch.Append(CurrentCommand.Parameters); + Driver.AdjustCommand(batchCommand); + LogBatchCommand(batchCommand); + _currentBatch.Append(batchCommand.Parameters); if (_currentBatch.CountOfCommands >= BatchSize) { - await (ExecuteBatchWithTimingAsync(batchUpdate, cancellationToken)).ConfigureAwait(false); + await (ExecuteBatchWithTimingAsync(batchCommand, cancellationToken)).ConfigureAwait(false); } } diff --git a/src/NHibernate/Dialect/Dialect.cs b/src/NHibernate/Dialect/Dialect.cs index 1b637174718..1483cdd4f08 100644 --- a/src/NHibernate/Dialect/Dialect.cs +++ b/src/NHibernate/Dialect/Dialect.cs @@ -2451,6 +2451,11 @@ public virtual string LowercaseFunction /// public virtual int? MaxNumberOfParameters => null; + /// + /// The character used to terminate a SQL statement. + /// + public virtual char StatementTerminator => ';'; + /// /// The syntax used to add a column to a table. Note this is deprecated /// diff --git a/src/NHibernate/Driver/BasicResultSetsCommand.cs b/src/NHibernate/Driver/BasicResultSetsCommand.cs index f03c891d75d..36e6a8a8485 100644 --- a/src/NHibernate/Driver/BasicResultSetsCommand.cs +++ b/src/NHibernate/Driver/BasicResultSetsCommand.cs @@ -16,11 +16,13 @@ public partial class BasicResultSetsCommand: IResultSetsCommand { private static readonly INHibernateLogger log = NHibernateLogger.For(typeof(BasicResultSetsCommand)); private SqlString sqlString = SqlString.Empty; + private readonly string _statementTerminator; public BasicResultSetsCommand(ISessionImplementor session) { Commands = new List(); Session = session; + _statementTerminator = session.Factory.Dialect.StatementTerminator.ToString(); } protected List Commands { get; private set; } @@ -30,7 +32,7 @@ public BasicResultSetsCommand(ISessionImplementor session) public virtual void Append(ISqlCommand command) { Commands.Add(command); - sqlString = sqlString.Append(command.Query).Append(";").Append(Environment.NewLine); + sqlString = sqlString.Append(command.Query).Append(_statementTerminator).Append(Environment.NewLine); } public bool HasQueries