Hibernate dirty property false positives

[Traivor]

Describe the bug

Beginning with Quarkus 2.14.1.Final and continuing through 2.15.2.Final, hibernate is detecting properties as dirty even if they have been set to the same value that is already in the entity.

eg

var myEntity = em.find(MyEntity.class, id);
myEntity.setProperty(myEntity.getProperty());

Will result in an update sent to the db. Envers will actually NOT store an audit unless there is also an update timestamp that gets updated on persist such as one managed by @UpdateTimestamp. Envers is how I found this was happening since I do have such a timestamp implemented.

Turning hibernate logs up to 11, these are the only interesting entries I get.

2023-01-06 20:23:48,418 masergyball quarkus-run.jar[3876796] TRACE [org.hib.eve.int.DefaultFlushEntityEventListener] (executor-thread-0) Found dirty properties [[com.masergy.nms.nom.datamodel.BizNetMap#914587]] : [networkObjectClass, networkObjectSubClass, bizObj, node, service, netObj, datasource, grade]
2023-01-06 20:23:48,419 masergyball quarkus-run.jar[3876796] TRACE [org.hib.eve.int.DefaultFlushEntityEventListener] (executor-thread-0) Found dirty properties [[com.masergy.nms.nom.datamodel.BizNetMap#914587]] : [modifiedDatetime, networkObjectClass, networkObjectSubClass, bizObj, node, service, netObj, datasource, grade]

2.14.0.Final and 2.13.3.Final do not exhibit this behavior.

Expected behavior

If an entity property is set to the same value it already contained, the entity should not be marked dirty and updated in the database.

Actual behavior

Extra updates (and potentially envers audits) are sent to the database.

How to Reproduce?

No response

Output of uname -a or ver

Linux masergyball 6.0.15-300.fc37.x86_64 #1 SMP PREEMPT_DYNAMIC Wed Dec 21 18:33:23 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Output of java -version

openjdk version "11.0.17" 2022-10-18 OpenJDK Runtime Environment (Red_Hat-11.0.17.0.8-1.fc37) (build 11.0.17+8) OpenJDK 64-Bit Server VM (Red_Hat-11.0.17.0.8-1.fc37) (build 11.0.17+8, mixed mode, sharing)

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.14.1.Final - 2.15.2.Final

Build tool (ie. output of mvnw --version or gradlew --version)

Apache Maven 3.8.5 (3599d3414f046de2324203b78ddcf9b5e4388aa0) Maven home: /home/hcampbell/tools/maven Java version: 11.0.17, vendor: Red Hat, Inc., runtime: /usr/lib/jvm/java-11-openjdk-11.0.17.0.8-1.fc37.x86_64 Default locale: en_US, platform encoding: UTF-8 OS name: "linux", version: "6.0.15-300.fc37.x86_64", arch: "amd64", family: "unix"

Additional information

No response

[quarkus-bot]

/cc @Sanne(hibernate-orm), @gsmet(hibernate-orm), @yrodiere(hibernate-orm)

[Traivor]

In case it matters, my entites are in a separately built jar rather than being part of the quarkus app proper.

[yrodiere]

Hey @Traivor , thanks for the report.

There are multiple things at play here, and dirty checking obviously works in some situations (the ones from our tests, at least), so it's a bit hard to understand what went wrong in your case.

Can you please provide a simple reproducer?

Thanks.

[Postremus]

@yrodiere Not the op of this issue, but I can also reproduce problems with dirty checking since 2.14.1.Final (but not e.g. 2.13.3.Final).

Here is a reproducer:
update-onetoone.zip

1. mvn clean compile quarkus:dev
2. Visit http://localhost:8080/hello/insert to insert an entity
3. Visit http://localhost:8080/hello?id=3 to load this entity. This also does a simple entity.setField(entity.getField()); to test this problem.
4. Look in the terminal. SQL logging is activated. During the last HTTP call an update was made.

[Postremus]

I tested a few different versions.
The problem occurs at least in the versions, and most likely any in between: 2.13.4.Final, 2.13.5.Final, 2.13.6.Final, 2.14.1.Final, 2.15.2.Final, 2.15.3.Final, 2.16.0.CR1

Version 2.13.3.Final and down are not affected.

2.13.4.Final included an hibernate update. Maybe it is related.
https://github.com/quarkusio/quarkus/releases/tag/2.13.4.Final

[Postremus]

@yrodiere
InlineDirtyCheckerEqualsHelper#areEquals returns false if the property is not lazy.
https://github.com/hibernate/hibernate-orm/blob/5.6/hibernate-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/InlineDirtyCheckerEqualsHelper.java#L16

No update gets done if I annotate the field as (correct behaviour)

    @Basic(fetch = FetchType.LAZY)
    private String field;

I believe this might be caused by hibernate/hibernate-orm#5461

[Traivor]

@yrodiere Not the op of this issue, but I can also reproduce problems with dirty checking since 2.14.1.Final (but not e.g. 2.13.3.Final).

Here is a reproducer: update-onetoone.zip

1. mvn clean compile quarkus:dev

2. Visit http://localhost:8080/hello/insert to insert an entity

3. Visit http://localhost:8080/hello?id=3 to load this entity. This also does a simple `entity.setField(entity.getField());` to test this problem.

4. Look in the terminal. SQL logging is activated. During the last HTTP call an update was made.

Thank you! I have no idea when I might have had time to create a repropducer.

[Postremus]

A possible workaround is to circumvent the in-built dirty check of hibernate, and write our own:

import io.quarkus.hibernate.orm.PersistenceUnitExtension;
import org.hibernate.EmptyInterceptor;
import org.hibernate.bytecode.enhance.spi.interceptor.BytecodeLazyAttributeInterceptor;
import org.hibernate.engine.internal.ManagedTypeHelper;
import org.hibernate.engine.spi.PersistentAttributeInterceptable;
import org.hibernate.engine.spi.PersistentAttributeInterceptor;
import org.hibernate.type.Type;

import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

@PersistenceUnitExtension
public class DirtyCheckInterceptor extends EmptyInterceptor {
    @Override
    public int[] findDirty(Object entity, Serializable id, Object[] currentState, Object[] previousState, String[] propertyNames, Type[] types) {
        BytecodeLazyAttributeInterceptor byteInterceptor = null;
        if (ManagedTypeHelper.isPersistentAttributeInterceptable(entity)) {
            PersistentAttributeInterceptable persistentAttributeInterceptable = ManagedTypeHelper.asPersistentAttributeInterceptable(entity);
            PersistentAttributeInterceptor persistentAttributeInterceptor = persistentAttributeInterceptable.$$_hibernate_getInterceptor();

            if (persistentAttributeInterceptor instanceof BytecodeLazyAttributeInterceptor) {
                byteInterceptor = (BytecodeLazyAttributeInterceptor) persistentAttributeInterceptor;
            }
       }

        List<Integer> changed = new ArrayList<>();
        for (int i = 0; i < currentState.length; i++) {
            if (byteInterceptor != null) {
                // check if the field was loaded, i.e. accessed before.
                // otherwise no modifcation could have happened
                if (!byteInterceptor.isAttributeLoaded(propertyNames[i])) {
                    continue;
                }
            }

            if (ManagedTypeHelper.isManagedEntity(previousState[i])) {
                // was field entity previosly, but now set as null? -> changed (prevent deepequals)
                if (currentState[i] == null) {
                    changed.add(i);
                }
            } else if (ManagedTypeHelper.isManagedEntity(currentState[i])) {
                // is field entity currently, but was set as null? -> changed (prevent deepequals)
                if (previousState[i] != null) {
                    changed.add(i);
                }
            } else if (!Objects.deepEquals(currentState[i], previousState[i])) {
                changed.add(i);
            }
        }

        return changed.stream().mapToInt(i -> i).toArray();
    }
}

This obiously does not handle every possible case for the dirty detection, but works in my small reproducer app for now.

[yrodiere]

I believe this might be caused by hibernate/hibernate-orm#5461

I concur, that's the most likely cause. We started using that patch in Quarkus with #28881 , in Quarkus 2.13.4.

I'm still having a look at the reproducer, I'll probably have to convert it to a Quarkus-free reproducer for the Hibernate ORM team to have a look.

[yrodiere]

Reported upstream as https://hibernate.atlassian.net/browse/HHH-16049

Thanks @Postremus for your help!

[ribizli]

My trial on this issue lead to compile time byte-code enhancement (plugin: hibernate-enhance-maven-plugin), and that works for my use-case.

https://docs.jboss.org/hibernate/orm/5.0/topical/html/bytecode/BytecodeEnhancement.html

[yrodiere]

@ribizli Be aware that if you don't use the exact same version of Hibernate ORM for compile-time bytecode enhancement as the version of Hibernate ORM used at runtime, you may avoid this particular bug, but you will run into many other bugs caused by incompatibility.

[Postremus]

@yrodiere Is there any plan for a new ORM 5.6 release with this bugfix?

[yrodiere]

I think Sanne started working on some performance improvements, I don't know if he's done, but after that yes we'll probably release another 5.6. @Sanne any objection to releasing 5.6 soon?

[Sanne]

sure, I'm long done with the 5.6 changes and moved on to tuning ORM 6 - we could release a 5.6 anytime on the Hibernate side.

A bigger question is if there will actually be a Quarkus release to include the new version: with Quarkus 3.0 being the next version and it being expected to be based on Hibernate ORM 6, the 5.x upgrade would need to be done in a micro maintenance.

But let's release ORM 5 either way, then at very least @Postremus has the option to override the version and we can see if there's going to be an upgrade in Quarkus or not - plans are flexible and at least we'll be ready for it. This could be useful for benchmarks too.

[maxant]

I am pretty sure that I have this issue in 3.9.1. Still testing...
The work around with the DirtyCheckInterceptor works though.
without it, merging a detached entity which is identical to the state in the database, causes the version to be updated and an update statement to be executed.

[gsmet]

@maxant I would recommend to open another issue with a simple reproducer.

[maxant]

@gsmet : #39792

[maxant]

In case anyone stumbles over this and needs the DirtyCheckInterceptor above, a) I think it has two bugs and b) it is kind of deprecated in hibernate 6.

a) bug 1?

        } else if (ManagedTypeHelper.isManagedEntity(currentState[i])) {
            // is field entity currently, but was set as null? -> changed (prevent deepequals)
            if (previousState[i] != null) {
                changed.add(i);
            }

That should test == null rather than != null, imho. tests where i set a field that is an entity, to null, failed with the non-null test.

a) bug 2?

        if (ManagedTypeHelper.isManagedEntity(previousState[i])) {
            // was field entity previously, but now set as null? -> changed (prevent deepEquals)
            if (currentState[i] == null) {
                changed.add(i);
            } else if (types[i].getClass().equals(org.hibernate.type.ManyToOneType.class)) {
                if(!Objects.deepEquals(currentState[i], previousState[i])) {
                    changed.add(i);
                }
            } // else: ignore OneToMany. ignore OneToOne? assuming the FKs cannot change

I noticed that when a ManyToOne field is encounted, it is ignored in the original code. Above is an additional check which ensures that the foreign key hasn't changed and if it has, marks the field as changed.

b) this works with hibernate 6:

