Merge fails with composite identifier containing association

mederly · February 6, 2018, 1:20pm

Hello. We are using Hibernate to manage repository of midPoint, an open source identity management system. There are approximately 70 entities.

Recently we have come across an issue with merging entities that have composite identifiers containing associations.

(Note that this is actually a modified version of my 4 days old question from stack overflow that unfortunately was left unanswered. I do apologize for cross-posting, as well as for the long post but I have tried to fight the issue for a couple of days and I am completely stuck. I would appreciate your opinions on the most correct way of resolving it. As a new user, I am not allowed to include more than 2 links in this post. So I had to remove all links to code (pointing to the source code on github) and majority of all other links.)

The issue can be demonstrated on a really trivial example. It is almost directly derived from the Hibernate 5.2 documentation (2.6.5. Composite identifiers with associations); except for OneToMany side of the Parent-Child association.

Let us have a parent and a child entity:

Parent.java

@Entity
public class Parent implements Serializable {

    @Id
    private Integer id;

    @OneToMany(fetch = FetchType.LAZY, mappedBy = "parent", 
               orphanRemoval = true, cascade = CascadeType.ALL)
    private Set<Child> children = new HashSet<>();

    // getters, setters, equals, hashCode omitted for brevity
}

Child.java

@Entity
public class Child implements Serializable {

    @Id
    @ManyToOne(fetch = FetchType.LAZY, optional = false)
    private Parent parent;

    @Id
    private String value;

    // getters, setters, equals, hashCode omitted for brevity
}

The scenario is:

Create a parent-child pair and persist it into the database.
Create a detached instance of the parent object that contains a new (transient) child instead of the original one.
Merge the detached instance of the parent object.

The code is like this (simplified):

public static void main(String[] args) {
    create();
    update();
}

private static void create() {
    Session session = HibernateUtil.getSessionFactory().openSession();
    session.beginTransaction();

    Parent parent = new Parent();
    parent.setId(10);

    Child child = new Child();
    child.setParent(parent);
    parent.getChildren().add(child);
    child.setValue("old");

    session.save(parent);
    session.getTransaction().commit();
    session.close();
}

private static void update() {
    Session session = HibernateUtil.getSessionFactory().openSession();
    session.beginTransaction();

    Parent parent = new Parent();
    parent.setId(10);

    Child child = new Child();
    child.setParent(parent);
    parent.getChildren().add(child);
    child.setValue("new");

    session.merge(parent);
    session.getTransaction().commit();
    session.close();
}

And it fails because Hibernate tries to insert null values into the child table.

Exception in thread "main" javax.persistence.PersistenceException: org.hibernate.exception.ConstraintViolationException: could not execute statement
    at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:149)
    at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:157)
    at org.hibernate.internal.ExceptionConverterImpl.convert(ExceptionConverterImpl.java:164)
    at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1443)
    at org.hibernate.internal.SessionImpl.managedFlush(SessionImpl.java:493)
    at org.hibernate.internal.SessionImpl.flushBeforeTransactionCompletion(SessionImpl.java:3207)
    at org.hibernate.internal.SessionImpl.beforeTransactionCompletion(SessionImpl.java:2413)
    at org.hibernate.engine.jdbc.internal.JdbcCoordinatorImpl.beforeTransactionCompletion(JdbcCoordinatorImpl.java:473)
    at org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorImpl.beforeCompletionCallback(JdbcResourceLocalTransactionCoordinatorImpl.java:156)
    at org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorImpl.access$100(JdbcResourceLocalTransactionCoordinatorImpl.java:38)
    at org.hibernate.resource.transaction.backend.jdbc.internal.JdbcResourceLocalTransactionCoordinatorImpl$TransactionDriverControlImpl.commit(JdbcResourceLocalTransactionCoordinatorImpl.java:231)
    at org.hibernate.engine.transaction.internal.TransactionImpl.commit(TransactionImpl.java:68)
    at simple.TestMergeParentChild.update(TestMergeParentChild.java:49)
    at simple.TestMergeParentChild.main(TestMergeParentChild.java:10)
