FIX: Restoring backup from PG12 could fail on PG10

FIX: Restoring backup from PG12 could fail on PG10

The EXECUTE FUNCTION syntax for CREATE TRIGGER statements was introduced in PostgreSQL 11. We need to replace EXECUTE FUNCTION with EXECUTE PROCEDURE in order to be able to restore backups created with PG12 on PG10.

diff --git a/lib/backup_restore.rb b/lib/backup_restore.rb
index 4d75734..5b18ef6 100644
--- a/lib/backup_restore.rb
+++ b/lib/backup_restore.rb
@@ -82,6 +82,10 @@ module BackupRestore
     ActiveRecord::Migrator.current_version
   end
 
+  def self.postgresql_major_version
+    DB.query_single("SHOW server_version").first[/\d+/].to_i
+  end
+
   def self.move_tables_between_schemas(source, destination)
     owner = database_configuration.username
 
diff --git a/lib/backup_restore/database_restorer.rb b/lib/backup_restore/database_restorer.rb
index 929e318..9ccb6a5 100644
--- a/lib/backup_restore/database_restorer.rb
+++ b/lib/backup_restore/database_restorer.rb
@@ -95,7 +95,8 @@ module BackupRestore
       raise DatabaseRestoreError.new("psql failed: #{last_line}") if Process.last_status&.exitstatus != 0
     end
 
-    # Removes unwanted SQL added by certain versions of pg_dump.
+    # Removes unwanted SQL added by certain versions of pg_dump and modifies
+    # the dump so that it works on the current version of PostgreSQL.
     def sed_command
       unwanted_sql = [
         "DROP SCHEMA", # Discourse <= v1.5
@@ -104,11 +105,15 @@ module BackupRestore
         "SET default_table_access_method" # PostgreSQL 12
       ].join("|")
 
-      "sed -E '/^(#{unwanted_sql})/d'"
+      command = "sed -E '/^(#{unwanted_sql})/d' #{@db_dump_path}"
+      if BackupRestore.postgresql_major_version < 11
+        command = "#{command} | sed -E 's/^(CREATE TRIGGER.+EXECUTE) FUNCTION/\\1 PROCEDURE/'"
+      end
+      command
     end
 
     def restore_dump_command
-      "#{sed_command} #{@db_dump_path} | #{self.class.psql_command} 2>&1"
+      "#{sed_command} | #{self.class.psql_command} 2>&1"
     end
 
     def self.psql_command
diff --git a/spec/fixtures/db/restore/trigger.sql b/spec/fixtures/db/restore/trigger.sql
new file mode 100644
index 0000000..78621ec
--- /dev/null
+++ b/spec/fixtures/db/restore/trigger.sql
@@ -0,0 +1,55 @@
+--
+-- PostgreSQL database dump
+--
+
+-- Dumped from database version 12.2 (Debian 12.2-2.pgdg100+1)
+-- Dumped by pg_dump version 12.2 (Debian 12.2-2.pgdg100+1)
+
+-- Started on 2020-06-15 08:06:34 UTC
+
+SET statement_timeout = 0;
+SET lock_timeout = 0;
+SET idle_in_transaction_session_timeout = 0;
+SET client_encoding = 'UTF8';
+SET standard_conforming_strings = on;
+SELECT pg_catalog.set_config('search_path', '', false);
+SET check_function_bodies = false;
+SET xmloption = content;
+SET client_min_messages = warning;
+SET row_security = off;
+
+--
+-- TOC entry 5 (class 2615 OID 2200)
+-- Name: public; Type: SCHEMA; Schema: -; Owner: -
+--
+
+CREATE SCHEMA public;
+
+
+--
+-- TOC entry 7007 (class 0 OID 0)
+-- Dependencies: 5
+-- Name: SCHEMA public; Type: COMMENT; Schema: -; Owner: -
+--
+
+COMMENT ON SCHEMA public IS 'standard public schema';
+
+SET default_tablespace = '';
+
+SET default_table_access_method = heap;
+
+--
+-- TOC entry 198 (class 1259 OID 16585)
+-- Name: foo; Type: TABLE; Schema: public; Owner: -
+--
+
+CREATE TABLE public.foo (
+    id integer NOT NULL,
+    topic_id integer,
+    user_id integer
+);
+
+
+CREATE TRIGGER foo_topic_id_readonly BEFORE INSERT OR UPDATE OF redeemed_at ON public.foo FOR EACH ROW WHEN ((new.topic_id IS NOT NULL)) EXECUTE FUNCTION discourse_functions.raise_foo_topic_id_readonly();
+
+CREATE TRIGGER foo_user_id_readonly BEFORE INSERT OR UPDATE OF user_id ON public.foo FOR EACH ROW WHEN ((new.user_id IS NOT NULL)) EXECUTE FUNCTION discourse_functions.raise_foo_user_id_readonly();
diff --git a/spec/lib/backup_restore/database_restorer_spec.rb b/spec/lib/backup_restore/database_restorer_spec.rb
index 8a7b9ec..09c2038 100644
--- a/spec/lib/backup_restore/database_restorer_spec.rb
+++ b/spec/lib/backup_restore/database_restorer_spec.rb
@@ -127,6 +127,51 @@ describe BackupRestore::DatabaseRestorer do
       end
     end
 
+    context "rewrites database dump" do
+      let(:logger) do
+        Class.new do
+          attr_reader :log_messages
+
+          def initialize
+            @log_messages = []
+          end
+
+          def log(message, ex = nil)
+            @log_messages << message if message
+          end
+        end.new
+      end
+
+      def restore_and_log_output(filename)
+        path = File.join(Rails.root, "spec/fixtures/db/restore", filename)
+        BackupRestore::DatabaseRestorer.stubs(:psql_command).returns("cat")
+        execute_stubbed_restore(stub_psql: false, dump_file_path: path)
+        logger.log_messages.join("\n")
+      end
+
+      it "replaces `EXECUTE FUNCTION` when restoring on PostgreSQL < 11" do
+        BackupRestore.stubs(:postgresql_major_version).returns(10)
+        log = restore_and_log_output("trigger.sql")
+
+        expect(log).not_to be_blank
+        expect(log).not_to match(/CREATE SCHEMA public/)
+        expect(log).not_to match(/EXECUTE FUNCTION/)
+        expect(log).to match(/^CREATE TRIGGER foo_topic_id_readonly .+? EXECUTE PROCEDURE discourse_functions.raise_foo_topic_id_readonly/)
+        expect(log).to match(/^CREATE TRIGGER foo_user_id_readonly .+? EXECUTE PROCEDURE discourse_functions.raise_foo_user_id_readonly/)
+      end
+
+      it "does not replace `EXECUTE FUNCTION` when restoring on PostgreSQL >= 11" do
+        BackupRestore.stubs(:postgresql_major_version).returns(11)
+        log = restore_and_log_output("trigger.sql")
+
+        expect(log).not_to be_blank
+        expect(log).not_to match(/CREATE SCHEMA public/)
+        expect(log).not_to match(/EXECUTE PROCEDURE/)
+        expect(log).to match(/^CREATE TRIGGER foo_topic_id_readonly .+? EXECUTE FUNCTION discourse_functions.raise_foo_topic_id_readonly/)
+        expect(log).to match(/^CREATE TRIGGER foo_user_id_readonly .+? EXECUTE FUNCTION discourse_functions.raise_foo_user_id_readonly/)
+      end
+    end
+
     context "database connection" do
       it 'reconnects to the correct database', type: :multisite do
         RailsMultisite::ConnectionManagement.establish_connection(db: 'second')

GitHub sha: 859d9b75

1 Like