Feature propagate on delete cascade foreign key option

[subfusc]

Tell rails to run destroy on appropriate foreign key when on_delete is cascade.

[urkle]

Honestly not sure I like this one.. As I generally use foreign keys to avoid using rails dependent: :destroy functionality (As it's REALLY slow)

[subfusc]

Right now if we have a foreign key with a not null constraint it will crash because the behaviour is not to delete the row from the referenced table but set the key to null. Also, if a table has a foreign key with on delete cascade and we did a rails destroy on a model-class from that table wouldn't it be expected behaviour to also run the destroy for the referencing model for consistency?

Alternatively, if speed is the desired outcome, wouldn't running the SQL directly using the models connection and bypass Rails entirely + let the database handle the keys as expected be the best option?

There is also the possibility of making this default but optional and turn it off globally/per table. Then the developers needing that speed would be aware of the inconsistency between the model and the database.

[urkle]

@subfusc you should be explicit when creating your foreign keys to state on_delete: :cascade as an option. I never leave the default FK that is generated in the migrations and always update it.

t.belongs_to :user, foreign_key: {on_update: :restrict, on_delete: :cascade}, index: true, null: false

This will correctly work and delete the related objects.

Whereas

t.belongs_to :user, foreign_key: {on_update: :restrict, on_delete: :nullify}, index: true

would be the way to declare a foreign key that you want to be "nulled" when the user is deleted.

[subfusc]

@urkle Would that give the desired outcome of running the referenced objects destroy?

[urkle]

If you have foreign keys setup everywhere you shouldn't ever need to use the rails dependent: anything. Rails' dependent: :destroy is simply a "application level" foreign key.. instead of a database level foreign key.

I ALWAYS prefer DB level foreign keys as they are faster than the rails application level implementation.

If you have a need for application-level then you'll should define the association manually instead of using this gem for that model. As this suggested PR would hurt every application that is doing full foreign keys.

Really the only scenario that would occur in would be if you use polymorphic associations since a DB can't define a foreign key on that kind of link.

[ronen]

Sorry for the delay in weighing in. Overall I agree with @urkle -- the general reason to use foreign keys is for enforcing correctness at the DB level rather than the app level, and I use CASCADE precisely in order to allow the DB to delete everything without needing to run rails code. So I don't want to bring this PR in as new default behavior.

In fact, arguably the opposite should hold: if the DB constraint is not set to CASCADE, then in order to delete the record, the app would need to delete the foreign records first, so it could make sense to set the rails association depend: :destroy to trigger that.

But in any case, for a particular app & schema, the developer might have specific reasons to do particular different things than whatever schema_associations has chosen. That might argue for schema_associations to have a simple way to specify extra/custom arguments on the auto-generated associations, globally and/or per-table and/or per-association. If somebody came up with a clean syntax for doing that, I'd be happy to accept a PR. [A really nifty thing would be to make a separate gem that supports storing/reading metadata in the table/column definition, probably as JSON-encoded strings in the table or column COMMENT; then schema_associations could look for appropriate metadata keys to guide the auto creation]

In the meantime, I'd say the short answer is just that schema_associations simply provides a shortcut that takes care of what it considers the common case; but if somebody wants something different they can always define the association manually (schema_associations is careful not to override a manually-defined association).

So I'm going to close this out. But I appreciate the suggestion, it did provide me some food for thought.

Cheers