I think changing the semantics of `add_foreign_key` is best saved for another discussion. The naming change seems to be contentious enough by itself. ;)
A quick summary of the tradeoffs for human readable names: + Consistency with index naming + When they show up in query plans, they'll be understandable - They can fall out of date - They can be too long and require manual naming To my mind, the code for renaming them shouldn't be a huge leap from the same code that's there for indexes. The most obvious issue is support for constraint renaming in different databases. Postgres only has this in 9.2+, so we'd have to do version checks. By contrast index renaming support goes back to at least 8.0. I have no idea about MySQL. That said, the foreigner gem (which this built-in functionality replaces) has been generating human readable FK names without renaming them for ages. I'd be really interested in knowing whether this was a major issue for people using it (other than me). From what I can see, this got brought up once (https://github.com/matthuhiggins/foreigner/issues/141) and wasn't discussed. I'm getting the feeling this is an issue which just needs someone who's in charge of activerecord to make a call. I'd like to have the readable names back myself, but it's not my call and opinions are pretty divided on here. On 17 January 2015 at 12:08, Will Bryant <will.bry...@gmail.com> wrote: > True - good point. Using random values is a bit weird though, since two > people running the same migration will get different names, and then their > schemas will never quite match. This means dev machines’ schemas will > differ from staging/test environments and them from production. > > Wouldn’t a simple CRC hash of the input data equally solve the issue of > names potentially getting out of date, without introducing non-determinancy? > > More way-too-late bikeshedding: IMHO the method in 4.2 should be renamed > to add_foreign_key_with_constraint. A foreign key is just the key for an > association. A foreign key constraint is a different concept to a foreign > key. > > IMHO it’s really important that users understand the difference, and only > add constraints when they want them. > > The thing is, if you live your life on recent versions of PostgreSQL, they > work well enough to be usable. But on MySQL particularly - and in > PostgreSQL before they implemented the special weakened lock types - they > cause a lot of unintended side effects because they result in a standard > shared lock being taken on the parent row. > > On one of my big apps we put FKCs in in ~2008, and we’re now actually > going through dropping FKCs when we get the chance, because they just cause > too many problems at runtime. > > Basically, if table A is a parent to table B, and you have an FKC linking > the two, then an update to a B row will take some kind of shared lock on > A. But it’s also common to have a form for A records which nests B > records, and then in practice the #update action is going to save A, and > then save the Bs… and then you get deadlocks. > > I’ve been finding this sort of problem (there’s others) comes up so often > that I’ve gone right off the idea of foreign key constraints, and now tell > ppl to only add them when they specifically need them. (And I find usually > it’s relatively rare that you do in practice because a lot of parent tables > don’t get deletes, and those that do you tend to want to seralize things > using update locks anyway.) > > TL;DR I would like people to learn from our mistakes and not to > accidentally get foreign key constraints when they think they are just > adding a foreign key :). > > > On 17/01/2015, at 23:32 , Yves Senn <yves.s...@gmail.com> wrote: > > Yes, they are shown in constraint error messages, but so are the metadata > (table name and referenced column name). > Following are example messages: > > MySQL: > ----------- > ERROR 1452 (23000): Cannot add or update a child row: a foreign key > constraint fails (`activerecord_unittest`.`fk_test_has_fk`, > CONSTRAINT `fk_rails_bde121d5a6` FOREIGN KEY (`fk_id`) REFERENCES > `fk_test_has_pk` (`id`)) > > PostgreSQL: > ----------------- > ERROR: insert or update on table "fk_test_has_fk" violates foreign key > constraint "fk_rails_bde121d5a6" > DETAIL: Key (fk_id)=(98493849) is not present in table "fk_test_has_pk". > > Both messages have all the detail needed to understand what's going on. > From that perspective > I don't see the need to duplicate the metadata in the constraint name. > > Simon: I didn't know about that. Is there a discussion or documentation > that goes into > further detail? I'd like to read up on these planed changes. This could > affect what > implementation we are going to pursue. > > Cheers, > -- Yves > > On Saturday, January 17, 2015 at 2:01:25 AM UTC+1, will.bryant wrote: >> >> They may not be shown in query plans, but they are shown in constraint >> error messages, and the possibility of hitting those errors is the whole >> reason one adds foreign key constraints. It’s helpful to have an >> indication which kind of constraint was invalidated without having to look >> through the schema. >> >> We do already tolerate the index names being irrelevant if they become >> inconsistent, but in my personal experience that doesn’t happen very often >> at all, since I’ve found indexed columns tend to be important and keep >> their names. >> >> So I would vote for meaningful names too. Just my 2c. >> >> >> On 16/01/2015, at 01:42 , Yves Senn <yves...@gmail.com> wrote: >> >> Hey Chris >> >> Sorry for my late response. >> >> tl;dr: I'd rather not go for consistency with index naming but for a >> consistent hash approach. >> >> We have descriptive names for indexes because they are shown to the user >> in query plans. >> This is not the case with foreign keys. When inspecting a database you >> can look at the >> constraint metadata. I don't see much value in encoding that metadata >> into the name. >> >> In my opinion the drawbacks in the implementation are more sever. We >> faced a lot of issues >> when automatically renaming indexes. There are still cases where they >> fall out of sync >> with the actual metadata. Having descriptive names, which no longer >> represent the metadata >> is way worse in my opinion. >> Also constant renaming when renaming tables and columns is not very >> practical. The added >> issue of the limited size makes this even worse. >> >> My gut feeling is in favor of constant non-descriptive names. I do >> remember that I implemented >> that exact behavior but we pivoted to purely random names. I'd like to go >> through our archived >> discussions regarding that change to make sure we have all the needed >> information. >> I haven't had the time to do so. >> >> Cheers, >> -- Yves >> >> On Thursday, January 15, 2015 at 1:25:18 PM UTC+1, ch...@sinjakli.co.uk >> wrote: >>> >>> My own preference is towards the longer, descriptive names. Partly for >>> consistency with index naming, and partly because it's nice to be able to >>> glance at them and know what they are (though I understand the argument for >>> them falling out of date). >>> >>> I'll go ahead and make a PR for that option. If there's serious >>> objections to it, hashing is easy enough to add. Thanks for sharing your >>> thoughts on this! >>> >>> Cheers, >>> Chris >>> >>> On Tuesday, January 13, 2015 at 9:19:37 PM UTC, James Coleman wrote: >>>> >>>> As far as the Postgres limit, that also applies to index names. So I >>>> don't see why we need to handle it any different for foreign keys (i.e., >>>> with indexes it just means that it fails on creation if the auto-generated >>>> one is too long and the dev has to provide a manual name.) >>>> >>>> On Tue, Jan 13, 2015 at 4:15 PM, Amiel Martin <am...@carnesmedia.com> >>>> wrote: >>>> >>>>> My vote would be to be consistent with the way index names work; the >>>>> long human readable version. >>>>> >>>>> -Amiel >>>>> >>>>> On Tue, Jan 13, 2015 at 12:31 PM, Ken Collins <k...@actionmoniker.com> >>>>> wrote: >>>>> >>>>>> Though I have not felt the pain, I have been a long time user of >>>>>> structure files in legacy environments and think consistent names are a >>>>>> good idea. On the flip side, the same could be said for constraint name >>>>>> “churn” too. I have seen this in big Oracle teams and unavoidable to my >>>>>> knowledge and/or only fixed with a custom script after the rake migration >>>>>> task. >>>>>> >>>>>> All that said, I think this is something Rails could solve and the PG >>>>>> limit does seem like a big constraint too. >>>>>> >>>>>> - Cheers, >>>>>> Ken >>>>>> >>>>>> On January 13, 2015 at 11:12:47 AM, ch...@sinjakli.co.uk ( >>>>>> ch...@sinjakli.co.uk) wrote: >>>>>> >>>>>> I'm really glad to see foreign key support has made its way into >>>>>> Rails migrations, but very quickly ran into an issue with it when working >>>>>> in teams using structure.sql. >>>>>> >>>>>> Previously, we'd been using the foreigner gem, which generated FK >>>>>> names based on the table and column. This meant the name was consistent >>>>>> every time you ran the migration. With the version that made it into >>>>>> 4.2, part >>>>>> of the FK name is random >>>>>> <https://github.com/senny/rails/commit/8768305f20d12c40241396092a63e0d56269fefe#diff-a0775e1ec64264dc76a8ee71a5211ae3R697>. >>>>>> This means every dev who runs the migration locally generates a different >>>>>> version of structure.sql, leading to conflicts in version control. >>>>>> >>>>>> Right now we're working round this by explicitly setting the FK name >>>>>> in our migrations. I'd like to find a way of fixing this in Rails itself >>>>>> though. >>>>>> >>>>>> My understanding was the reason for switching away from the old >>>>>> format was that the name is arbitrary, and can fall out of date on column >>>>>> renames. By itself I'm not totally convinced that's enough reason to get >>>>>> rid of the more descriptive names (though I agree it's debatable). The >>>>>> restriction on the length of the name (63 characters in Postgres, for >>>>>> example) is a more compelling reason for me. >>>>>> >>>>>> Either way, I'd like to contribute a patch to make migrations >>>>>> generate consistent FK names. I'm happy to implement either of the >>>>>> following (or something else if anyone's got a better ideas): >>>>>> >>>>>> * Switch back to generating human-readable names, accept that people >>>>>> with long table/column names will occasionally have to set them manually >>>>>> * Generate that human-readable name, hash it, and append a substring >>>>>> of the hash to "fk_rails_". This would give you a name which is both >>>>>> short >>>>>> and consistent. >>>>>> >>>>>> Would really appreciate feedback on those approaches. >>>>>> >>>>>> Cheers, >>>>>> Chris >>>>>> -- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "Ruby on Rails: Core" group. >>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>> send an email to rubyonrails-co...@googlegroups.com. >>>>>> To post to this group, send email to rubyonra...@googlegroups.com. >>>>>> Visit this group at http://groups.google.com/group/rubyonrails-core. >>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>> >>>>>> >>>>>> -- >>>>>> You received this message because you are subscribed to the Google >>>>>> Groups "Ruby on Rails: Core" group. >>>>>> To unsubscribe from this group and stop receiving emails from it, >>>>>> send an email to rubyonrails-co...@googlegroups.com. >>>>>> To post to this group, send email to rubyonra...@googlegroups.com. >>>>>> Visit this group at http://groups.google.com/group/rubyonrails-core. >>>>>> For more options, visit https://groups.google.com/d/optout. >>>>>> >>>>> >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "Ruby on Rails: Core" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to rubyonrails-co...@googlegroups.com. >>>>> To post to this group, send email to rubyonra...@googlegroups.com. >>>>> Visit this group at http://groups.google.com/group/rubyonrails-core. >>>>> For more options, visit https://groups.google.com/d/optout. >>>>> >>>> >>>> >> -- >> You received this message because you are subscribed to the Google Groups >> "Ruby on Rails: Core" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to rubyonrails-co...@googlegroups.com. >> To post to this group, send email to rubyonra...@googlegroups.com. >> Visit this group at http://groups.google.com/group/rubyonrails-core. >> For more options, visit https://groups.google.com/d/optout. >> >> >> > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscr...@googlegroups.com. > To post to this group, send email to rubyonrails-core@googlegroups.com. > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/d/optout. > > > -- > You received this message because you are subscribed to the Google Groups > "Ruby on Rails: Core" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to rubyonrails-core+unsubscr...@googlegroups.com. > To post to this group, send email to rubyonrails-core@googlegroups.com. > Visit this group at http://groups.google.com/group/rubyonrails-core. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. To unsubscribe from this group and stop receiving emails from it, send an email to rubyonrails-core+unsubscr...@googlegroups.com. To post to this group, send email to rubyonrails-core@googlegroups.com. Visit this group at http://groups.google.com/group/rubyonrails-core. For more options, visit https://groups.google.com/d/optout.
