mirror of
https://gh.wpcy.net/https://github.com/discourse/discourse.git
synced 2026-04-30 18:08:34 +08:00
## Problem `Migration::ColumnDropper.mark_readonly` could incorrectly detect a default value from a non-public schema table. The query checking for column defaults did not filter by `table_schema`: ```sql SELECT column_default IS NOT NULL FROM information_schema.columns WHERE table_name = :table_name AND column_name = :column_name ``` When multiple schemas contain tables with the same name (e.g., from backup/restore operations), this query returns multiple rows. The code uses `.first`, making behavior dependent on PostgreSQL's row ordering. ## Root Cause Don's diagnostic queries on an affected instance confirmed the issue: ```plain table_schema | column_name | column_default --------------+---------------------------+---------------- backup | discourse_rewind_disabled | false ← returned first public | discourse_rewind_disabled | false ``` The `backup` schema (from a previous `pg_dump`) contained a copy of `user_options` with the old default. PostgreSQL returned this row first, causing `mark_readonly` to fail with "You must drop a column's default value before marking it as readonly". ## Fix Just a oneline -> adding `table_schema = 'public'` to the `WHERE` clause to ensure only the `public` schema is considered. ## Why it couldn't be reproduced locally? - Fresh dev databases don't have `backup` schemas - CI environments use clean databases - Row ordering depends on database history (OIDs, backup/restore cycles) Ref - https://meta.discourse.org/t/393049
50 lines
1.6 KiB
Ruby
50 lines
1.6 KiB
Ruby
# frozen_string_literal: true
|
|
|
|
require "migration/base_dropper"
|
|
|
|
module Migration
|
|
class ColumnDropper
|
|
def self.mark_readonly(table_name, column_name)
|
|
has_default = DB.query_single(<<~SQL, table_name: table_name, column_name: column_name).first
|
|
SELECT column_default IS NOT NULL
|
|
FROM information_schema.columns
|
|
WHERE table_schema = 'public'
|
|
AND table_name = :table_name
|
|
AND column_name = :column_name
|
|
SQL
|
|
|
|
raise "You must drop a column's default value before marking it as readonly" if has_default
|
|
|
|
BaseDropper.create_readonly_function(table_name, column_name)
|
|
|
|
DB.exec <<~SQL
|
|
CREATE TRIGGER #{BaseDropper.readonly_trigger_name(table_name, column_name)}
|
|
BEFORE INSERT OR UPDATE OF #{column_name}
|
|
ON #{table_name}
|
|
FOR EACH ROW
|
|
WHEN (NEW.#{column_name} IS NOT NULL)
|
|
EXECUTE PROCEDURE #{BaseDropper.readonly_function_name(table_name, column_name)};
|
|
SQL
|
|
end
|
|
|
|
def self.execute_drop(table, columns)
|
|
table = table.to_s
|
|
|
|
columns.each do |column|
|
|
column = column.to_s
|
|
self.drop_readonly(table, column)
|
|
# safe cause it is protected on method entry, can not be passed in params
|
|
DB.exec("ALTER TABLE #{table} DROP COLUMN IF EXISTS #{column}")
|
|
end
|
|
end
|
|
|
|
def self.drop_readonly(table_name, column_name)
|
|
BaseDropper.drop_readonly_function(table_name, column_name)
|
|
|
|
# Backward compatibility for old functions created in the public schema
|
|
DB.exec(
|
|
"DROP FUNCTION IF EXISTS #{BaseDropper.old_readonly_function_name(table_name, column_name)} CASCADE",
|
|
)
|
|
end
|
|
end
|
|
end
|