[hibernate-issues] [Hibernate-JIRA] Commented: (HHH-979) Merge operation should retain existing identifiers

[Eugen Paraschiv (JIRA)]

    [ https://hibernate.onjira.com/browse/HHH-979?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=45557#comment-45557 ] 

Eugen Paraschiv commented on HHH-979:
-------------------------------------

I see the resolution to this is `Rejected` - can someone explain the reasoning behind this? 
This behaviour is something you'd reasonably expect - for example, creating a new entity, setting a random id on it and merging should (in my view) fail with some sort of not found exception. Instead, the detached entity is persisted, and the existing id silently ignored. 
There may be valid reasons to do so - but I do think some quick info on this would be helpful to understand why. 
Thank you. 

> Merge operation should retain existing identifiers
> --------------------------------------------------
>
>                 Key: HHH-979
>                 URL: https://hibernate.onjira.com/browse/HHH-979
>             Project: Hibernate ORM
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 3.0.5
>            Reporter: Aleksei Valikov
>            Priority: Minor
>   Original Estimate: 20m
>  Remaining Estimate: 20m
>
> Currently, merge(...) operation does not retain existing object identifiers.
> Here's a demonstrating test case:
>   public void testIt() throws Exception {
>     final A a1 = new A();
>     final B b1 = new B();
>     a1.setId("A");
>     a1.setB(b1);
>     a1.setD("d1");
>     b1.setId("B");
>     b1.setC("c1");
>     save(a1);
>     
>     final A a2 = new A();
>     final B b2 = new B();
>     a2.setId("A");
>     a2.setB(b2);
>     a2.setD("d2");
>     b2.setId("B");
>     b2.setC("c2");
>     save(a2);
>     
>     final A a3 = load("A");
>     
>     assertEquals(a3.getD(), a2.getD());
>     assertEquals(a3.getB().getC(), a2.getB().getC());
>   }
>   
>   public void save(A a)
>   {
>     final Session s = openSession();
>     final Transaction t = s.beginTransaction();
>     s.merge(a);
>     t.commit();
>     s.close();
>   }
>   
>   public A load(String id)
>   {
>     final Session s = openSession();
>     final A a = (A) s.get(A.class, id);
>     s.close();
>     return a;
>   }
> load("A") returns null since merged objects have newly generated identifiers. Moreover, we get two entities persisted in the DB, not one:
> INSERT INTO B VALUES('4028e4fc067e203f01067e2042690001','c1')
> INSERT INTO A VALUES('4028e4fc067e203f01067e2042790002','4028e4fc067e203f01067e2042690001','d1')
> COMMIT
> INSERT INTO B VALUES('4028e4fc067e203f01067e2042e60003','c2')
> INSERT INTO A VALUES('4028e4fc067e203f01067e2042e60004','4028e4fc067e203f01067e2042e60003','d2')
> COMMIT
> I think a more sensible behaviour is to retain identifiers. This can be achieved with a one-line code patch.
> Before DefaultMergeEventListener.entityIsDetached(...)  calls entityIsTransient(), it should set event.requestedId:
>   // ....
>   Serializable id = event.getRequestedId();
>     if ( id == null ) {
>       id = persister.getIdentifier( entity, source.getEntityMode() );
>     }
>     else {
>       //TODO: check that entity id = requestedId
>     }
>     final Object result = source.get(entityName, id);
>     if ( result == null ) {
>       //TODO: we should throw an exception if we really *know* for sure  
>       //      that this is a detached instance, rather than just assuming
>       //throw new StaleObjectStateException(entityName, id);
>       event.setRequestedId(id);
>       
>       // we got here because we assumed that an instance
>       // with an assigned id was detached, when it was
>       // really persistent
>       return entityIsTransient(event, copyCache);
>     }
>   // ...
> This results in following DB operations:
> INSERT INTO B VALUES('B','c1')
> INSERT INTO A VALUES('A','B','d1')
> COMMIT
> DELETE FROM B WHERE ID='B'
> INSERT INTO B VALUES('B','c2')
> DELETE FROM A WHERE ID='A'
> INSERT INTO A VALUES('A','B','d2')
> COMMIT
> At the moment, I've implemented is as my own listener, but would like in any case to know your opinion.
> Anyways, something like MergeMode (analogous to ReplicationMode) would be also nice to have.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira