Permalink
Browse files

Add a `foreign_key` option to `references` while creating the table

Rather than having to do:

    create_table :posts do |t|
      t.references :user
    end

    add_foreign_key :posts, :users

You can instead do:

    create_table :posts do |t|
      t.references :user, foreign_key: true
    end

Similar to the `index` option, you can also pass a hash. This will be
passed as the options to `add_foreign_key`. e.g.:

    create_table :posts do |t|
      t.references :user, foreign_key: { primary_key: :other_id }
    end

is equivalent to

    create_table :posts do |t|
      t.references :user
    end

    add_foreign_key :posts, :users, primary_key: :other_id
  • Loading branch information...
sgrif committed Dec 22, 2014
1 parent a9c0c46 commit 99a6f9e60ea55924b44f894a16f8de0162cf2702
@@ -94,11 +94,12 @@ class TableDefinition
# An array of ColumnDefinition objects, representing the column changes
# that have been defined.
attr_accessor :indexes
attr_reader :name, :temporary, :options, :as
attr_reader :name, :temporary, :options, :as, :foreign_keys
def initialize(types, name, temporary, options, as = nil)
@columns_hash = {}
@indexes = {}
@foreign_keys = {}
@native = types
@temporary = temporary
@options = options
@@ -286,6 +287,10 @@ def index(column_name, options = {})
indexes[column_name] = options
end
def foreign_key(table_name, options = {}) # :nodoc:
foreign_keys[table_name] = options
end
# Appends <tt>:datetime</tt> columns <tt>:created_at</tt> and
# <tt>:updated_at</tt> to the table. See SchemaStatements#add_timestamps
#
@@ -297,9 +302,12 @@ def timestamps(*args)
column(:updated_at, :datetime, options)
end
# Adds a reference. Optionally adds a +type+ column, if <tt>:polymorphic</tt> option is provided.
# <tt>references</tt> and <tt>belongs_to</tt> are acceptable. The reference column will be an +integer+
# by default, the <tt>:type</tt> option can be used to specify a different type.
# Adds a reference. Optionally adds a +type+ column, if
# <tt>:polymorphic</tt> option is provided. <tt>references</tt> and
# <tt>belongs_to</tt> are acceptable. The reference column will be an
# +integer+ by default, the <tt>:type</tt> option can be used to specify
# a different type. A foreign key will be created if a +foreign_key+
# option is passed.
#
# t.references(:user)
# t.references(:user, type: "string")
@@ -310,11 +318,18 @@ def references(
*args,
polymorphic: false,
index: false,
foreign_key: false,
type: :integer,
**options
)
polymorphic_options = polymorphic.is_a?(Hash) ? polymorphic : options
index_options = index.is_a?(Hash) ? index : {}
foreign_key_options = foreign_key.is_a?(Hash) ? foreign_key : {}
if polymorphic && foreign_key
raise ArgumentError, "Cannot add a foreign key on a polymorphic relation"
end
args.each do |col|
column("#{col}_id", type, options)
@@ -325,6 +340,10 @@ def references(
if index
self.index(polymorphic ? %w(type id).map { |t| "#{col}_#{t}" } : "#{col}_id", index_options)
end
if foreign_key
self.foreign_key(col.to_s.pluralize, foreign_key_options)
end
end
end
alias :belongs_to :references
@@ -204,7 +204,17 @@ def create_table(table_name, options = {})
end
result = execute schema_creation.accept td
td.indexes.each_pair { |c, o| add_index(table_name, c, o) } unless supports_indexes_in_create?
unless supports_indexes_in_create?
td.indexes.each_pair do |column_name, index_options|
add_index(table_name, column_name, index_options)
end
end
td.foreign_keys.each_pair do |other_table_name, foreign_key_options|

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Apr 22, 2015

Member

@sgrif we need to check supports_foreign_keys? here (#19794)

@pixeltrix

pixeltrix Apr 22, 2015

Member

@sgrif we need to check supports_foreign_keys? here (#19794)

This comment has been minimized.

Show comment
Hide comment
@derekprior

derekprior Apr 24, 2015

Contributor

Hmmm. Should we? As far as I know, this behavior -- adding foreign keys on references -- is opt-in. You have to pass foreign_key: true. If that's not the case, then I think that's a bug.

If you pass foreign_key: true and your adapter does not support foreign keys, should it be a no-op or an error?

@derekprior

derekprior Apr 24, 2015

Contributor

Hmmm. Should we? As far as I know, this behavior -- adding foreign keys on references -- is opt-in. You have to pass foreign_key: true. If that's not the case, then I think that's a bug.

If you pass foreign_key: true and your adapter does not support foreign keys, should it be a no-op or an error?

This comment has been minimized.

Show comment
Hide comment
@pixeltrix

pixeltrix Apr 24, 2015

Member

@derekprior we do the check for supports_indexes_in_create? and you can make the same argument for that - if you're adding an index in the create_table block and your adapter doesn't support it then should it blow up? Anyway the problem is that td.foreign_keys is nil and not an empty array so it raises even if you've not passed foreign_key: true.

@pixeltrix

pixeltrix Apr 24, 2015

Member

@derekprior we do the check for supports_indexes_in_create? and you can make the same argument for that - if you're adding an index in the create_table block and your adapter doesn't support it then should it blow up? Anyway the problem is that td.foreign_keys is nil and not an empty array so it raises even if you've not passed foreign_key: true.

This comment has been minimized.

Show comment
Hide comment
@senny

senny Apr 27, 2015

Member

@pixeltrix I'll take a look at the nil array. However, I don't think we should check for supports_foreign_keys?. add_foreign_key is a no-op for adapters that don't support it. This is to address users testing for example with SQLite and deploying with Oracle or PostgreSQL.

There is also the use-case of applications supporting multiple database backends.

@senny

senny Apr 27, 2015

Member

@pixeltrix I'll take a look at the nil array. However, I don't think we should check for supports_foreign_keys?. add_foreign_key is a no-op for adapters that don't support it. This is to address users testing for example with SQLite and deploying with Oracle or PostgreSQL.

There is also the use-case of applications supporting multiple database backends.

This comment has been minimized.

Show comment
Hide comment
@senny

senny Apr 27, 2015

Member

@derekprior @pixeltrix to me this is a compatibility issue with the DB2 adapter gem and Rails 4.2. See this comment for more details.

@senny

senny Apr 27, 2015

Member

@derekprior @pixeltrix to me this is a compatibility issue with the DB2 adapter gem and Rails 4.2. See this comment for more details.

add_foreign_key(table_name, other_table_name, foreign_key_options)
end
result
end
@@ -0,0 +1,60 @@
require 'cases/helper'
if ActiveRecord::Base.connection.supports_foreign_keys?
module ActiveRecord
class Migration
class ReferencesForeignKeyTest < ActiveRecord::TestCase
setup do
@connection = ActiveRecord::Base.connection
@connection.transaction do
@connection.create_table(:testing_parents, force: true)
end
end
teardown do
@connection.execute("drop table if exists testings")
@connection.execute("drop table if exists testing_parents")
end
test "foreign keys can be created with the table" do
@connection.create_table :testings do |t|
t.references :testing_parent, foreign_key: true
end
fk = @connection.foreign_keys("testings").first
assert_equal "testings", fk.from_table
assert_equal "testing_parents", fk.to_table
end
test "no foreign key is created by default" do
@connection.create_table :testings do |t|
t.references :testing_parent
end
assert_equal [], @connection.foreign_keys("testings")
end
test "options hash can be passed" do
@connection.change_table :testing_parents do |t|
t.integer :other_id
t.index :other_id, unique: true
end
@connection.create_table :testings do |t|
t.references :testing_parent, foreign_key: { primary_key: :other_id }
end
fk = @connection.foreign_keys("testings").find { |k| k.to_table == "testing_parents" }
assert_equal "other_id", fk.primary_key
end
test "foreign keys cannot be added to polymorphic relations when creating the table" do
@connection.create_table :testings do |t|
assert_raises(ArgumentError) do
t.references :testing_parent, polymorphic: true, foreign_key: true
end
end
end
end
end
end
end

2 comments on commit 99a6f9e

@senny

This comment has been minimized.

Show comment
Hide comment
@senny

senny Dec 23, 2014

Member

@sgrif can you add an entry to the CHANGELOG regarding the new option and the generator behavior?

Member

senny replied Dec 23, 2014

@sgrif can you add an entry to the CHANGELOG regarding the new option and the generator behavior?

@zzak

This comment has been minimized.

Show comment
Hide comment
@zzak

zzak Jun 18, 2015

Member

Ping #20612

Member

zzak replied Jun 18, 2015

Ping #20612

Please sign in to comment.