Caused by: org.hibernate.exception.ConstraintViolationException: could not execute statement
    at org.hibernate.exception.internal.SQLStateConversionDelegate.convert(SQLStateConversionDelegate.java:112)
    at org.hibernate.exception.internal.StandardSQLExceptionConverter.convert(StandardSQLExceptionConverter.java:42)
    at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:111)
    at org.hibernate.engine.jdbc.spi.SqlExceptionHelper.convert(SqlExceptionHelper.java:97)
    at org.hibernate.engine.jdbc.internal.ResultSetReturnImpl.executeUpdate(ResultSetReturnImpl.java:178)
    at org.hibernate.engine.jdbc.batch.internal.NonBatchingBatch.addToBatch(NonBatchingBatch.java:45)
    at org.hibernate.persister.entity.AbstractEntityPersister.insert(AbstractEntityPersister.java:3013)
    at org.hibernate.persister.entity.AbstractEntityPersister.insert(AbstractEntityPersister.java:3513)
    at org.hibernate.action.internal.EntityInsertAction.execute(EntityInsertAction.java:89)
    at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:589)
    at org.hibernate.engine.spi.ActionQueue.executeActions(ActionQueue.java:463)
    at org.hibernate.event.internal.AbstractFlushingEventListener.performExecutions(AbstractFlushingEventListener.java:337)
    at org.hibernate.event.internal.DefaultFlushEventListener.onFlush(DefaultFlushEventListener.java:39)
    at org.hibernate.internal.SessionImpl.doFlush(SessionImpl.java:1437)
    ... 10 more
Caused by: org.h2.jdbc.JdbcSQLException: NULL not allowed for column "VALUE"; SQL statement:
/* insert simple.Child */ insert into Child (value, parent_id) values (?, ?) [23502-193]
    at org.h2.message.DbException.getJdbcSQLException(DbException.java:345)
    at org.h2.message.DbException.get(DbException.java:179)
    at org.h2.message.DbException.get(DbException.java:155)
    at org.h2.table.Column.validateConvertUpdateSequence(Column.java:311)
    at org.h2.table.Table.validateConvertUpdateSequence(Table.java:784)
    at org.h2.command.dml.Insert.insertRows(Insert.java:151)
    at org.h2.command.dml.Insert.update(Insert.java:114)
    at org.h2.command.CommandContainer.update(CommandContainer.java:98)
    at org.h2.command.Command.executeUpdate(Command.java:258)
    at org.h2.jdbc.JdbcPreparedStatement.executeUpdateInternal(JdbcPreparedStatement.java:160)
    at org.h2.jdbc.JdbcPreparedStatement.executeUpdate(JdbcPreparedStatement.java:146)
    at org.hibernate.engine.jdbc.internal.ResultSetReturnImpl.executeUpdate(ResultSetReturnImpl.java:175)
    ... 19 more

What is wrong here and how to resolve? Here are my thoughts and attempts.

Is this a bug in Hibernate? I have tried to diagnose/debug it for a some time and found the following

image1778×537 110 KB

It looks like that DefaultMergeEventListener.copyValues method (called from line 223) does not copy anything from the transient child entity into a newly created instance (because it omits the identifiers). But I am too little experienced in Hibernate to tell if that’s OK or not.

From the documentation I know that using multiple @Id’s is deprecated…

The restriction that a composite identifier has to be represented by a “primary key class” is only JPA specific. Hibernate does allow composite identifiers to be defined without a “primary key class”, although that modeling technique is deprecated and therefore omitted from this discussion. (from the docs, section 2.6.2)

…but I think that means “it is not recommended but it should work”. I think so also because just the next example - on which my code is based - uses this technique. Overall, I consider it to be a good idea, because it eliminates much clutter from the client source code.
Reading the above, I replaced plain identifiers with @IdClass and/or @EmbeddedId. While this resolved the issue for simple parent-child case (simpleWithIdClass in the github project), it did not help for more complex scenario (testWithIdClass in the github project), more directly related to our midPoint usage.

The problem with more complex scenario is the Unable to find column with logical name: owner_owner_oid in org.hibernate.mapping.Table(AssignmentExtension) and its related supertables and secondary tables exception I get. When trying to resolve it using additional fields with @Column(…, insertable=false, updateable=false) annotations (inspired by earlier work of my colleague on midPoint) I have finally made it function, at the cost of making the client code totally incomprehensible and, to be honest, really ugly. Moreover, I don’t know if now it works by accident or “by design”. Please see testWithHacks in the github project.
Having thought that the problem lies in missing identifier values I tried to tell Hibernate about the identifier values by using GeneratedValue annotations (inspired by “How to combine the Hibernate assigned generator with a sequence or an identity column” blog post), but it didn’t help - see simpleWithIdentifierGenerators in the github project).
As a last resort I have tried to work around the code in DefaultMergeEventListener by persuading it that the transient entity has, in fact, an identifier. I used a custom persister to do that. It helps (even in our application), but I am afraid of the ugliness of the “solution”, in particular of messing with Hibernate internals that I do not understand. See simpleWithCustomPersister package.

