Merge branch...

Merge branch '247496-add-rollback-migration-helpers-for-change-column-type-concurrently' into 'master'

Add undo helpers for change_column_type_concurrently

See merge request gitlab-org/gitlab!44155

changelogs/unreleased/247496-add-rollback-migration-helpers-for-change-column-type-concurrently.yml

+---
+title: Add undo helpers for change_column_type_concurrently and cleanup_concurrent_column_type_change
+merge_request: 44155
+author:
+type: other

doc/development/what_requires_downtime.md

@@ -176,7 +176,7 @@ class ChangeUsersUsernameStringToText < ActiveRecord::Migration[4.2]
   end
 
   def down
-    cleanup_concurrent_column_type_change :users, :username
+    undo_change_column_type_concurrently :users, :username
   end
 end
 ```
@@ -197,7 +197,7 @@ class ChangeUsersUsernameStringToTextCleanup < ActiveRecord::Migration[4.2]
   end
 
   def down
-    change_column_type_concurrently :users, :username, :string
+    undo_cleanup_concurrent_column_type_change :users, :username, :string
   end
 end
 ```

lib/gitlab/database/migration_helpers.rb

@@ -544,6 +544,16 @@ module Gitlab
         rename_column_concurrently(table, column, temp_column, type: new_type, type_cast_function: type_cast_function, batch_column_name: batch_column_name)
       end
 
+      # Reverses operations performed by change_column_type_concurrently.
+      #
+      # table - The table containing the column.
+      # column - The name of the column to change.
+      def undo_change_column_type_concurrently(table, column)
+        temp_column = "#{column}_for_type_change"
+
+        undo_rename_column_concurrently(table, column, temp_column)
+      end
+
       # Performs cleanup of a concurrent type change.
       #
       # table - The table containing the column.
@@ -560,6 +570,65 @@ module Gitlab
         end
       end
 
+      # Reverses operations performed by cleanup_concurrent_column_type_change.
+      #
+      # table - The table containing the column.
+      # column - The name of the column to change.
+      # old_type - The type of the original column used with change_column_type_concurrently.
+      # type_cast_function - Required if the conversion back to the original type is not automatic
+      # batch_column_name - option for tables without a primary key, in this case
+      #            another unique integer column can be used. Example: :user_id
+      def undo_cleanup_concurrent_column_type_change(table, column, old_type, type_cast_function: nil, batch_column_name: :id)
+        temp_column = "#{column}_for_type_change"
+
+        # Using a descriptive name that includes orinal column's name risks
+        # taking us above the 63 character limit, so we use a hash
+        identifier = "#{table}_#{column}_for_type_change"
+        hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10)
+        temp_undo_cleanup_column = "tmp_undo_cleanup_column_#{hashed_identifier}"
+
+        unless column_exists?(table, batch_column_name)
+          raise "Column #{batch_column_name} does not exist on #{table}"
+        end
+
+        if transaction_open?
+          raise 'undo_cleanup_concurrent_column_type_change can not be run inside a transaction'
+        end
+
+        check_trigger_permissions!(table)
+
+        begin
+          create_column_from(
+            table,
+            column,
+            temp_undo_cleanup_column,
+            type: old_type,
+            batch_column_name: batch_column_name,
+            type_cast_function: type_cast_function
+          )
+
+          transaction do
+            # This has to be performed in a transaction as otherwise we might
+            # have inconsistent data.
+            rename_column(table, column, temp_column)
+            rename_column(table, temp_undo_cleanup_column, column)
+
+            install_rename_triggers(table, column, temp_column)
+          end
+        rescue
+          # create_column_from can not run inside a transaction, which means
+          #  that there is a risk that if any of the operations that follow it
+          #  fail, we'll be left with an inconsistent schema
+          # For those reasons, we make sure that we drop temp_undo_cleanup_column
+          #  if an error is caught
+          if column_exists?(table, temp_undo_cleanup_column)
+            remove_column(table, temp_undo_cleanup_column)
+          end
+
+          raise
+        end
+      end
+
       # Cleans up a concurrent column name.
       #
       # This method takes care of removing previously installed triggers as well

spec/lib/gitlab/database/migration_helpers_spec.rb

