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

Dynamic component with nullable property always causes an update on flush

Open nfplee opened this issue 1 year ago • 3 comments

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.

nfplee avatar Aug 30 '23 12:08 nfplee

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

bahusoid avatar Sep 01 '23 16:09 bahusoid

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 avatar Mar 06 '24 09:03 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:

https://github.com/nhibernate/nhibernate-core/blob/f9fcdcd73d27ce2794564cef08f2f299bc6d9c75/src/NHibernate/Type/ComponentType.cs#L189-L192

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:

https://github.com/nhibernate/nhibernate-core/blob/f9fcdcd73d27ce2794564cef08f2f299bc6d9c75/src/NHibernate/Type/ComponentType.cs#L193

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

https://github.com/nhibernate/nhibernate-core/blob/f9fcdcd73d27ce2794564cef08f2f299bc6d9c75/src/NHibernate/Tuple/Component/PocoComponentTuplizer.cs#L73-L76

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:

https://github.com/nhibernate/nhibernate-core/blob/f9fcdcd73d27ce2794564cef08f2f299bc6d9c75/src/NHibernate/Tuple/Component/AbstractComponentTuplizer.cs#L85-L91

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:

https://github.com/nhibernate/nhibernate-core/blob/f9fcdcd73d27ce2794564cef08f2f299bc6d9c75/src/NHibernate/Type/ComponentType.cs#L189-L192

Also I assume the following should also be removed:

https://github.com/nhibernate/nhibernate-core/blob/f9fcdcd73d27ce2794564cef08f2f299bc6d9c75/src/NHibernate/Type/ComponentType.cs#L163-L166

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.

nfplee avatar Jun 06 '24 12:06 nfplee