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.

lib/backup_restore.rb

@@ -82,6 +82,10 @@ def self.current_version
     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
 

lib/backup_restore/database_restorer.rb

@@ -95,7 +95,8 @@ def restore_dump
       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 @@ def sed_command
         "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

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();

spec/lib/backup_restore/database_restorer_spec.rb

@@ -127,6 +127,51 @@ def expect_restore_to_work(filename)
       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')