Approaches that I considered but not implemented:

Introducing artificial (generated) simple identifiers for ExtBoolean (and related) tables. I’d like to avoid that because of the necessity to change DB schema, in particular by introducing another DB column (performance and storage effects).
I have recently found the following recommendation in a thread that seems a bit similar to this one:

If you bump into such issue, it means you were doing the merge like this:

you try to clear the collection

then you re-add back the entries sent from the client

That’s not a good approach. You should do the merging from the incoming collection and the one in the database like this:

add the new entries

update the remaining ones

delete the ones that are no longer found in the incoming collection
(link)

To be honest I am not sure if this applies to my case. Because of the way we work with our internal objects and their Hibernate representation it would be very complicated to do this. Moreover, shouldn’t Hibernate be able merge arbitrary detached object structure?

Please, what is the cleanest and most recommended way to resolve this issue?

Thank you very much in advance.

vlad · February 6, 2018, 1:35pm

The problem is here:

Why did you use merge when you should have used persist? Merge is for detached entities so that their state can be copied unto a newly fetched one. Merge is not a universal save operation. When you use JPA and Hibernate you need to use:

persist for new entities.
merge for detached ones.
and no operation is needed for managed entities since the directly checking mechanism will trigger the flushs automatically.

mederly · February 6, 2018, 1:46pm

Vlad,

thank you for the reply.

The entity in my update() test method is really detached. It has the same identifier as an existing entity in the database (id=10).

If I change merge() to persist() call in the update() method I do get

Caused by: org.h2.jdbc.JdbcSQLException: Unique index or primary key violation: "PRIMARY KEY ON PUBLIC.PARENT(ID)"; SQL statement:
/* insert simple.Parent */ insert into Parent (id) values (?)

exception - because of identifier collision.