@@ -933,6 +933,19 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
     end
   end
 
+  describe '#undo_change_column_type_concurrently' do
+    it 'reverses the operations of change_column_type_concurrently' do
+      expect(model).to receive(:check_trigger_permissions!).with(:users)
+
+      expect(model).to receive(:remove_rename_triggers_for_postgresql)
+        .with(:users, /trigger_.{12}/)
+
+      expect(model).to receive(:remove_column).with(:users, "old_for_type_change")
+
+      model.undo_change_column_type_concurrently(:users, :old)
+    end
+  end
+
   describe '#cleanup_concurrent_column_type_change' do
     it 'cleans up the type changing procedure' do
       expect(model).to receive(:cleanup_concurrent_column_rename)
@@ -945,6 +958,94 @@ RSpec.describe Gitlab::Database::MigrationHelpers do
     end
   end
 
+  describe '#undo_cleanup_concurrent_column_type_change' do
+    context 'in a transaction' do
+      it 'raises RuntimeError' do
+        allow(model).to receive(:transaction_open?).and_return(true)
+
+        expect { model.undo_cleanup_concurrent_column_type_change(:users, :old, :new) }
+          .to raise_error(RuntimeError)
+      end
+    end
+
+    context 'outside a transaction' do
+      let(:temp_column) { "old_for_type_change" }
+
+      let(:temp_undo_cleanup_column) do
+        identifier = "users_old_for_type_change"
+        hashed_identifier = Digest::SHA256.hexdigest(identifier).first(10)
+        "tmp_undo_cleanup_column_#{hashed_identifier}"
+      end
+
+      let(:trigger_name) { model.rename_trigger_name(:users, :old, :old_for_type_change) }
+
+      before do
+        allow(model).to receive(:transaction_open?).and_return(false)
+      end
+
+      it 'reverses the operations of cleanup_concurrent_column_type_change' do
+        expect(model).to receive(:check_trigger_permissions!).with(:users)
+
+        expect(model).to receive(:create_column_from).with(
+          :users,
+          :old,
+          temp_undo_cleanup_column,
+          type: :string,
+          batch_column_name: :id,
+          type_cast_function: nil
+        ).and_return(true)
+
+        expect(model).to receive(:rename_column)
+          .with(:users, :old, temp_column)
+
+        expect(model).to receive(:rename_column)
+          .with(:users, temp_undo_cleanup_column, :old)
+
+        expect(model).to receive(:install_rename_triggers_for_postgresql)
+          .with(trigger_name, '"users"', '"old"', '"old_for_type_change"')
+
+        model.undo_cleanup_concurrent_column_type_change(:users, :old, :string)
+      end
+
+      it 'passes the type_cast_function and batch_column_name' do
+        expect(model).to receive(:column_exists?).with(:users, :other_batch_column).and_return(true)
+        expect(model).to receive(:check_trigger_permissions!).with(:users)
+
+        expect(model).to receive(:create_column_from).with(
+          :users,
+          :old,
+          temp_undo_cleanup_column,
+          type: :string,
+          batch_column_name: :other_batch_column,
+          type_cast_function: :custom_type_cast_function
+        ).and_return(true)
+
+        expect(model).to receive(:rename_column)
+          .with(:users, :old, temp_column)
+
+        expect(model).to receive(:rename_column)
+          .with(:users, temp_undo_cleanup_column, :old)
+
+        expect(model).to receive(:install_rename_triggers_for_postgresql)
+          .with(trigger_name, '"users"', '"old"', '"old_for_type_change"')
+
+        model.undo_cleanup_concurrent_column_type_change(
+          :users,
+          :old,
+          :string,
+          type_cast_function: :custom_type_cast_function,
+          batch_column_name: :other_batch_column
+        )
+      end
+
+      it 'raises an error with invalid batch_column_name' do
+        expect do
+          model.undo_cleanup_concurrent_column_type_change(:users, :old, :new, batch_column_name: :invalid)
+        end.to raise_error(RuntimeError, /Column invalid does not exist on users/)
+      end
+    end
+  end
+
   describe '#install_rename_triggers_for_postgresql' do
     it 'installs the triggers for PostgreSQL' do
       expect(model).to receive(:execute)