Add primary key to preview_cards_statuses join table (includes deduplication migration) (#25243)

This commit is contained in:
Claire 2023-08-03 11:12:52 +02:00 committed by GitHub
parent a0fad5c8bb
commit 6b896b20cc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 93 additions and 5 deletions

View file

@ -61,10 +61,14 @@ class FetchLinkCardService < BaseService
end end
def attach_card def attach_card
with_redis_lock("attach_card:#{@status.id}") do
return if @status.preview_cards.any?
@status.preview_cards << @card @status.preview_cards << @card
Rails.cache.delete(@status) Rails.cache.delete(@status)
Trends.links.register(@status) Trends.links.register(@status)
end end
end
def parse_urls def parse_urls
urls = if @status.local? urls = if @status.local?

View file

@ -0,0 +1,39 @@
# frozen_string_literal: true
class AddUniqueIndexOnPreviewCardsStatuses < ActiveRecord::Migration[6.1]
disable_ddl_transaction!
def up
add_index :preview_cards_statuses, [:status_id, :preview_card_id], name: :preview_cards_statuses_pkey, algorithm: :concurrently, unique: true
rescue ActiveRecord::RecordNotUnique
deduplicate_and_reindex!
end
def down
remove_index :preview_cards_statuses, name: :preview_cards_statuses_pkey
end
private
def deduplicate_and_reindex!
deduplicate_preview_cards!
safety_assured { execute 'REINDEX INDEX preview_cards_statuses_pkey' }
rescue ActiveRecord::RecordNotUnique
retry
end
def deduplicate_preview_cards!
# Statuses should have only one preview card at most, even if that's not the database
# constraint we will end up with
duplicate_ids = select_all('SELECT status_id FROM preview_cards_statuses GROUP BY status_id HAVING count(*) > 1;').rows
duplicate_ids.each_slice(1000) do |ids|
# This one is tricky: since we don't have primary keys to keep only one record,
# use the physical `ctid`
safety_assured do
execute "DELETE FROM preview_cards_statuses p WHERE p.status_id IN (#{ids.join(', ')}) AND p.ctid NOT IN (SELECT q.ctid FROM preview_cards_statuses q WHERE q.status_id = p.status_id LIMIT 1)"
end
end
end
end

View file

@ -0,0 +1,20 @@
# frozen_string_literal: true
class AddPrimaryKeyToPreviewCardsStatusesJoinTable < ActiveRecord::Migration[6.1]
disable_ddl_transaction!
def up
safety_assured do
execute 'ALTER TABLE preview_cards_statuses ADD PRIMARY KEY USING INDEX preview_cards_statuses_pkey'
end
end
def down
safety_assured do
# I have found no way to demote the primary key to an index, instead, re-create the index
execute 'CREATE UNIQUE INDEX CONCURRENTLY preview_cards_statuses_pkey_tmp ON preview_cards_statuses (status_id, preview_card_id)'
execute 'ALTER TABLE preview_cards_statuses DROP CONSTRAINT preview_cards_statuses_pkey'
execute 'ALTER INDEX preview_cards_statuses_pkey_tmp RENAME TO preview_cards_statuses_pkey'
end
end
end

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.0].define(version: 2023_07_24_160715) do ActiveRecord::Schema[7.0].define(version: 2023_08_03_112520) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -805,7 +805,7 @@ ActiveRecord::Schema[7.0].define(version: 2023_07_24_160715) do
t.index ["url"], name: "index_preview_cards_on_url", unique: true t.index ["url"], name: "index_preview_cards_on_url", unique: true
end end
create_table "preview_cards_statuses", id: false, force: :cascade do |t| create_table "preview_cards_statuses", primary_key: ["status_id", "preview_card_id"], force: :cascade do |t|
t.bigint "preview_card_id", null: false t.bigint "preview_card_id", null: false
t.bigint "status_id", null: false t.bigint "status_id", null: false
t.index ["status_id", "preview_card_id"], name: "index_preview_cards_statuses_on_status_id_and_preview_card_id" t.index ["status_id", "preview_card_id"], name: "index_preview_cards_statuses_on_status_id_and_preview_card_id"

View file

@ -63,6 +63,11 @@ namespace :tests do
puts 'Account domains not properly normalized' puts 'Account domains not properly normalized'
exit(1) exit(1)
end end
unless Status.find(12).preview_cards.pluck(:url) == ['https://joinmastodon.org/']
puts 'Preview cards not deduplicated as expected'
exit(1)
end
end end
desc 'Populate the database with test data for 2.4.3' desc 'Populate the database with test data for 2.4.3'
@ -238,6 +243,11 @@ namespace :tests do
(10, 2, '@admin hey!', NULL, 1, 3, now(), now()), (10, 2, '@admin hey!', NULL, 1, 3, now(), now()),
(11, 1, '@user hey!', 10, 1, 3, now(), now()); (11, 1, '@user hey!', 10, 1, 3, now(), now());
INSERT INTO "statuses"
(id, account_id, text, created_at, updated_at)
VALUES
(12, 1, 'check out https://joinmastodon.org/', now(), now());
-- mentions (from previous statuses) -- mentions (from previous statuses)
INSERT INTO "mentions" INSERT INTO "mentions"
@ -326,6 +336,21 @@ namespace :tests do
(1, 6, 2, 'Follow', 2, now(), now()), (1, 6, 2, 'Follow', 2, now(), now()),
(2, 2, 1, 'Mention', 4, now(), now()), (2, 2, 1, 'Mention', 4, now(), now()),
(3, 1, 2, 'Mention', 5, now(), now()); (3, 1, 2, 'Mention', 5, now(), now());
-- preview cards
INSERT INTO "preview_cards"
(id, url, title, created_at, updated_at)
VALUES
(1, 'https://joinmastodon.org/', 'Mastodon - Decentralized social media', now(), now());
-- many-to-many association between preview cards and statuses
INSERT INTO "preview_cards_statuses"
(status_id, preview_card_id)
VALUES
(12, 1),
(12, 1);
SQL SQL
end end
end end