onDelete cascade for paranoid tables

[lao]

I have a relationship between two paranoid tables (User and Email) and I have the onDelete: 'cascade' option on the relationship ( Email.belongsTo(User, onDelete:'cascade') ).

The problem is that when I delete the user his email is not being deleted by cascade.

Is this a bug? Or this is how it is supposed to work?

I am using sequelize 2.0.0-rc2 on a postgres database.

Thanks.

PS.: Please take a look at the code I used for test:

var Sequelize = require('sequelize')
    , sequelize = new Sequelize('test', 'test', 'test', {dialect: 'postgres'});

var User = sequelize.define('User', {
    username: Sequelize.STRING,
    birthday: Sequelize.DATE
}, {paranoid: true}); //Without paranoid the code works fine

var Email = sequelize.define('Email', {
    email: Sequelize.STRING,
    primary: Sequelize.BOOLEAN
}, {paranoid: true}); //Without paranoid the code works fine

Email.belongsTo(User, {onDelete: 'cascade'});

sequelize.sync().success(function () {
    var userCreatedInstance = null;

    var deletedUserId = null;
    var cascadeDeletedEmailId = null;

    User
        .create({
            username: 'user1',
            birthday: new Date(1986, 06, 28)
        })
        .then(function (_user) {
            userCreatedInstance = _user;
            deletedUserId = userCreatedInstance.id;
            return Email.create({email: 'email1@test.com', primary: true, UserId: userCreatedInstance.id});
        })
        .then(function (createdEmail) {
            cascadeDeletedEmailId = createdEmail.id;
            return userCreatedInstance.destroy();
        })
        .then(function () {
            return User.find(deletedUserId);
        })
        .then(function (foundUser) {
            if(foundUser){
                throw new Error("Shouldn't find the user with this id");
            }
            return Email.find(cascadeDeletedEmailId);
        })
        .then(function (foundEmail){
            if(foundEmail){
                console.log(foundEmail.values);
                throw new Error("Shouldn't find the email with this id");
            }
        });
});

[mickhansen]

You need the opposite relationship aswell, User.hasMany(Email, {onDelete: 'cascade'});

[lao]

Thanks for the fast reply. I tried with the opposite relationship as well but it didn't work. =(

Is the code above with the opposite relationship working on your machine?

[mickhansen]

Did you force sync?

[lao]

I was deleting the db myself. But now I just set the option force to true but it is still not working.

Here is the updated code:

var Sequelize = require('sequelize')
    , sequelize = new Sequelize('advo_test', 'advo', 'preveza', {dialect: 'postgres'});

var User = sequelize.define('User', {
    username: Sequelize.STRING,
    birthday: Sequelize.DATE
}, {paranoid: true}); //Without paranoid the code works fine

var Email = sequelize.define('Email', {
    email: Sequelize.STRING,
    primary: Sequelize.BOOLEAN
}, {paranoid: true}); //Without paranoid the code works fine

User.hasMany(Email, {onDelete: 'cascade'});
Email.belongsTo(User, {onDelete: 'cascade'});

sequelize.sync({force:true}).success(function () {

..... REST OF THE CODE OMITTED ...... 

[mickhansen]

I completely missed the paranoid part.
No unfortuneatly cascades won't work with paranoid deletes - Since a delete doesnt actually happen there's nothing for the database to cascade.

Edit: The onDelete: 'cascade' option is for setting up database cascading, there's no cascading being done by sequelize at the moment.

[lao]

Ahh. =/

Is cascade for paranoid tables a feature that you guys are planning on adding on other versions of sequelize?

I was reading some older issues and I found this one: #2404

And I saw there was a patch to make it work on sequelize 1.7. So my question is, did you guys remove the cascade for paranoid tables on version 2.0.0? Is there a reason for that?

Thanks again for the fast reply. =)

[mickhansen]

That issue doesn't really have anything to do with your issue here.
Cascading paranoid tables is something we've discussed previously - It's likely something we want to do but it's not a top priority right now.

[lao]

True. I thought that it had but that was just related to the transaction.

Thanks again. That feature is probably something we are going to work on here. I will post a PR later if we find a good generic solution for it.

PS.: I noticed that I created a reference between this issue with the issue related to the transaction not been passed on the option hash for paranoid tables. Should I remove the reference by updating my comment? Or there is a better way to remove the reference?

[felixfbecker]

[diosney]

Any updates on this? It is planned to be included in v4?

[felixfbecker]

@diosney if it was, it would have the v4 milestone 😉
Nobody of the team is working on it but if someone wants to implement it we can include it :)

[diosney]

Jajaja! You made my day 😄 That was an indirect? If so I'm willingly to do it with the correct guidance 😉

[felixfbecker]

Almost all of Sequelize features come from user contributions 😉
I am the wrong maintainer to guide you as I didn't write any code related to this - maybe you should do a git blame on the association files and ping the author. You can also join us on Slack.

[ajmueller]

@lao what solution did you eventually come up with? I need the same functionality on an app I'm building, so I'm just using managed transactions to do this.

[richardpringle]

+1for the feature

[lao]

