Skip to content

Commit

Permalink
Merge pull request #47633 from Shopify/allow-deriving-composite-prima…
Browse files Browse the repository at this point in the history
…ry-key-from-schema

Allow composite primary key to be derived from schema
  • Loading branch information
eileencodes committed Jun 6, 2023
2 parents e7be1e7 + 257f012 commit 791f109
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 34 deletions.
18 changes: 18 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
* Allow composite primary key to be derived from schema

Booting an application with a schema that contains composite primary keys
will not issue warning and won't `nil`ify the `ActiveRecord::Base#primary_key` value anymore.

Given a `travel_routes` table definition and a `TravelRoute` model like:
```ruby
create_table :travel_routes, primary_key: [:origin, :destination], force: true do |t|
t.string :origin
t.string :destination
end

class TravelRoute < ActiveRecord::Base; end
```
The `TravelRoute.primary_key` value will be automatically derived to `["origin", "destination"]`

*Nikita Vasilevsky*

* Include the `connection_pool` with exceptions raised from an adapter.

The `connection_pool` provides added context such as the connection used
Expand Down
13 changes: 1 addition & 12 deletions activerecord/lib/active_record/attribute_methods/primary_key.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ def get_primary_key(base_name) # :nodoc:
base_name.foreign_key
else
if ActiveRecord::Base != self && table_exists?
pk = connection.schema_cache.primary_keys(table_name)
suppress_composite_primary_key(pk)
connection.schema_cache.primary_keys(table_name)
else
"id"
end
Expand Down Expand Up @@ -178,16 +177,6 @@ def inherited(base)
@quoted_primary_key = nil
end
end

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

warn <<~WARNING
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 @@ -166,19 +166,6 @@ def test_proper_usage_of_primary_keys_and_join_table
assert_equal 1, country.treaties.count
end

def test_join_table_composite_primary_key_should_not_warn
country = Country.new(name: "India")
country.country_id = "c1"
country.save!

treaty = Treaty.new(name: "peace")
treaty.treaty_id = "t1"
warning = capture(:stderr) do
country.treaties << treaty
end
assert_no_match(/WARNING: Active Record does not support composite primary key\./, warning)
end

def test_has_and_belongs_to_many
david = Developer.find(1)

Expand Down
16 changes: 8 additions & 8 deletions activerecord/test/cases/primary_keys_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -463,16 +463,16 @@ def test_id_predicate_composite
end
end

def test_primary_key_issues_warning
model = Class.new(ActiveRecord::Base) do
def self.table_name
"uber_barcodes"
def test_derives_composite_primary_key
def test_primary_key_issues_warning
model = Class.new(ActiveRecord::Base) do
def self.table_name
"uber_barcodes"
end
end

assert_equal ["region", "code"], model.primary_key
end
warning = capture(:stderr) do
assert_nil model.primary_key
end
assert_match(/WARNING: Active Record does not support composite primary key\./, warning)
end

def test_collectly_dump_composite_primary_key
Expand Down
1 change: 0 additions & 1 deletion activerecord/test/models/cpk/book.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
module Cpk
class Book < ActiveRecord::Base
self.table_name = :cpk_books
self.primary_key = [:author_id, :number]

belongs_to :order, autosave: true, query_constraints: [:shop_id, :order_id]
belongs_to :author, class_name: "Cpk::Author"
Expand Down
2 changes: 2 additions & 0 deletions activerecord/test/models/cpk/order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
module Cpk
class Order < ActiveRecord::Base
self.table_name = :cpk_orders
# explicit definition is to allow schema definition to be simplified
# to be shared between different databases
self.primary_key = [:shop_id, :id]

has_many :order_agreements, primary_key: :id
Expand Down

0 comments on commit 791f109

Please sign in to comment.