Wrong FK value update on orphanRemoval & oneToOne when replacing target #5561

Closed
opened 2026-01-22 15:11:17 +01:00 by admin · 4 comments
Owner

Originally created by @akomm on GitHub (May 26, 2017).

Having entity A and B and OneToOne association between them:

A mapping

targetEntity="B",
inversedBy="a",
orphanRemoval=true,
cascade={"persist", "remove"}
nullable=true
onDelete="SET NULL"

B mapping:

targetEntity="A" 
mappedBy="b"
  1. Transaction/flush
  • Create & persist A
  • Create & persist B & assign to A

persisted correctly with A.b = B(id:n)
executed queries:

  • insert B with id = n
  • insert A with A.b = n
  1. Transaction/flush
  • Create & persist new B
  • Assign new B to A and persist A

persisted incorrectly: A.b = null
executed queries:

  • insert B with B.id = n+1 (ok)
  • update A with A.b = n (wrong)
  • delete B where ID = n (triggers cascade SET NULL)

either 2. is wrong and should update A.b = n+1 or when doctrine is smart to only update values instead of delete & insert, then 3. is wrong and shoult not be executed.

Assignment was done from the owning side with API like this:

A::assignNewB() {
  this.b = new B(this);
}
B::__constructor(A a) {
  this.a = a;
}
Originally created by @akomm on GitHub (May 26, 2017). Having entity A and B and OneToOne association between them: A mapping ``` targetEntity="B", inversedBy="a", orphanRemoval=true, cascade={"persist", "remove"} nullable=true onDelete="SET NULL" ``` B mapping: ``` targetEntity="A" mappedBy="b" ``` 1. Transaction/flush - Create & persist A - Create & persist B & assign to A persisted correctly with A.b = B(id:n) executed queries: - insert B with id = n - insert A with A.b = n 2. Transaction/flush - Create & persist new B - Assign new B to A and persist A persisted incorrectly: A.b = null executed queries: - insert B with B.id = n+1 (ok) - update A with A.b = __n__ (wrong) - delete B where ID = n (triggers cascade SET NULL) either 2. is wrong and should update A.b = n+1 or when doctrine is smart to only update values instead of delete & insert, then 3. is wrong and shoult not be executed. Assignment was done from the owning side with API like this: ``` A::assignNewB() { this.b = new B(this); } B::__constructor(A a) { this.a = a; } ```
admin closed this issue 2026-01-22 15:11:17 +01:00
Author
Owner

@Ocramius commented on GitHub (May 26, 2017):

This needs a test case. As it currently is described, it just looks like
something that worked correctly for ages, yet you found a broken scenario,
and the culprit is unclear.

On 26 May 2017 3:52 p.m., "Andriy Komm" notifications@github.com wrote:

Having entity A and B and OneToOne association between them:

A mapping

targetEntity="B",
inversedBy="a",
orphanRemoval=true,
cascade={"persist", "remove"}
nullable=true
onDelete="SET NULL"

B mapping:

targetEntity="A"
mappedBy="b"

  1. Transaction/flush
  • Create & persist A
  • Create & persist B & assign to A

persisted correctly with A.b = B(id:n)
executed queries:

  • insert B with id = n
  • insert A with A.b = n
  1. Transaction/flush
  • Create & persist new B
  • Assign new B to A and persist A

persisted incorrectly: A.b = null
executed queries:

  • insert B with B.id = n+1 (ok)
  • update A with A.b = n (wrong)
  • delete B where ID = n (triggers cascade SET NULL)

either 2. is wrong and should update A.b = n+1 or when doctrine is smart
to only update values instead of delete & insert, then 3. is wrong and
shoult not be executed.

Assignment was done from the owning side.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
https://github.com/doctrine/doctrine2/issues/6474, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakLT7MUU-ajD7UF1oAQW7QtJ4-ThJks5r9tkagaJpZM4NnoGh
.

@Ocramius commented on GitHub (May 26, 2017): This needs a test case. As it currently is described, it just looks like something that worked correctly for ages, yet you found a broken scenario, and the culprit is unclear. On 26 May 2017 3:52 p.m., "Andriy Komm" <notifications@github.com> wrote: > Having entity A and B and OneToOne association between them: > > A mapping > > targetEntity="B", > inversedBy="a", > orphanRemoval=true, > cascade={"persist", "remove"} > nullable=true > onDelete="SET NULL" > > B mapping: > > targetEntity="A" > mappedBy="b" > > > 1. Transaction/flush > > > - Create & persist A > - Create & persist B & assign to A > > persisted correctly with A.b = B(id:n) > executed queries: > > - insert B with id = n > - insert A with A.b = n > > > 1. Transaction/flush > > > - Create & persist new B > - Assign new B to A and persist A > > persisted incorrectly: A.b = null > executed queries: > > - insert B with B.id = n+1 (ok) > - update A with A.b = *n* (wrong) > - delete B where ID = n (triggers cascade SET NULL) > > either 2. is wrong and should update A.b = n+1 or when doctrine is smart > to only update values instead of delete & insert, then 3. is wrong and > shoult not be executed. > > Assignment was done from the owning side. > > — > You are receiving this because you are subscribed to this thread. > Reply to this email directly, view it on GitHub > <https://github.com/doctrine/doctrine2/issues/6474>, or mute the thread > <https://github.com/notifications/unsubscribe-auth/AAJakLT7MUU-ajD7UF1oAQW7QtJ4-ThJks5r9tkagaJpZM4NnoGh> > . >
Author
Owner

