ComponentType mappings for with value types (structs) cause incorrect dirty checking

Description

If a member of a class is a value type T and mapped as a component, NHibernate will always detect objects with a value of default(T) as dirty.

The problem is in ComponentType.IsDirty(): the original value as ready by NHibernate will be null, whereas the value returned from an instance will be a boxed default(T). The current logic does not treat null as equivalent to default(T) for value types, which it should do.

It is possible in the constructor of ComponentType to box a default(T) and use this instead of null in the IsDirty() methods - this works as expected.

public override bool IsDirty( object x, object y, ISessionImplementor session )
{
if( x == y )
{
return false;
}

// In ComponentType() the object componentClassDefault is
// set to a new instance of componentClass iff componentClass is a value type.
if (x == null && y.Equals(componentClassDefault) ||
y == null && x.Equals(componentClassDefault))
{
return false;
}

if( x == null || y == null )
{
return true;
}

Environment

None

Activity

Show:

Karl Chu 
November 4, 2007 at 4:17 PM

Back ported fix from trunk to 1.2.x branch (r3082)

Former user 
October 3, 2007 at 12:49 AM

Reopening for me to remember to merge the fix into 1.2.1

Karl Chu 
October 2, 2007 at 10:10 PM

Fixed in r3022.

The fix was actually in ComponentType.Hydrate(). The property values returned by Hydrate() is cached in the session, and these values are later used to do dirty check.

Nicholas Blumhardt 
March 2, 2007 at 11:18 PM

I do think this should be considered a bug rather than an improvement, because NHibernate does not warn at configuration time that structs are not supported by component mappings.

The implications of this bug when reading say, 500 objects that contain value type components, are 500 update statements being executed at the end of the transaction. The time taken for such an update could easily cause locking issues in a moderately-high-volume system, and will also execute database triggers on the mapped table unnecessarily.

I can provide a patch and unit tests for this one if they could be included in the release. Failing that, NHibernate really does have to warn about this issue at configuration time if new users are not to get a nasty surprise when using value types.

(BTW - value types are an excellent way of mapping components of a class as unlike reference types, there is no difference between a default instance and an instance containing all nulls. NHibernate currently assumes that reference type components with all-null properties are themselves null, which may be unacceptable in some object models, and creates a situation where the object is not loaded with the same state as when it was saved.)

Fixed

Details

Assignee

Reporter

Components

Fix versions

Affects versions

Priority

Who's Looking?

Open Who's Looking?
Created February 13, 2007 at 3:08 PM
Updated June 29, 2008 at 1:46 PM
Resolved November 4, 2007 at 4:17 PM
Who's Looking?