import io.quarkus.hibernate.orm.PersistenceUnitExtension;
import org.hibernate.Interceptor;
import org.hibernate.bytecode.enhance.spi.interceptor.BytecodeLazyAttributeInterceptor;
import org.hibernate.engine.internal.ManagedTypeHelper;
import org.hibernate.engine.spi.PersistentAttributeInterceptable;
import org.hibernate.engine.spi.PersistentAttributeInterceptor;
import org.hibernate.type.Type;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/**
 * <a href="https://github.com/quarkusio/quarkus/issues/30234">...</a>
 * still seems to be around for 3.9.1!
 * logged as bug: <a href="https://github.com/quarkusio/quarkus/issues/39792">...</a>
 */
@PersistenceUnitExtension
public class DirtyCheckInterceptor implements Interceptor {

    @Override
    public  int[] findDirty(
            Object entity,
            Object id,
            Object[] currentState,
            Object[] previousState,
            String[] propertyNames,
            Type[] types) {
        BytecodeLazyAttributeInterceptor byteInterceptor = null;
        if (ManagedTypeHelper.isPersistentAttributeInterceptable(entity)) {
            PersistentAttributeInterceptable persistentAttributeInterceptable = ManagedTypeHelper.asPersistentAttributeInterceptable(entity);
            PersistentAttributeInterceptor persistentAttributeInterceptor = persistentAttributeInterceptable.$$_hibernate_getInterceptor();

            if (persistentAttributeInterceptor instanceof BytecodeLazyAttributeInterceptor) {
                byteInterceptor = (BytecodeLazyAttributeInterceptor) persistentAttributeInterceptor;
            }
       }

        List<Integer> changed = new ArrayList<>();
        for (int i = 0; i < currentState.length; i++) {
            if (byteInterceptor != null) {
                // check if the field was loaded, i.e. accessed before.
                // otherwise no modification could have happened
                if (!byteInterceptor.isAttributeLoaded(propertyNames[i])) {
                    continue;
                }
            }

            if (ManagedTypeHelper.isManagedEntity(previousState[i])) {
                // was field entity previously, but now set as null? -> changed (prevent deepEquals)
                if (currentState[i] == null) {
                    changed.add(i);
                }
            } else if (ManagedTypeHelper.isManagedEntity(currentState[i])) {
                // is field entity currently, but was set as null? -> changed (prevent deepEquals)
                if (previousState[i] == null) {
                    changed.add(i);
                }
            } else if (!Objects.deepEquals(currentState[i], previousState[i])) {
                changed.add(i);
            }
        }

        return changed.stream().mapToInt(i -> i).toArray();
    }
}

@Postremus where did you get that code from orginally? It would be cool if it was in a library, or indeed hibernate itself, so that I can be used if required. I think that when bytecode enhancement is turned on, dirty checking doesn't work properly when merging detached objects and forces an update statement even if the object is unchanged compared to the DB. Quarkus uses bytecode by default.