Skip to content
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

Skip joined entity creation for empty relation (#10889) #11194

Open
wants to merge 1 commit into
base: 2.19.x
Choose a base branch
from

Conversation

noemi-salaun
Copy link

(The following is translated from french with Google Translate, I'm sorry :/ )

In case the root entity has its primary key as a relationship and the joined entity has a simple primary key, the RSM orders the ID of the joined entity before having the ID which defined the root entity.

The ObjectHydrator therefore first tries to hydrate the joined entity. We then fall into a special case where the parent (the root entity) has not yet been created and the hydrator tries to start by creating the child, except that in our case, the child is supposed to be null.

I therefore added the isset($nonemptyComponents[$dqlAlias]) check which we find further in the code and which is made to skip the creation of the joined entity when the parent is already hydrated. I think it makes sense to do the same check in the case where the hydrator starts with the joined entity.

@mpdude
Copy link
Contributor

mpdude commented Feb 4, 2024

What does it mean when the GH10889Hand class has an @Id column that is a one to one association and a generated value at the same time?

@noemi-salaun
Copy link
Author

ah you're right it's an oversight. During my tests I wanted to see how the ObjectHydrator behaved with a simple ID rather than a relationship, and I left an annotation. It doesn't change anything about the problem here but I'm going to remove it ASAP

@noemi-salaun
Copy link
Author

@mpdude It's corrected. I checked, I still have the same problem without the GeneratedValue annotation, and the problem is still fixed with my changes.

@noemi-salaun
Copy link
Author

I don't think the failed psalm static analysis is related to my changes

@noemi-salaun
Copy link
Author

@mpdude
Any info on when these changes might be merge? I am still locked to v2.15 in my project because of this

@Prokyonn
Copy link

We've tested the changes from this PR in Sulu, everything seems to be running without encountering any issues.

However, until this is merged, we also have to lock the doctrine version in our customer projects to v2.15. Given this situation, we would greatly appreciate it if you could prioritise merging this PR.

@derrabus derrabus changed the base branch from 2.18.x to 2.19.x April 17, 2024 20:33
@derrabus
Copy link
Member

if you could prioritise merging this PR.

The PR cannot be merged due to conflicts.

@winiarekk
Copy link

@noemi-salaun will you be able to resolve these conflicts?

@noemi-salaun
Copy link
Author

@derrabus I just rebased my work on the 2.19.x branch, it should be good now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants