Fix #3725: DbBatchBatcher empty batch execution bug
Fixes #3725
Description
DbBatchBatcher throws InvalidOperationException when attempting to execute an empty batch, while
GenericBatchingBatcher and SqlClientBatchingBatcher handle this scenario correctly.
Root Cause
DbBatchBatcher.DoExecuteBatch() and DoExecuteBatchAsync() lack empty batch validation. When ExecuteBatch()
is called with an empty batch (_currentBatch.BatchCommands.Count == 0), the code attempts to execute a DbBatch
with no commands, causing:
System.InvalidOperationException: ExecuteNonQuery: CommandText property has not been initialized
Changes
Added empty batch check at the beginning of both methods, matching the pattern used in GenericBatchingBatcher:
if (_currentBatch.BatchCommands.Count == 0)
{
Expectations.VerifyOutcomeBatched(_totalExpectedRowsAffected, 0, ps);
return;
}
Modified:
- src/NHibernate/AdoNet/DbBatchBatcher.cs - Added guard clauses to DoExecuteBatch() and DoExecuteBatchAsync()
Added:
- Test cases in src/NHibernate.Test/Ado/BatcherFixture.cs verifying the fix
- Generated async test counterparts
Good catch and thanks for the fix!
My very personal reflection is that your tests should be included in the normal batcher tests, rather than as a NHSpecificTest.
Good point! I'll move the tests
I am trying the test without the fix, and I do not get a failure. The test must demonstrate the trouble.
Test run with PostgreSQL, after having reset mixed the commit, and undone the change on NHibernate to only keep the change on NHibernate.Test.
I have updated the test to be more like the scenario I encountered the bug in and verified that the tests now fail without the fix on SQL Server
I have tried with your test update and SQL Server: under net8, the test is ignored. Under net4.8, there is still no failure without your fix. With PostgreSQL, still no failures either.
So, I am now launching the whole CI on your test to check if anything starts failing, see #3733.
Did you use NHibernate.Driver.MicrosoftDataSqlClientDriver as the driver on .NET8? Wasn't ignored for me. On net4.8 it wont fail though as this bug only exists in the DbBatchBatcher afaik. Maybe it should be locked down to only run for .NET8 then.