-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hibernate fails on complex orphanDelete=true cases and bump version upon merge calls involving entities without change #7462
Comments
Thanks @masini . Will you be able to provide a test? |
on the mailing list, @emmanuelbernard suggested that this might be caused by us having disabled automatic management of bi-directional relations. You might want to verify that all relations are consistently updated on both sides? See examples of "righ" and "wrong" here: |
Some information:
|
Also was mentioned that versioned entities were forcing version changes even if no change was actually real. |
I tried a minimal project to reproduce what you saw especially the version bump. |
It tooks me hours to clean as much as I could from the original project with many submodules and classes, now it has only 70 classes and tests to reproduce the problems. To reproduce unzip the attached "reproducer" and from project dir:
This should run two tests:
Let me know if you need further infos to understand the problem. |
Thanks @masini I've spent a few hours looking at your test. You are using Lombok which making the debugging of collection mutation much harder (not that it's simple initially). First off "org.hibernate.HibernateException: A collection with cascade="all-delete-orphan" was no longer referenced by the owning entity instance" can come from a few areas:
You use Lombok so I could not figure out whether Lombok was messing with 1. According to some people, it can as it changes the value of the collection when using the builder https://stackoverflow.com/a/49106446/200911 (the builder is used at creation time so that does not full compute to me. I noted quite a few calls to change the property but they seemed to come from Jackson deserializing things which I imagine are net new objects. I see that you use identity equality in some entities and I am pretty sure that could potentially mess up with the I understand that these proposed explanation do not explain why it was working in 1.2 and not in 1.3 Alpha. But we did disable the association management from being handled by the bytecode to having to be manual (a good practice). I suspect the bug shows up because your manual code does not update both sides of a relationship somewhere. I did not dig through your code to see if that was a correct assumption, it's quite hairy and I was running out of time. Another question I have, in |
Hi Emmanuel, thank you for your deep analisys. I think that the setPiattoordinato is done during deserialization. The collection and the owner are also using @JsonManagedReference and @JsonBackReference, so that is Jackson that during deserialization take care of settings correctly the pointer around of the data coming from the client. And debugging I can see that this is done correctly. Maybe this is really the effect of a change tracking changes in Quarkus code ? From what I understood debugging, I noticed that the problem come from the only collection mapped with orphanRemoval = true, which is PiattoOrdinati.abbinamento. This come also from the client, is deserialized and when it is merged for some reason two collection instances exists with the same content, one pointed by PiattoOrdinato.abbinamento and the other not point by any other owning side. From what experience what is wrong in our mapping that can cause this kind of problems ? |
Now short answers to some of your questions:
|
only I tried to use the IntelliJ IDEA plugin indeed but it did not help me on the debug side. I am probably not familiar with the tool enough.
Can you provide some info in how you found the double collections? I have not been able to find them in the debugger but that's the core of the problem. |
The original object deserialised by Jackson is: The abbinamenti collection is somewhere transformed to a PersistentBag but in a way I don’t fully understand, there must be still some enhancement magic under the hood because the setter method is never called and the collection from ArrayList is transformed to a PersistentBag. Infact, here we can see that the unreachable abbinamenti/PersistentBag@15547 point to his owner “PiattoOrdinato” which point to another abbinamenti/PersistentBag@13569 |
Right we are getting to the core of the problem. |
Hi @emmanuelbernard, the object deserialized by Jackson has only ArrayList (picture number 1), the PersistentBag come when we enter inside the EntityManager#merge method (picture number 2), why do you think the opposite ? |
@masini Ah I did not know it was in the merge. You said the original object was and right there was the cast to PersistentBag. |
In the second TestMergeOrdine.testCreateAndMergeOrdine "salvaOrdine" call, I see that inside HasSaveOrUpdate.saveOrUpdateInternal (from OrdiniService), then the ordine entity is merge returning a different Ordine instance as expected but it still contains reference to the ArrayList and not the PersistentBag. As if the merge operation did not cascade to the abbinamenti instance. The other copllections of Ordini seem PersistentBag alright like menus and piattiOrdinatti @masini @Sanne can you get a second pair of eyes maybe you will see why the cascade is not happening (e.g. wring mapping??) |
Oh and importantly, during the merge I do see the PersistentBag creation of the abbinamenti instances, so it's as if hibernate was not setting the values of these collections. |
Correct @emmanuelbernard, the created PersistentBag then raise the Exception. I want to debug again, may be I can notice something, but I'm pessimistic since you haven't seen it.... @Sanne help us :) |
I tried to reproduce with a simpler Foo ->orphanDlete=true -> Bar but this one works perfectly. |
hi all, looks like I need to catch up on a lot of reading :) TBH I also need to finish some other PR reviews first. But at first glance, I see being mentioned this is about a merge and with Jackson .. which raises some red flags as there are other open issues related to that; what Jackson does as "optimisations" are very tricky to spot changes in how default fields of classes are being initialized. I'll spare you the details, but the short story is that entities being deserialized by Jackson don't have the full state that we would expect. So @masini do you think you could extract a reproducer with & without jackson and compare? See also #2815 ; to be clear this is how Hibernate ORM always did things so I'm not classifying it as a regression; we should fix it but it's rather complex and I won't be able to do it for this cycle. I'd recommend to not use Jackson on managed entities. |
I did some other experiments, I hope that with this data you can understand what is going on. During "merge" When this is true then during The proof of this behaviour is that inverting the So the key is “when” is loaded, during the first query or during the subsequents lazy fetching Can you add an |
I decided to track the version bump problem in a separate issue #7770 |
Back on the But if you look at it, you will find
I suspect in the end that we might find the same problem we found for #7770 but we can't rule things out. |
Thank you for keeping private, I'll create some data so that we can understand better what is going on. |
Pushed a replicator of the original Exception. This is very rude now, but it works. Start Quarkus in dev mode and then:
you will get the Exception:
Let me know. |
@masini yes it looks like a good reproducer. I would try and remove more if possible. Like the embedded object, I wonder if it is necessary. Likewise the GenericEntity, if the error happens without version object we can remove GenericEntity altogether to get to the core of the problem. |
Ok, I created a new version
I also created a new test method It's pushed on github, let me know if your thoughts. |
I have similar problem.
I've added one record to the database:
In result I've got error:
Here is the reproducer code-with-quarkus.zip |
Hi, I got exactly the same kind of issues. One parent with two children lists. If only one is "loaded" before using "merge", it works fine, if both are loaded, got the "A collection with cascade....", |
I think the problem is in Hibernate when entities are enhanced in build phase. I've reproduce this problem in plain hibernate with hibernate-enhance-maven-plugin. I've created bug in Hibernate https://hibernate.atlassian.net/browse/HHH-14387 |
From what I can see, HHH-14387 was fixed upstream, and Quarkus upgraded to the version of ORM that fixes the problem. I also ran this reproducer: And tests pass on Quarkus 2.14.0. So, I'll close this issue as fixed. Feel free to reopen or ping me if that's not true. |
I've ran my test case on Quarkus 2.14.0.Final and there is still an exception. As I can see HHH-14387 has been fixed in Hibernate 6.1.3 but Quarkus uses Hibernate 5.6.12.Final so the fix is not included. I've attached updated test case: Run It should pass but there is an exception: |
The fix was backported to 5.6.10. See https://hibernate.atlassian.net/browse/HHH-14387.
Thanks, I'll have a look. It may just be a different bug. |
So that was just plain wrong, the fix was only in 6.0, but Quarkus supposedly changed default settings in 2.12 or 2.13 which should have fixed the bug. Apparently it didn't, at least not in all cases. Still investigating. |
Ok, we need patches in Hibernate ORM (which we don't have in Quarkus 2) on top of changing a default setting (which we did in Quarkus 2.13) in order to fix this. Reopening; most likely we will have to wait for Quarkus 3 (and Hibernate ORM 6) before this gets fixed. This shouldn't take ages, so I guess that's fine. |
Hibernate ORM 6.2 / Quarkus 3 is apparently still affected, after upgrading the reproducer
|
Same problem in Quarkus 3.5: https://github.com/yrodiere/quarkus-playground/tree/i7462
|
Still present in Quarkus 3.12, reported upstream as https://hibernate.atlassian.net/browse/HHH-18389 |
Describe the bug
A production application, working with 1.2.1 and ported to 1.3.0.Alpha2 stopped working with the aforementioned Exception during merge.
Expected behavior
No HibernateException
Actual behavior
An Exception is thrown
Environment (please complete the following information):
java -version
: 1.8.0_152-b16Additional context
The code and Hibernate version are the same, but something in the Quarkus integration code is driving that difference in behaviour.
The text was updated successfully, but these errors were encountered: