nhibernate-core icon indicating copy to clipboard operation
nhibernate-core copied to clipboard

Fix #3725: DbBatchBatcher empty batch execution bug

Open ja-kar opened this issue 1 month ago • 6 comments

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

ja-kar avatar Nov 17 '25 10:11 ja-kar

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.

gliljas avatar Nov 17 '25 13:11 gliljas

Good point! I'll move the tests

ja-kar avatar Nov 17 '25 14:11 ja-kar

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. image

fredericDelaporte avatar Nov 29 '25 15:11 fredericDelaporte

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

ja-kar avatar Dec 01 '25 09:12 ja-kar

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.

fredericDelaporte avatar Dec 07 '25 18:12 fredericDelaporte

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.

ja-kar avatar Dec 08 '25 07:12 ja-kar