In midPoint such entities are created from our internal data and we do really want to reconcile their state with the database. Therefore we use merge() call. (Just by the way, we are in the process of abandoning majority of merge() calls and replacing them with the approach you mentioned as #3 “no operation is needed for managed entities”. It was quite complex to implement but my colleague managed that. But some of merges still remain, therefore I asked that question.)

vlad · February 6, 2018, 2:08pm

The entity in my update() test method is really detached. It has the same identifier as an existing entity in the database (id=10).

But that’s not how a detached entity is supposed to work because you are practically recreating a duplicate entry here.

If you want to pass a detached entity, then you should use one that was dereferenced and you kept it all along between requests.

In your case, you dont even need to use merge`. Just fetch the entity and moddify it like this:

private static void update() {
    Session session = HibernateUtil.getSessionFactory().openSession();
    session.beginTransaction();

    Parent parent = session.find(Parent, 10L);

	Child child = new Child();
	child.setParent(parent);
	parent.getChildren().add(child);
	child.setValue("new");
	
    session.getTransaction().commit();
    session.close();
}

See, no merge is needed since the Parent is a managed entity.

If you want to merge a DTO back to an entity, you have to copy the DTO state manually onto an entity that you fetch from the Persistence Context. That’s how it is supposed to work.

The JPA merge is not supposed to take arbitrary transient objects. In your case, the Parent entity contains only the new child entity in the collection, not two entries. If you fetch it from the DB, you will see that there should already be a childd entry there.

mederly · February 6, 2018, 2:34pm

Thank you for the explanation. Actually we have used Hibernate in midPoint in this way (for years), at the cost of having a lot of awkward annotations on entities that have relations as part as their identifiers. So I was convinced it is a JPA/hibernate feature.

vlad · February 6, 2018, 2:59pm

It might work in some scenarios, but it’s not guaranteed to work in all situations.

If you take a look in the JPA specification, you will see that merge was designed for “re-attaching” entities that got detached.

mederly · February 6, 2018, 5:24pm

I don’t want to waste your time, @vlad. But I’d like to understand it really clearly. (Besides other things, I would like to close original StackOverflow question with some reasonable conclusion.)

If you would have a minute to help me understand it, I’d be very grateful.

I have adapted the test code so that now it explicitly creates a detached object by calling session.detach(parent). (I suppose that parent would become detached simply by committing the transaction that created it; but this should be more certain way to do it.)

public class TestMergeParentChild {
    public static void main(String[] args) {
        Parent detached = createAndDetach();
        update(detached);
    }

    private static Parent createAndDetach() {
        Session session = HibernateUtil.getSessionFactory().openSession();

        session.beginTransaction();

        Parent parent = new Parent();
        parent.setId(10);

        Child child = new Child();
        child.setParent(parent);
        parent.getChildren().add(child);
        child.setValue("old");

        session.save(parent);
        session.flush();
        session.detach(parent);                    // <------ this is the difference 
        session.getTransaction().commit();

        session.close();
        return parent;
    }

And now the update() operates on the detached entity, instead of creating a new one. It removes existing child and creates a new one. (An alternative would be to change existing Child value directly but that would involve changing the identifier, which is forbidden, as far as I understand - and it fails as well.)

    private static void update(Parent detached) {
        Session session = HibernateUtil.getSessionFactory().openSession();

        session.beginTransaction();

        // now we don't start from scratch; we use detached entity
        Child oldChild = detached.getChildren().iterator().next();
        Child newChild = new Child();      // this is transient (new)
        newChild.setParent(detached);
        newChild.setValue("new");

        detached.getChildren().add(newChild);
        detached.getChildren().remove(oldChild);

        session.merge(detached);
        session.getTransaction().commit();

        session.close();
    }
}

Now I am not sure how to understand JPA 2.0, section 3.2.7.1, in particular:

The merge operation allows for the propagation of state from detached entities onto persistent entities
managed by the entity manager.

The semantics of the merge operation applied to an entity X are as follows:

If X is a detached entity, the state of X is copied onto a pre-existing managed entity instance X’
of the same identity or a new managed copy X’ of X is created.

If X is a new entity instance, a new managed entity instance X’ is created and the state of X is
copied into the new managed entity instance X’.
(…)

For all entities Y referenced by relationships from X having the cascade element value cascade=MERGE or cascade=ALL, Y is merged recursively as Y’. For all such Y referenced by X, X’ is set to reference Y’. (Note that if X is managed then X is the same object as X’.)

In update method, detached is a real detached entity, is that true?
It had originally oldChild (value=“old”) that gets removed. Now it has newChild (value=“new”). It is transient, or new (as JPA calls it).
When calling session.merge(detached), as per point 1 from the spec, a new managed copy of detached (let’s call it detached') is created and the state of detached is copied into detached'.
As per point 3 from the spec, for all entities Y referenced by relationships from detached, Y is merged recursively. Because there is newChild referenced from detached, session.merge is executed against it.
As per point 2, new managed instance of newChild should be created and state of newChild should be copied into it.

(The specification says nothing about the removed entities.)

So, should this scenario be supported? If not, where is the error in understanding? Is it in the removing the oldChild?

The scenario does not work for Hibernate; failing with the same error as the original one.

(I see that there is a section about lazily loaded items - 3.2.7.2 - which I don’t quite understand. But even after changing Parent->Child relationship to eagerly loaded, the issue persists.)

I am sure I must be missing something. But what? Thank you again.

vlad · February 6, 2018, 5:40pm

Maybe it is an issue. Try to replicate it with the JPA test case template, and if you can, you should create a Jira issue.

Meanwhile, it’s better to use @EmbeddedId since it’s a standard JPA feature and it allows you to reference the entity identifier much better than in your example where the child entity itself becomes the composite identifier.

mederly · February 6, 2018, 6:35pm

Thanks! Done (HHH-12276).

I have tried @EmbeddedId few days ago; it worked for simple parent-child case but I got into problems for more complex case, emulating the usage in midPoint. I will revisit it, maybe it is some stupid mistake on my side.

Topic		Replies	Views
Session.merge issue an "INSERT" while it should issue an "UPDATE" -- Is It The Expected Behaviour? Hibernate ORM	4	363	May 15, 2022
Duplicate association elements when merge Hibernate ORM	1	348	December 8, 2018
Nested relationship with Composite IDs that have Entity Ids Hibernate ORM	3	148	January 18, 2024
JoinColumn on multiple fields not works Hibernate ORM	1	322	February 25, 2021
How to map many to one relationship to itself when using composite id? Hibernate ORM	1	301	February 17, 2023

Merge fails with composite identifier containing association

Related Topics