Empty dynamic components cause phantom updates

[nfplee]

I've noticed whilst profiling recently that I keep getting random calls to update particular entities when I flush changes to the database, even if they haven't changed.

After looking into this further, i've discovered this seems to happen when the entity has a dynamic component with nullable properties.

For example, say I have the following application:

using var sessionFactory = BuildConfiguration().BuildSessionFactory();
using var session = sessionFactory.OpenSession();

var products = await session.Query<Product>().ToListAsync();

await session.FlushAsync();

Console.ReadLine();

Configuration BuildConfiguration() {
    var configuration = new Configuration();

    configuration.DataBaseIntegration(db => {
        db.ConnectionString = @"Data Source=TDB2008R2;Initial Catalog=KIT5-TEMP;Persist Security Info=True;User ID=sa;Password=m3spexec4@sadrAb;Connect Timeout=30";
        db.Dialect<MsSql2008Dialect>();
    });

    var mapper = new ModelMapper();
    mapper.AddMappings(Assembly.GetCallingAssembly().ExportedTypes);
    configuration.AddMapping(mapper.CompileMappingForAllExplicitlyAddedEntities());

    return configuration;
}

With the following model and mapping:

public class Product {
    public virtual int Id { get; set; }
    public virtual string Name { get; set; } = default!;

    private IDictionary<string, object?>? _attributes;
    public virtual IDictionary<string, object?> Attributes {
        get => _attributes ??= new Dictionary<string, object?>();
        set => _attributes = value;
    }
}

public class ProductMapping : ClassMapping<Product> {
    public ProductMapping() {
        Table("Products");
        Id(x => x.Id, m => m.Generator(Generators.Identity));
        Property(x => x.Name);
        Component(x => x.Attributes, new {
            Sku = (string?)null
        }, dc => {
            dc.Property(x => x.Sku);
        });
    }
}

Here's the SQL to setup the database table with some sample data:

CREATE TABLE [dbo].[Products](
    [Id] [int] IDENTITY(1,1) NOT NULL,
    [Name] [nvarchar](50) NOT NULL,
    [Sku] [nvarchar](50) NULL,
    CONSTRAINT [PK_Products] PRIMARY KEY CLUSTERED ([Id] ASC)
)

INSERT INTO [dbo].[Products] ([Name], [Sku]) VALUES ('Test 1', NULL)
INSERT INTO [dbo].[Products] ([Name], [Sku]) VALUES ('Test 2', 'ABC')
INSERT INTO [dbo].[Products] ([Name], [Sku]) VALUES ('Test 3', NULL)

Now when profiling the application, notice how it will issue 2 updates (only the products where the Sku is null) to the database when you flush the changes.

This seems like a bug to me but I thought I'd check before creating a test case.

I have discovered that this isn't an issue if I change my Attributes property to the following:

public virtual IDictionary<string, object?> Attributes { get; set; } = new Dictionary<string, object?>();

However this will still set the Attributes to null against the products where all the values of the Attributes are null and will likely lead to much bigger problems.

[bahusoid]

However this will still set the Attributes to null against the products where all the values of the Attributes are null and will likely lead to much bigger problems.

#2112 looks related

[nfplee]

Please see the following test case:

GH3421.zip

I used an SqlInterceptor to monitor the queries sent to the database. The test confirms that when flushing changes it will trigger an update for any entity where all the values are null against a dynamic component, even if it hasn't changed.

[nfplee]

I've just looked into this and compared it with a standard component type mapping, e.g. where you use a complex type instead of a dictionary for the component:

private ComplexType _complexType;
public virtual ComplexType ComplexType {
  get {
    if (_complexType == null)
      _complexType = new ComplexType();
    return _complexType;
  }
  set => _complexType = value;
}

This is then mapped like so:

rc.Component(x => x.ComplexType, m => {
    m.Property(x => x.Sku);
})

as opposed to the following (when using a dynamic component):

rc.Component(x => x.Attributes, new {
  Sku = (string?)null
=}, dc => {
  dc.Property(x => x.Sku);
});

The standard component type doesn't suffer from this issue. So I stepped through the NHibernate source code and found the following code is the cause of the problem:

nhibernate-core/src/NHibernate/Type/ComponentType.cs

Lines 189 to 192 in f9fcdcd

	if (EntityMode != EntityMode.Poco && (x == null || y == null))
	{
	return true;
	}

After doing a flush, for a standard component when x is null and y isn't (but all of it's properties are null), the EntityMode is Poco and therefore this condition is not true. However for a dynamic component, when x is null and y isn't (but the collection is empty), the EntityMode is Map and therefore the condition is true so it's always marked as dirty.

I then looked at what happens if you comment out the lines 189 to 192 in the ComponentType class and now for both cases it will proceed to execute the following:

nhibernate-core/src/NHibernate/Type/ComponentType.cs

Line 193 in f9fcdcd

object[] xvalues = GetPropertyValues(x);

For a standard component (when the EntityMode is Poco), it called the following:

nhibernate-core/src/NHibernate/Tuple/Component/PocoComponentTuplizer.cs

Lines 73 to 76 in f9fcdcd

	if (Equals(BackrefPropertyAccessor.Unknown, component) || Equals(LazyPropertyInitializer.UnfetchedProperty, component) || component == null)
	{
	return new object[propertySpan];
	}

When x is null, this condition is true and an object array (of the appropriate size) is returned. Now when it compares it to the property values of y both are identical and therefore it isn't returned as dirty.

Now for a dynamic component (when the EntityMode is Map, it calls:

nhibernate-core/src/NHibernate/Tuple/Component/AbstractComponentTuplizer.cs

Lines 85 to 91 in f9fcdcd

	object[] values = new object[propertySpan];
	// NH Different behavior : for NH-1101
	if (component != null)
	for (int i = 0; i < propertySpan; i++)
	values[i] = GetPropertyValue(component, i);
	return values;

So when x is null, it will do the same as in the PocoComponentTuplizer and return an object array (of the appropriate size). Once again when it compares the property values of x and y they are identical and therefore it isn't returned as dirty.

I believe the solution to the problem is to remove the following:

nhibernate-core/src/NHibernate/Type/ComponentType.cs

Lines 189 to 192 in f9fcdcd

	if (EntityMode != EntityMode.Poco && (x == null || y == null))
	{
	return true;
	}

Also I assume the following should also be removed:

nhibernate-core/src/NHibernate/Type/ComponentType.cs

Lines 163 to 166 in f9fcdcd

	if (EntityMode != EntityMode.Poco && (x == null || y == null))
	{
	return true;
	}

I would submit it as a pull request, but I was unable to run all the tests against it, only my test fixture. I would appreciate it if someone could look at this as it's causing me major performance issues, since every time I retrieve something from the database and do a flush, it pushes hundreds of additional updates to the database.

[fredericDelaporte]

So, for the record, NH-1101 was about exactly what you are doing, but restricted to standard component. The original proposed patch there was not restricted to standard components but would have covered dynamic ones too, by the look of it. (And furthermore it was also about giving an option to enable the behavior asked by #2112.)

There are no comment there allowing to know why it was restricted to standard components when committed (6dfc5d9).

[fredericDelaporte]

Looking at Hibernate code, it has been changed for all cases by HHH-7610, so, your change also realigns with current state of Hibernate. That Hibernate change was also about providing an option for having the component instantiated with all properties being null. Part of implementing it implied removing the shortcut you are removing.