@akomm commented on GitHub (Jun 2, 2017):

I had some problem reproduce it, when I noticed, that my inversed and mapped side is inversed in the test, which made it pass.

So the problem is, when you have a OneToOne and orphanRemoval=true from the inversed side.
https://github.com/akomm/doctrine2/tree/test-for-%236474

I know about the inversed side sync requirement, but in case of one-to-one, even when the assignNewProofCopy checks for a "current proofCopy" before replacing it and sets its reference to itself to NULL it still does not work. I'm forced to em->remove on the proofCopy. But it is often so, that in your domain you have certain types which are rather managed via others entity API. It is prone, when you have to always know that you have to explicitly em->remove those managed entity, instead of detaching / removing it as being not referenced anymore. This works well in OneToMany, when the One-Side, even though from orm perspective the inversed side, is managin the ManySide, I want to remove via the One-Side and persist the one side and the changes to propagate to the Many side. Works well. But not with OneToOne.

@akomm commented on GitHub (Jun 2, 2017): I had some problem reproduce it, when I noticed, that my inversed and mapped side is inversed in the test, which made it pass. So the problem is, when you have a OneToOne and orphanRemoval=true from the inversed side. https://github.com/akomm/doctrine2/tree/test-for-%236474 I know about the inversed side sync requirement, but in case of one-to-one, even when the `assignNewProofCopy` checks for a "current proofCopy" before replacing it and sets its reference to itself to NULL it still does not work. I'm forced to em->remove on the proofCopy. But it is often so, that in your domain you have certain types which are rather managed via others entity API. It is prone, when you have to always know that you have to explicitly em->remove those managed entity, instead of detaching / removing it as being not referenced anymore. This works well in OneToMany, when the One-Side, even though from orm perspective the inversed side, is managin the ManySide, I want to remove via the One-Side and persist the one side and the changes to propagate to the Many side. Works well. But not with OneToOne.
Author
Owner

@akomm commented on GitHub (Jun 2, 2017):

facepalm

Initially it was not an bi-directional association and so that is the reason I initial had the mapping side on proofCopy. My model required me to make it bi-directional. After changing it I set reivew as inversed and lept proofCopy as mapped. But why the hell not swap them... I'm checking whether it makes any issues in model, if not its resolved I think.

@akomm commented on GitHub (Jun 2, 2017): *facepalm* Initially it was not an bi-directional association and so that is the reason I initial had the mapping side on proofCopy. My model required me to make it bi-directional. After changing it I set reivew as inversed and lept proofCopy as mapped. But why the hell not swap them... I'm checking whether it makes any issues in model, if not its resolved I think.
Author
Owner

@akomm commented on GitHub (Jun 2, 2017):

Works like a charm.

I think the problem in OneToOne is, that one you switch the reference, the persist is not cascaced to the detached entity, so you have to mark it for removal. And even when you set the reference from the owning side to null in that case, still without explicit persist on it, after detaching the reference on the inverse side and persisting it the persist never reaches the detached entity.

So I think it is not a bug. When a brand new B is persisted (owning side) it is maybe possible to generate a "blind" delete A statement with where A.id = B.a, before inserting the new B. I assume generating probably not required delete statements might be not a desired behavior. But I'm not an expect in this.

If you think the same way this issue can be closed.

@akomm commented on GitHub (Jun 2, 2017): Works like a charm. I think the problem in OneToOne is, that one you switch the reference, the persist is not cascaced to the detached entity, so you have to mark it for removal. And even when you set the reference from the owning side to null in that case, still without explicit persist on it, after detaching the reference on the inverse side and persisting it the persist never reaches the detached entity. So I think it is not a bug. When a brand new B is persisted (owning side) it is maybe possible to generate a "blind" delete A statement with where A.id = B.a, before inserting the new B. I assume generating probably not required delete statements might be not a desired behavior. But I'm not an expect in this. If you think the same way this issue can be closed.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: doctrine/archived-orm#5561