Generators add foreign keys on references

[derekprior]

If you run a generator such as:

rails generate model accounts supplier:references

The resulting migration will now add the corresponding foreign key
constraint unless the reference was specified to be polymorphic.

[sgrif]

Perfect, thanks for working on this. Will merge in the morning.

[senny]

@sgrif I'd hold onto this until 5.0. Arguably references itself could add the foreign key directly. We decided to ship a small implementation first and work towards tighter integration later on.

[derekprior]

@senny : that would invalidate any foreign key migrations that were added by people running 4.2 (or Foreigner), forcing people to modify past migrations or schema to upgrade to 5.0.

Is that a breaking change you want to add for 5.0? If so, I'm still not certain why this implementation couldn't ship in 4.2, other than that it increases the surface area of that breaking change I guess?

[senny]

@derekprior we have the possibility to add it with an option foreign_key: true and keep the default false. This won't force people to change the migration.

As for the reason to not ship it with 4.2, we haven't yet discussed how add_foreign_key should be integrated in the rest of the migration DSL. There are people that don't use foreign keys so we need to keep this in mind.

[guilleiguaran]

Agree with @senny on this

[sgrif]

I've brought this up a couple of times now and the consensus has been that we felt it was a good idea by default. It sucks to change our mind on this after someone has gone and done the work...

I think rails as a whole should be encouraging referential integrity, and generators are the best way to do that. Perhaps we should add a config option which defaults to true like we do for other cases that not everyone uses (like fixtures).

I do feel very strongly that this should be on by default, however. I spend a lot of time mentoring new developers who are starting with rails. This would have a very positive influence on teaching best practices in this regard, same as generating tests.

I agree a foreign key option on references would be nice but we can add that later, and I don't think it should block this.

Would you guys be comfortable with this in 4.2 if we added a config option to turn it off for the people who specifically don't want FKs?

[guilleiguaran]

afaik Rails didn't have support for foreign keys in past releases because @dhh wasn't a big fan of it (like many other people).

Now we have support for foreign keys in migration DSL but I'm not sure if everyone is ok with the idea of make it the default when generating references in all new apps.

/cc @dhh

[derekprior]

I feel like it's worth pointing out again that this doesn't change the behavior of references or add_reference, only the migration generator.

If the foreign key isn't desired it can be removed, but like indexes they are well-proven to be useful. Encouraging their use is a good thing. Otherwise, what's the feature in rails for?

[rafaelfranca]

Yeah, agree it is fine to ship this with 4.2 since we are not changing references to create automatic foreign key.