diff --git a/src/NHibernate.Test/Ado/BatcherFixture.cs b/src/NHibernate.Test/Ado/BatcherFixture.cs index ff3bfd7aace..6e26081cce6 100644 --- a/src/NHibernate.Test/Ado/BatcherFixture.cs +++ b/src/NHibernate.Test/Ado/BatcherFixture.cs @@ -1,3 +1,4 @@ +using System.Linq; using NHibernate.AdoNet; using NHibernate.Cfg; using NUnit.Framework; @@ -26,11 +27,13 @@ protected override string[] Mappings get { return new[] { "Ado.VerySimple.hbm.xml", "Ado.AlmostSimple.hbm.xml" }; } } + private const int BatchSize = 10; + protected override void Configure(Configuration configuration) { configuration.SetProperty(Environment.FormatSql, "true"); configuration.SetProperty(Environment.GenerateStatistics, "true"); - configuration.SetProperty(Environment.BatchSize, "10"); + configuration.SetProperty(Environment.BatchSize, BatchSize.ToString()); #if NET6_0_OR_GREATER if (_useDbBatch) { @@ -303,5 +306,55 @@ public void AbstractBatcherLogFormattedSql() Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); Cleanup(); } + + [Test] + [Description("Inserting exactly BatchSize entities should not throw on commit. See GH-3725.")] + public void InsertExactlyBatchSizeEntitiesShouldNotThrowOnCommit() + { + // This test verifies that DbBatchBatcher handles empty batches correctly. + // The bug (GH-3725): When inserting exactly BatchSize entities, the batch auto-executes + // when full (via ExecuteBatchWithTiming), which clears _currentBatch but NOT _batchCommand. + // On commit, ExecuteBatch() is called, sees _batchCommand is set, and calls DoExecuteBatch + // on an empty _currentBatch, causing InvalidOperationException. + + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // Insert exactly BatchSize entities - this fills the batch and triggers auto-execution. + for (int i = 0; i < BatchSize; i++) + { + session.Save(new VerySimple { Id = 1000 + i, Name = $"Test{i}", Weight = i * 1.1 }); + } + + // Commit triggers ExecuteBatch() which would fail on empty batch without the fix, + // depending on the driver. It fails with Microsoft.Data.SqlClient by example, not with + // System.Data.SqlClient. + transaction.Commit(); + } + + Cleanup(); + } + + [Test] + [Description("Inserting a multiple of BatchSize entities should not throw on commit. See GH-3725.")] + public void InsertMultipleOfBatchSizeEntitiesShouldNotThrowOnCommit() + { + // Same issue as above but with multiple full batches + const int batchSize = 10; + const int multiplier = 3; + + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + for (int i = 0; i < batchSize * multiplier; i++) + { + session.Save(new VerySimple { Id = 2000 + i, Name = $"Test{i}", Weight = i * 1.1 }); + } + + transaction.Commit(); + } + + Cleanup(); + } } } diff --git a/src/NHibernate.Test/Async/Ado/BatcherFixture.cs b/src/NHibernate.Test/Async/Ado/BatcherFixture.cs index 96146f114a3..9f39982e17f 100644 --- a/src/NHibernate.Test/Async/Ado/BatcherFixture.cs +++ b/src/NHibernate.Test/Async/Ado/BatcherFixture.cs @@ -8,6 +8,7 @@ //------------------------------------------------------------------------------ +using System.Linq; using NHibernate.AdoNet; using NHibernate.Cfg; using NUnit.Framework; @@ -38,11 +39,13 @@ protected override string[] Mappings get { return new[] { "Ado.VerySimple.hbm.xml", "Ado.AlmostSimple.hbm.xml" }; } } + private const int BatchSize = 10; + protected override void Configure(Configuration configuration) { configuration.SetProperty(Environment.FormatSql, "true"); configuration.SetProperty(Environment.GenerateStatistics, "true"); - configuration.SetProperty(Environment.BatchSize, "10"); + configuration.SetProperty(Environment.BatchSize, BatchSize.ToString()); #if NET6_0_OR_GREATER if (_useDbBatch) { @@ -275,5 +278,55 @@ public async Task AbstractBatcherLogFormattedSqlAsync() Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1)); await (CleanupAsync()); } + + [Test] + [Description("Inserting exactly BatchSize entities should not throw on commit. See GH-3725.")] + public async Task InsertExactlyBatchSizeEntitiesShouldNotThrowOnCommitAsync() + { + // This test verifies that DbBatchBatcher handles empty batches correctly. + // The bug (GH-3725): When inserting exactly BatchSize entities, the batch auto-executes + // when full (via ExecuteBatchWithTiming), which clears _currentBatch but NOT _batchCommand. + // On commit, ExecuteBatch() is called, sees _batchCommand is set, and calls DoExecuteBatch + // on an empty _currentBatch, causing InvalidOperationException. + + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + // Insert exactly BatchSize entities - this fills the batch and triggers auto-execution. + for (int i = 0; i < BatchSize; i++) + { + await (session.SaveAsync(new VerySimple { Id = 1000 + i, Name = $"Test{i}", Weight = i * 1.1 })); + } + + // Commit triggers ExecuteBatch() which would fail on empty batch without the fix, + // depending on the driver. It fails with Microsoft.Data.SqlClient by example, not with + // System.Data.SqlClient. + await (transaction.CommitAsync()); + } + + await (CleanupAsync()); + } + + [Test] + [Description("Inserting a multiple of BatchSize entities should not throw on commit. See GH-3725.")] + public async Task InsertMultipleOfBatchSizeEntitiesShouldNotThrowOnCommitAsync() + { + // Same issue as above but with multiple full batches + const int batchSize = 10; + const int multiplier = 3; + + using (var session = OpenSession()) + using (var transaction = session.BeginTransaction()) + { + for (int i = 0; i < batchSize * multiplier; i++) + { + await (session.SaveAsync(new VerySimple { Id = 2000 + i, Name = $"Test{i}", Weight = i * 1.1 })); + } + + await (transaction.CommitAsync()); + } + + await (CleanupAsync()); + } } } diff --git a/src/NHibernate/AdoNet/DbBatchBatcher.cs b/src/NHibernate/AdoNet/DbBatchBatcher.cs index 4914a107764..748a8aee30d 100644 --- a/src/NHibernate/AdoNet/DbBatchBatcher.cs +++ b/src/NHibernate/AdoNet/DbBatchBatcher.cs @@ -115,6 +115,12 @@ public override Task AddToBatchAsync(IExpectation expectation, CancellationToken protected override void DoExecuteBatch(DbCommand ps) { + if (_currentBatch.BatchCommands.Count == 0) + { + Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0, ps); + return; + } + try { Log.Debug("Executing batch"); @@ -145,6 +151,12 @@ protected override void DoExecuteBatch(DbCommand ps) protected override async Task DoExecuteBatchAsync(DbCommand ps, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); + if (_currentBatch.BatchCommands.Count == 0) + { + Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0, ps); + return; + } + try { Log.Debug("Executing batch");