Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add foreign key support to migrations and schema.rb dump #668

Closed
lighthouse-import opened this Issue · 9 comments

1 participant

@lighthouse-import

Imported from Lighthouse. Original ticket at: http://rails.lighthouseapp.com/projects/8994/tickets/4347
Created by matthuhiggins - 2011-02-17 08:07:22 UTC

This patch adds the following methods to migrations:

add_foreign_key(referencing_table, referenced_table, options = {})
remove_foreign_key(referencing_table, options)

change_table ... do |t|
  t.foreign_key(referenced_table, options)
  t.remove_foreign_key(options)
end

Options are :name, :column, :primary_key, :dependent.

In addition, ActiveRecord::SchemaDumper reads foreign keys from the SQL structure and puts them into schema.rb.


Some thoughts and concerns I came across while implementing this:

  • Extending t.references to automatically add a foreign key seems like a natural progression, but I didn't want to jam too much in at once.

  • The :primary_key value defaults to 'id'. This is a sensible default, but there is other code in the adapter that attempts to extract the primary key from the table. I do not know the history behind this, and was trying to clone the :primary_key API for associations.

  • Many active_record tests run with "use_transactional_fixtures = false", which results in leftover data during the foreign key tests. This makes adding foreign keys difficult, when the existing data has invalid references.

  • drop_table blows up if the table being dropped has foreign keys pointing to it. One option is to ignore foreign key checks during this (e.g. in MySql use "SET FOREIGN_KEY_CHECKS = 0; super; SET FOREIGN_KEY_CHECKS = 1;"

@lighthouse-import

Imported from Lighthouse.
Comment by matthuhiggins - 2010-04-08 18:27:37 UTC

Oops, this patch left out tests for the schema dumper.

@lighthouse-import

Imported from Lighthouse.
Comment by matthuhiggins - 2010-04-08 20:52:03 UTC

foreign_keys-2.diff tests that foreign keys are dumped to schema.rb after the tables

@lighthouse-import

Imported from Lighthouse.
Comment by 2kan - 2010-11-23 11:48:46 UTC

+1. I haven't read the patch, but it is very good idea.

@lighthouse-import

Imported from Lighthouse.
Comment by wtn - 2010-11-23 11:58:46 UTC

-1

I'm fine with using existing gems and leaving this out of the official AR.

@lighthouse-import

Imported from Lighthouse.
Comment by Daniel - 2010-11-23 20:16:14 UTC

+1 Migrations have always felt incomplete to me because they lack support for adding foreign key constraints. I greatly appreciate the Foreigner gem, but I would really like to see foreign keys supported in the framework.

@lighthouse-import

Imported from Lighthouse.
Comment by Michał Łomnicki - 2011-01-12 11:39:07 UTC

+1 for foreign keys

It is absolute must-have for ActiveRecord. Foreign keys are the only way to keep relations consistent as validations are prone to race conditions and one can't really rely on them.

@lighthouse-import

Imported from Lighthouse.
Comment by Ryan - 2011-01-14 02:24:19 UTC

+1 This is absolutely necessary. Foreign keys should definitely be supported out of the box for migrations.

@lighthouse-import

Imported from Lighthouse.
Comment by Franco Catena - 2011-02-15 22:20:28 UTC

+1 for me too. I agree, is the one thing that was always missed in migrations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.