@ajmueller unfortunately we didn't work on a generic solution. I think we ended up using managed transactions as well. But I am not sure. It was a few years ago and I don't work on the project anymore. =/

[ajmueller]

@lao thanks for the reply. I actually ended up changing to not use the paranoid feature for deletes. Instead I built a common interface whereby users must manually type "DELETE" into a text input in a modal if they want to delete something.

[trjstewart]

Anyone made any progress on this? Considering spending some time on it but not 100% sure where to start.

[alexey2baranov]

Hello!
Why this issue closed? Is it implemented?

[blankstar85]

Would be nice if the docs would indicate that the onDelete needs to be in the table creation. Sat here for a hour trying to figure out why the cascade wasn't working. I don't use the sync as I hand write the migrations to have better control of the DB. Thanks for the info on this issue. Glad i was able to find it :D

[utf4]

Desperately waiting for this. For now I had to handle cascade delete manually in model hooks.

[fareshan]

+1 for the feature

[aderiushev]

+1

[subashCognitive]

+1

[subashCognitive]

+200

[subashCognitive]

+1000000000

[renancsilva86]

Since 2014 waiting for this feature!

[mustyzod]

Since 2014 waiting for this feature!

Well...i just did my part. Enjoy!

[lukasgjetting]

For anyone else looking for an easy solution, the following seems to work just fine:

UserGroup.addHook('beforeDestroy', async (group) => {
	await group.setUsers([]);
});

If you need to do the same when users are deleted, you would need to set the same hook on the User model.

Our model associations are defined as follows:

UserGroup.belongsToMany(models.User, {
	through: 'UsersInUserGroups',
	foreignKey: 'userGroupId',
	otherKey: 'userId',
});

User.belongsToMany(models.UserGroup, {
	through: 'UsersInUserGroups',
	as: 'groups',
	foreignKey: 'userId',
	otherKey: 'userGroupId',
});

[aldasmallthings]

i'm surprised this is still not a thing. HOW has this ticket been open for 7 years? I'm starting to get a lil disappointed, maintainers...

[tommybananas]

It's been open for 7 years because scalable applications don't handle cascades through an ORM layer.

[Shahaed]

It's been open for 7 years because scalable applications don't handle cascades through an ORM layer.

@tommybananas What do you mean by this? Are you saying scalable applications don't use an ORM layer? Then they wouldn't be using Sequalize. Or they do but have some of their logic in SQL and others in Javascript? Cause that introduces another scalability issue.

[hayksaryan]

My take on this is (maybe wrong):

We should completely ditch the usage of onDelete: 'cascade', because it's not optimal for tables with large number of rows anyway.

Instead, we could do the deletions manually:

1. add an beforeDestroy and beforeBulkDestroy hooks in the model,
2. wrap all of the destroy queries of the referenced tables (in correct order) in a single transaction
3. and run it inside the callback of the hooks.

This approach helps us perform deletion logic using bulk operations therefore making the operation faster and lighter.

Correct me if I am wrong, better ideas and comments are welcome.

[nicoabie]

I had a new years idea and created this package to try a solution for this issue using triggers.
I believe it is a good idea as entity hooks shouldn't know (in my opinion) about other entities.
And It is also possible that SQL engines use triggers behind the scenes or some similar mechanism to achieve the cascades.

https://github.com/ReMatter/sequelize-paranoid-delete

It is in development but I'd like to know your thoughts.

[ephys]

Should we delete any data from the database at all?

If you're from the European Union or have customers from the European Union, the right of erasure means you'll have to hard delete some data, not just soft delete. (disclaimer: not a lawyer).

[haykerman]

@nicoabie Of course, using cascading deletes is a matter of personal choice, and having a solution to keep using them is great (although I am a bit worried about performance of your solution on large datasets).

But I'd like to bring the focus on a bigger question:
Should we delete any data from the database at all?

• Storage is getting cheaper and cheaper.
• A lot of times we may physically delete the data, but later a business requirement comes in which assumes we kept the data. Say, a need to generate reports about how many users deleted their account.
• There are risks with cascading deletes, e.g. you might unexpectedly delete some data that shouldn't have been deleted.
• Cascading deletes may have bad performance on large datasets.

So, my preference is to use soft deletes and handle deletion of related entites manually. Implementation of this may differ.

I am currently using the repository pattern and update all related/child entities in root/parent entitie's repository when the root/parent entity is deleted.

This way you don't lose database integrity, you are more flexible and ready for future business requirements.

As a bonus, your database can tell you how the data has been created, transformed, and deleted at any point of time. You never lose information.

There are also drawbacks, like more storage required and more manual work, but you are also always in control, and this is, imho, very useful when working on enterprise solutions.

But, as always, it depends 🙂

[haykerman]

@ephys Agreed. That kind of data can be an exception and may be handled separately.

The Right to Erasure also known as the
‘Right to be Forgotten’, is a new right being
introduced to individuals under the GDPR.
The underlying principle of this right is that
when there is no compelling reason for
their data to be processed, the data subject
can:

• Request the data controller erase/remove
  their personal data.
• Stop any further distribution of their
  personal data.
