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

Cascade remove/set null behavior #278

Open
hrach opened this issue Feb 11, 2018 · 3 comments
Open

Cascade remove/set null behavior #278

hrach opened this issue Feb 11, 2018 · 3 comments

Comments

@hrach
Copy link
Member

hrach commented Feb 11, 2018

Currently, there three options for has many relationships (eg. Author has many Books)

  1. do not set nothing and the other side is nullable: removing the author make the book author-less.
  2. do not set nothing and the other side is not nullable: an exception is thrown;
  3. set cascade remove, the book is removed too;

I see a room for improvement. The main point it is quite easy to forget a cascade behavior, thought, bind a cascade behavior to the nullability is not so much correct.

The default behavior could be to throw an exception if a dependency is wired (e.g. something is in relationship), possible cascade values could be (the current) remove and (the new one) setNullOnRemove.


Also, I'm thinking about solution to the huge relationship, when it may be convenient to do the cascade in storage layer, then something like removeIgnore would be great and refreshAll()/new identity map would reload the correct db state.

@hrach
Copy link
Member Author

hrach commented Feb 23, 2020

E.g. the issue here is that when there is a nullable relationship, the default remove call do a set null without any notice. Not a big issue, but may be rethought as described.

@hrach hrach added this to the v4.0 milestone Apr 26, 2020
@hrach hrach modified the milestones: v4.0, v4.1 May 30, 2020
@hrach hrach removed this from the v5.0 milestone Dec 11, 2021
@mskocik
Copy link

mskocik commented Mar 4, 2022

I came across this one. Maybe I just don't understand it correctly. I would expect, that following code should work:

// returns 3 items
$existingTags = $target->tags->toCollection()->fetchAll();
// want to keep only 2 of them
$toKeep = [$existingTags[0], $existingTags[2]]; 

$target->tags->set($toKeep);
$targetRepository->persist($target);

But NullValueException is thrown that Property Tag::$target is not nullable.
I was able to make it behave as expected with this code:

// returns 3 items
$existingTags = $target->tags->toCollection()->fetchAll();
$tagRepo->remove($existingTags[1]);// 👈 this has been added
// want to keep only 2 of them
$toKeep = [$existingTags[0], $existingTags[2]]; 

$target->tags->set($toKeep);
$targetRepository->persist($target);
// add to line 3

So my question is. Should the first code example work? Or the line with $tagRepo is required?

@hrach
Copy link
Member Author

hrach commented Mar 5, 2022

@mskocik It seems your usecase is rather #205. Anyway, the second example is overcomplicated, the relationships are synced, i.e. calling remove is sufficient. No need to set() & persist on the other side.

@hrach hrach removed the feature label Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants