From: Andy Allan <git@gravitystorm.co.uk>
Date: Thu, 26 Oct 2023 18:40:57 +0000 (+0100)
Subject: Merge pull request #4300 from tomhughes/relation-members-primary-key
X-Git-Tag: live~1482
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/6e28c5fe1943d4bfcc72f282be736828c30519c5?hp=af8e08960e91596c19a6c9a731896737e8e084eb

Merge pull request #4300 from tomhughes/relation-members-primary-key

Simplify primary key for relation members tables
---

diff --git a/app/models/old_relation_member.rb b/app/models/old_relation_member.rb
index 4264e0221..5a1156605 100644
--- a/app/models/old_relation_member.rb
+++ b/app/models/old_relation_member.rb
@@ -20,7 +20,6 @@
 
 class OldRelationMember < ApplicationRecord
   self.table_name = "relation_members"
-  self.primary_key = %w[relation_id version sequence_id]
 
   belongs_to :old_relation, :query_constraints => [:relation_id, :version], :inverse_of => :old_members
   # A bit messy, referring to the current tables, should do for the data browser for now
diff --git a/app/models/relation_member.rb b/app/models/relation_member.rb
index 8ff825cee..2b72cd2b1 100644
--- a/app/models/relation_member.rb
+++ b/app/models/relation_member.rb
@@ -19,7 +19,6 @@
 
 class RelationMember < ApplicationRecord
   self.table_name = "current_relation_members"
-  self.primary_key = %w[relation_id sequence_id]
 
   belongs_to :relation
   belongs_to :member, :polymorphic => true
diff --git a/db/migrate/20231010194809_correct_relation_members_primary_key.rb b/db/migrate/20231010194809_correct_relation_members_primary_key.rb
new file mode 100644
index 000000000..09870836f
--- /dev/null
+++ b/db/migrate/20231010194809_correct_relation_members_primary_key.rb
@@ -0,0 +1,11 @@
+class CorrectRelationMembersPrimaryKey < ActiveRecord::Migration[7.0]
+  def up
+    alter_primary_key :current_relation_members, [:relation_id, :sequence_id]
+    alter_primary_key :relation_members, [:relation_id, :version, :sequence_id]
+  end
+
+  def down
+    alter_primary_key :relation_members, [:relation_id, :version, :member_type, :member_id, :member_role, :sequence_id]
+    alter_primary_key :current_relation_members, [:relation_id, :member_type, :member_id, :member_role, :sequence_id]
+  end
+end
diff --git a/db/structure.sql b/db/structure.sql
index 467d5143c..17f269666 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -1883,7 +1883,7 @@ ALTER TABLE ONLY public.current_nodes
 --
 
 ALTER TABLE ONLY public.current_relation_members
-    ADD CONSTRAINT current_relation_members_pkey PRIMARY KEY (relation_id, member_type, member_id, member_role, sequence_id);
+    ADD CONSTRAINT current_relation_members_pkey PRIMARY KEY (relation_id, sequence_id);
 
 
 --
@@ -2107,7 +2107,7 @@ ALTER TABLE ONLY public.redactions
 --
 
 ALTER TABLE ONLY public.relation_members
-    ADD CONSTRAINT relation_members_pkey PRIMARY KEY (relation_id, version, member_type, member_id, member_role, sequence_id);
+    ADD CONSTRAINT relation_members_pkey PRIMARY KEY (relation_id, version, sequence_id);
 
 
 --
@@ -3437,6 +3437,7 @@ INSERT INTO "schema_migrations" (version) VALUES
 ('23'),
 ('22'),
 ('21'),
+('20231010194809'),
 ('20231007141103'),
 ('20230830115220'),
 ('20230830115219'),
diff --git a/test/controllers/api/relations_controller_test.rb b/test/controllers/api/relations_controller_test.rb
index 7d011c17f..cdef1f5ba 100644
--- a/test/controllers/api/relations_controller_test.rb
+++ b/test/controllers/api/relations_controller_test.rb
@@ -102,7 +102,7 @@ module Api
       second_relation = create(:relation_member, :member => node).relation
       _super_relation = create(:relation_member, :member => second_relation).relation
       # should combine multiple relation_member references into just one relation entry
-      create(:relation_member, :member => node, :relation => relation_with_node, :sequence_id => 2)
+      create(:relation_member, :member => node, :relation => relation_with_node)
       # should not include deleted relations
       deleted_relation = create(:relation, :deleted)
       create(:relation_member, :member => node, :relation => deleted_relation)
@@ -122,7 +122,7 @@ module Api
       second_relation = create(:relation_member, :member => way).relation
       _super_relation = create(:relation_member, :member => second_relation).relation
       # should combine multiple relation_member references into just one relation entry
-      create(:relation_member, :member => way, :relation => relation_with_way, :sequence_id => 2)
+      create(:relation_member, :member => way, :relation => relation_with_way)
       # should not include deleted relations
       deleted_relation = create(:relation, :deleted)
       create(:relation_member, :member => way, :relation => deleted_relation)
@@ -142,7 +142,7 @@ module Api
       second_relation = create(:relation_member, :member => relation).relation
       _super_relation = create(:relation_member, :member => second_relation).relation
       # should combine multiple relation_member references into just one relation entry
-      create(:relation_member, :member => relation, :relation => relation_with_relation, :sequence_id => 2)
+      create(:relation_member, :member => relation, :relation => relation_with_relation)
       # should not include deleted relations
       deleted_relation = create(:relation, :deleted)
       create(:relation_member, :member => relation, :relation => deleted_relation)
diff --git a/test/factories/old_relation_member.rb b/test/factories/old_relation_member.rb
index 99305cd98..34e0bd310 100644
--- a/test/factories/old_relation_member.rb
+++ b/test/factories/old_relation_member.rb
@@ -1,5 +1,6 @@
 FactoryBot.define do
   factory :old_relation_member do
+    sequence(:sequence_id)
     member_role { "" }
 
     old_relation
diff --git a/test/factories/relation_member.rb b/test/factories/relation_member.rb
index bba2e7633..f8e14905e 100644
--- a/test/factories/relation_member.rb
+++ b/test/factories/relation_member.rb
@@ -1,5 +1,6 @@
 FactoryBot.define do
   factory :relation_member do
+    sequence(:sequence_id)
     member_role { "" }
 
     relation