Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move the warning about composite primary key to AttributeMethods::PrimaryKey #25578

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,10 @@ def self.retrieve_connection
left_model.retrieve_connection
end

def self.primary_key
false
private

def self.suppress_composite_primary_key(pk)
pk unless pk.is_a?(Array)
end
}

Expand Down
15 changes: 14 additions & 1 deletion activerecord/lib/active_record/attribute_methods/primary_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ def get_primary_key(base_name) #:nodoc:
base_name.foreign_key
else
if ActiveRecord::Base != self && table_exists?
connection.schema_cache.primary_keys(table_name)
pk = connection.schema_cache.primary_keys(table_name)
suppress_composite_primary_key(pk)
else
'id'
end
Expand All @@ -122,6 +123,18 @@ def primary_key=(value)
@quoted_primary_key = nil
@attributes_builder = nil
end

private

def suppress_composite_primary_key(pk)
return pk unless pk.is_a?(Array)

warn <<-WARNING.strip_heredoc
WARNING: Active Record does not support composite primary key.

#{table_name} has composite primary key. Composite primary key is ignored.
WARNING
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,9 @@ def column_exists?(table_name, column_name, type = nil, options = {})

# Returns just a table's primary key
def primary_key(table_name)
pks = primary_keys(table_name)
warn <<-WARNING.strip_heredoc if pks.count > 1
WARNING: Rails does not support composite primary key.

#{table_name} has composite primary key. Composite primary key is ignored.
WARNING

pks.first if pks.one?
pk = primary_keys(table_name)
pk = pk.first unless pk.size > 1
pk
end

# Creates a new table with the name +table_name+. +table_name+ may either
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ def sql_for_insert(sql, pk, id_value, sequence_name, binds) # :nodoc:
pk = primary_key(table_ref) if table_ref
end

pk = suppress_composite_primary_key(pk)

if pk && use_insert_returning?
sql = "#{sql} RETURNING #{quote_column_name(pk)}"
end
Expand Down Expand Up @@ -164,6 +166,12 @@ def commit_db_transaction
def exec_rollback_db_transaction
execute "ROLLBACK"
end

private

def suppress_composite_primary_key(pk)
pk unless pk.is_a?(Array)
end
end
end
end
Expand Down
7 changes: 1 addition & 6 deletions activerecord/lib/active_record/schema_dumper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,7 @@ def table(table, stream)
tbl = StringIO.new

# first dump primary key column
if @connection.respond_to?(:primary_keys)
pk = @connection.primary_keys(table)
pk = pk.first unless pk.size > 1
else
pk = @connection.primary_key(table)
end
pk = @connection.primary_key(table)

tbl.print " create_table #{remove_prefix_and_suffix(table).inspect}"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def test_join_table_composite_primary_key_should_not_warn
warning = capture(:stderr) do
country.treaties << treaty
end
assert_no_match(/WARNING: Rails does not support composite primary key\./, warning)
assert_no_match(/WARNING: Active Record does not support composite primary key\./, warning)
end

def test_has_and_belongs_to_many
Expand Down
10 changes: 8 additions & 2 deletions activerecord/test/cases/primary_keys_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ class CompositePrimaryKeyTest < ActiveRecord::TestCase

def setup
@connection = ActiveRecord::Base.connection
@connection.schema_cache.clear!
@connection.create_table(:barcodes, primary_key: ["region", "code"], force: true) do |t|
t.string :region
t.integer :code
Expand All @@ -270,10 +271,15 @@ def test_composite_primary_key
end

def test_primary_key_issues_warning
model = Class.new(ActiveRecord::Base) do
def self.table_name
"barcodes"
end
end
warning = capture(:stderr) do
assert_nil @connection.primary_key("barcodes")
assert_nil model.primary_key
end
assert_match(/WARNING: Rails does not support composite primary key\./, warning)
assert_match(/WARNING: Active Record does not support composite primary key\./, warning)
end

def test_collectly_dump_composite_primary_key
Expand Down