Changing the dirty flag in PersistentMap

Hibernate ORM
1

Would it hurt to use !Objects.equals(value, old) instead of reference equality in PersistentMap? This would be more consistent to the behavior in PersistentSet.add, which uses set semantics, formally applying Objects.equals for added values.

PersistentMap.put:

                final E old = map.put( key, value );
		// would be better to use the element-type to determine
		// whether the old and the new are equal here; the problem being
		// we do not necessarily have access to the element type in all
		// cases
		if ( value != old ) { // reference equality
			dirty();
		}

PersistentSet.add:

			if ( set.add( value ) ) { // Objects.equals
				dirty();
				return true;
			}

In our application, object serialization can often cause identical strings with different object references to be interpreted as changes, when a put on PersistentMap is triggered. It is not very convenient for us to perform the equals() check ourselves before calling put for an @ElementCollection or a @ManyToMany relationship, since PersistentMap is the data structure behind the scenes provided by Hibernate.

2

This comment says it all. Ideally, we’d use the Hibernate ORM Type contract to determine equality, but that isn’t necessarily available.

Maybe as an optimization, we could store the type in a transient field and lazily resolve it for checking equality, or, like you suggested, use equals() since PersistentSet also seems to rely on this.

I would suggest you to post this suggestion as a topic on our Zulip chat platform and see what others think about this. The change itself would be easy enough, you could also open a PR to try and see if tests fail when doing that.

3

OK thanks, we look into it and send a PR if the tests pass.

4

Well, the tests pass in Hibernate 6.6 but unfortunately the dirty() method is still called by PersistentMap.clear when EntityManager.merge is executed and the map is not empty. In short, the proposed change would not cover all cases, especially not the merging of detached entities (after deserialization) with EntityManager.merge, which is a common use case we have.

Debugging the EntityManager.merge execution revealed that PersistentMap.clear is called by org.hibernate.type.MapType#replaceElements before copying the new values:

PersistentMap.clear:

            if ( !map.isEmpty() ) {
                dirty();
                map.clear();
            }

The same behavior can be observed when Collections are merged and org.hibernate.type.CollectionType#replaceElements is called.

Merging detached entities with non-empty collections or non-empty maps always trigger an optimistic version increment, even when the values don’t change. However, if the collection or map is empty, the version remains unchanged.

Is this expected behavior, or should it be investigated further? Is it advisable to avoid using EntityManager.merge if nothing has changed in the entity, so that the version is not unnecessarily increased? Do you think it’s still worth to send a PR?

5

It’s hard to say for sure what is and what isn’t expected if I don’t have all the code. I would suggest you please try to create a reproducer with our test case template and if you are able to reproduce the issue, create a bug ticket in our issue tracker and attach that reproducer.

6

Thanks @beikov, HHH-19658 contains the issue description and the link to the reproducer.