silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
721 stars 821 forks source link

Clean up DB deletion queries #5290

Open phptek opened 8 years ago

phptek commented 8 years ago

Quite some time ago there was some debate around cleaning-up intermediary relations when one or the other side of a ManyMany relation was deleted, for internal consistency. The arguments for this at the time were a). we should be doing this anyway, it's just lazy not to b). Dissimilar results for ORM vs non-ORM queries.

See: https://groups.google.com/forum/#!topic/silverstripe-dev/nKc6pvDAvNI

Has this gotten anywhere since? Given that we have no solution for this at the moment, then candidate solutions could be ORM based or RDBMS adapter based - using native DB constraints, but with a preference for the latter.

tractorcow commented 8 years ago

It would be hard when versioned dataobjects might exist on multiple stages, but share a many_many table. In which case, which stage should cause these tables to be cleaned up? What about records that are restored from history?

Friksel commented 8 years ago

+1 I agree with @phptek . I don't understand why many_many tables don't have versioning/draft-tables too. (Or just take out all versioning from Silverstripe (and speed things up!) )

I recently had some nasty issues with this causing unexpected behaviour. I also got this feedback from clients and colleges I was working with. I like Silverstripe a lot, and it's well thought through. So I don't really understand why this is. I tried to explain better on these issues here: https://github.com/silverstripe/silverstripe-cms/issues/1462

tractorcow commented 8 years ago

I don't understand why many_many tables don't have versioning/draft-tables too. (Or just take out all versioning from Silverstripe (and speed things up!) )

The main reason is that many_many tables themselves aren't actually dataobjects, and thus can't have extensions.

Once https://github.com/silverstripe/silverstripe-framework/issues/4932 is implemented we'll have some kind of solution for versioned many_many.

sminnee commented 6 years ago

This has been listed as a bug since waaay back - #1903

This should be relatively uncomplicated in SS4, since the versioning work Damian has mentioned has been completed.

tractorcow commented 6 years ago

New solution is use many_many through with cascade_deletes, which handles all the logic I was concerned about (i.e. only deletes mapping if draft row is deleted, but won't break draft relations on pure unpublishes).

We don't have a retro-active cleanup task however, in case users don't jump through all the hoops to setup those cascade relationships.

ec8or commented 4 years ago

Having orphaned records in the many_many table also breaks queries on null:

DataObject::get()->filter([
    'DataGroups.ID' => [null, 1, 4]
]);

This will return records who used to belong to a DataGroup that has been deleted, as well as records who do not belong to any groups at all.