• Potentially stop third parties from
  processing their personal data.

Quoting from this website.

This basically means a separate logic should be implemented for physically removing data on user's request, while using soft deletions in any other use case.

But I am not an expert, this is just an assumption.

[nicoabie]

Here we are talking for a solution to the unsupported cascade deletes in paranoid mode (meaning setting the deletedAt field)

One solution proposed was to use hooks which IMHO will be getting more responsibility than they should.

The proposed approach is to automatically creating triggers when running migrations to emulate the cascade in paranoid mode.

Is there any other way to do this that isn't cumbersome or error prone?

[haykerman]

Well, my solution is to not have a solution at all for this specific problem. Because deleting related data might be out of the scope of the responsibilities of an ORM library, considering it being an application's own architectural problem.

[nicoabie]

@haykerman I truly do not understand what you are saying. "my solution is to not have a solution at all for this specific problem" the whole point of this thread is to find a solution for this problem.
"Because deleting related data might be out of the scope of the responsibilities of an ORM library" we would not be deleting any data because it is paranoid mode (soft delete) and it won't be responsibility of the ORM but the DB as if we were using the real cascade.

No offense or disrespect but have you read this thread?

[haykerman]

I have read the thread and I am well aware of the problem, as I've been investigating it for more than a month now.

What I am proposing is implementing deletion logic for related tables in application code, not on database level, because it's business logic. It's neither ORM's nor DB's responsibility to implement/contain this kind of business logic (imo, though this is debated). It's preferable to keep the DB as "dumb" as possible and treat it as a data store (only with some constraints).

I am aware that some people implement the solution on DB level (through triggers or stored procedures).

As @tommybananas mentioned:

...scalable applications don't handle cascades through an ORM layer.

But if it's absolutely necessary and applicable in your (and anyone interested) case, go ahead and implement a solution (as @nicoabie did, BTW good job).

To summarize, I don't think an ORM/DB should handle cascading soft deletes. But this is just my opinion.

For more details, I propose the community to do their own research on how this kind of problems are solved in existing enterprise solutions, what are the cons and pros.

I am always available for discussion (sequelize gitter).

[ephys]

I really like the trigger suggestion posted above. If it works we should look into importing it inside of sequelize itself. @nicoabie if you're ever interested in porting this inside of Sequelize, let me know :)

[nicoabie]

@ephys I can't believe one year passed and I haven't finished that package.
My idea now is working on a cli that will scan an existing db and ask which delete strategy one wants to add to the different relations between tables.

We need to support the case of existing projects.

I think we could port it into sequelize once it is proven to work. I'll try to work on it more and let you know its status.

[nicoabie]

@ephys package has just been published https://www.npmjs.com/package/@rematter/sequelize-paranoid-delete

All are welcome to try it and create issues if experiencing problems

[rathodajay2021]

I have a relationship between two paranoid tables (User and Email) and I have the onDelete: 'cascade' option on the relationship ( Email.belongsTo(User, onDelete:'cascade') ).

The problem is that when I delete the user his email is not being deleted by cascade.

Is this a bug? Or this is how it is supposed to work?

I am using sequelize 2.0.0-rc2 on a postgres database.

Thanks.

PS.: Please take a look at the code I used for test:

var Sequelize = require('sequelize')
    , sequelize = new Sequelize('test', 'test', 'test', {dialect: 'postgres'});

var User = sequelize.define('User', {
    username: Sequelize.STRING,
    birthday: Sequelize.DATE
}, {paranoid: true}); //Without paranoid the code works fine

var Email = sequelize.define('Email', {
    email: Sequelize.STRING,
    primary: Sequelize.BOOLEAN
}, {paranoid: true}); //Without paranoid the code works fine

Email.belongsTo(User, {onDelete: 'cascade'});

sequelize.sync().success(function () {
    var userCreatedInstance = null;

    var deletedUserId = null;
    var cascadeDeletedEmailId = null;

    User
        .create({
            username: 'user1',
            birthday: new Date(1986, 06, 28)
        })
        .then(function (_user) {
            userCreatedInstance = _user;
            deletedUserId = userCreatedInstance.id;
            return Email.create({email: 'email1@test.com', primary: true, UserId: userCreatedInstance.id});
        })
        .then(function (createdEmail) {
            cascadeDeletedEmailId = createdEmail.id;
            return userCreatedInstance.destroy();
        })
        .then(function () {
            return User.find(deletedUserId);
        })
        .then(function (foundUser) {
            if(foundUser){
                throw new Error("Shouldn't find the user with this id");
            }
            return Email.find(cascadeDeletedEmailId);
        })
        .then(function (foundEmail){
            if(foundEmail){
                console.log(foundEmail.values);
                throw new Error("Shouldn't find the email with this id");
            }
        });
});

You can overcome this issue by using afterDestroy hook.

var User = sequelize.define('User', {
username: Sequelize.STRING,
birthday: Sequelize.DATE
}, {paranoid: true
hooks: {
afterDestroy: function (instance, options) {
instance.getEmail().then(email=> email.destroy()); // Softdelete on email table
console.log('after destroy: destroyed');
}
}
})