With Version 5.5.x, Session.Merge leads to an additional unnecessary update statement, when merging transient versioned instances with child collections

[DominicUllmann]

When using session.Merge on a transient versioned entity which contains collections with children, an additional update statement is run on the DB starting with version 5.5.x to perform a row version update on the parent after inserting parent and child.

Consider the following example based on the VersionFixture test in NHibernate:

            [Test]
	public void CollectionVersionMerge()
	{
		ISession s = OpenSession();
		ITransaction t = s.BeginTransaction();
		Person gavin = new Person("Gavin");
		var thing = new Thing("Passport", gavin);
		gavin = s.Merge(gavin);
		t.Commit();
		s.Close();

		Assert.AreEqual(1, gavin.Version);

		s = OpenSession();
		t = s.BeginTransaction();
		s.Delete(gavin);
		t.Commit();
		s.Close();
	} 

I had to modify the Person class slightly to match my example and have collections initialized on construction (as otherwise an additional update statement is also present independent of the NHibernate release):

    public class Person
{
	private string name;
	private IList<Thing> things;
	private IList<Task> tasks;
	private int version;
            protected Person()
	{
                    // initialize here instead of only in public constructor...
		this.things = new List<Thing>();
		this.tasks = new List<Task>();
	}

	public Person(string name) : this()
	{
		this.name = name;
	}

            ....

    }

Before V5.5.x, the test leads to the following 2 statements to persist the person:

 exec sp_executesql N'INSERT INTO Person (Version, Name) VALUES (@p0, @p1)',N'@p0 int,@p1 nvarchar(4000)',@p0=1,@p1=N'Gavin'
 exec sp_executesql N'INSERT INTO Thing (Version, LongDescription, person, Description) VALUES (@p0, @p1, @p2, @p3)',N'@p0 int,@p1 nvarchar(4000),@p2 nvarchar(4000),@p3 nvarchar(4000)',@p0=1,@p1=NULL,@p2=N'Gavin',@p3=N'Passport'

In V5.5.x this changes to:

exec sp_executesql N'INSERT INTO Person (Version, Name) VALUES (@p0, @p1)',N'@p0 int,@p1 nvarchar(4000)',@p0=1,@p1=N'Gavin'
exec sp_executesql N'INSERT INTO Thing (Version, LongDescription, person, Description) VALUES (@p0, @p1, @p2, @p3)',N'@p0 int,@p1 nvarchar(4000),@p2 nvarchar(4000),@p3 nvarchar(4000)',@p0=1,@p1=NULL,@p2=N'Gavin',@p3=N'Passport'
exec sp_executesql N'UPDATE Person SET Version = @p0 WHERE Name = @p1 AND Version = @p2',N'@p0 int,@p1 nvarchar(4000),@p2 int',@p0=2,@p1=N'Gavin',@p2=1

This change is caused by PR #3326. It leads to a change in the collection merge process.
In V5.4, the collections are not yet wrapped into PersistenCollections after the session.Merge:

In V5.5, the collections are wrapped and the collection containing a child is marked as dirty:

The wrapping of the collection is caused by the change in AbstractSaveEventListener.PerformSaveOrReplicate. The wrapping itself replaces the empty List on the copy by an empty wrapped collection. This is initially not dirty.
The collection gets dirty by the DefaultMergeEventListener.CopyValues, which copies the properties from the original Person instance to the copied Person instance using CollectionType.ReplaceElements.

I wonder if this additional update statement is expected in this case or if this can be prevented. From a performance perspective, it is not nice to have unnecessary update statements.