bigint migration - events table - step 1

Author: philippthun

.rubocop_cc.yml

@@ -136,8 +136,8 @@ Rails/DangerousColumnNames: # Disabled, in comparison to active_record we need t
 Rails/SkipsModelValidations: # We don`t want any model at all in migrations and migration specs
   Enabled: true
   Exclude:
-    - db/migrations/*
-    - spec/migrations/*
+    - db/migrations/**/*
+    - spec/migrations/**/*
 
 #### ENABLED SECTION
 Gemspec/DeprecatedAttributeAssignment:

db/helpers/bigint_migration.rb

@@ -0,0 +1,84 @@
+# helpers
+def opt_out?
+  opt_out = VCAP::CloudController::Config.config&.get(:skip_bigint_id_migration)
+  opt_out.nil? ? false : opt_out
+rescue VCAP::CloudController::Config::InvalidConfigPath
+  false
+end
+
+# DSL
+def empty?(table)
+  raise unless is_a?(Sequel::Database)
+
+  self[table].count == 0
+end
+
+def change_pk_to_bigint(table)
+  raise unless is_a?(Sequel::Database)
+
+  set_column_type(table, :id, :Bignum) if column_type(self, table, :id) != BIGINT_TYPE
+end
+
+def add_bigint_column(table)
+  raise unless is_a?(Sequel::Database)
+
+  add_column(table, :id_bigint, :Bignum) unless schema(table).map(&:first).include?(:id_bigint)
+end
+
+def drop_trigger_function(table)
+  raise unless is_a?(Sequel::Database)
+
+  if database_type == :postgres
+    drop_trigger(table, trigger_name(table), if_exists: true)
+    drop_function(function_name(table), if_exists: true)
+  elsif database_type == :mysql
+    run("DROP TRIGGER #{trigger_name(table)};")
+  end
+end
+
+def create_trigger_function(table)
+  drop_trigger_function(table)
+
+  raise unless is_a?(Sequel::Database)
+
+  if database_type == :postgres
+    function = <<~FUNC
+      BEGIN
+        NEW.id_bigint := NEW.id;
+        RETURN NEW;
+      END;
+    FUNC
+    create_function(function_name(table), function, language: :plpgsql, returns: :trigger)
+    create_trigger(table, trigger_name(table), function_name(table), each_row: true, events: :insert)
+  elsif database_type == :mysql
+    run("CREATE TRIGGER #{trigger_name(table)} BEFORE INSERT ON #{table} FOR EACH ROW SET NEW.id_bigint = NEW.id;")
+  end
+end
+
+def revert_pk_to_integer(table)
+  raise unless is_a?(Sequel::Database)
+
+  set_column_type(table, :id, :integer) if column_type(self, table, :id) == BIGINT_TYPE
+end
+
+def drop_bigint_column(table)
+  raise unless is_a?(Sequel::Database)
+
+  drop_column(table, :id_bigint) if schema(table).map(&:first).include?(:id_bigint)
+end
+
+# internal constants
+BIGINT_TYPE = 'bigint'.freeze
+
+# internal helpers
+def column_type(db, table, column)
+  db.schema(table).find { |col, _| col == column }&.dig(1, :db_type)
+end
+
+def function_name(table)
+  :"#{table}_set_id_bigint_on_insert"
+end
+
+def trigger_name(table)
+  :"trigger_#{function_name(table)}"
+end

db/migrations/20250327142351_bigint_migration_events_step1.rb

@@ -0,0 +1,20 @@
+require File.expand_path('../helpers/bigint_migration', __dir__)
+
+Sequel.migration do
+  up do
+    unless opt_out?
+      if empty?(:events)
+        change_pk_to_bigint(:events)
+      else
+        add_bigint_column(:events)
+        create_trigger_function(:events)
+      end
+    end
+  end
+
+  down do
+    revert_pk_to_integer(:events)
+    drop_trigger_function(:events)
+    drop_bigint_column(:events)
+  end
+end

lib/cloud_controller/config_schemas/base/api_schema.rb

@@ -99,6 +99,7 @@ class ApiSchema < VCAP::Config
             optional(:max_migration_statement_runtime_in_seconds) => Integer,
             optional(:migration_psql_concurrent_statement_timeout_in_seconds) => Integer,
             optional(:migration_psql_worker_memory_kb) => Integer,
+            optional(:skip_bigint_id_migration) => bool,
             db: {
               optional(:database) => Hash, # db connection hash for sequel
               max_connections: Integer, # max connections in the connection pool

lib/cloud_controller/config_schemas/base/migrate_schema.rb

@@ -10,6 +10,7 @@ class MigrateSchema < VCAP::Config
             optional(:max_migration_statement_runtime_in_seconds) => Integer,
             optional(:migration_psql_concurrent_statement_timeout_in_seconds) => Integer,
             optional(:migration_psql_worker_memory_kb) => Integer,
+            optional(:skip_bigint_id_migration) => bool,
 
             db: {
               optional(:database) => Hash, # db connection hash for sequel

spec/migrations/20250327142351_bigint_migration_events_step1_spec.rb

@@ -0,0 +1,16 @@
+require 'spec_helper'
+require 'migrations/helpers/bigint_migration_step1_shared_context'
+
+RSpec.describe 'bigint migration - events table - step1', isolation: :truncation, type: :migration do
+  include_context 'bigint migration step1' do
+    let(:migration_filename) { '20250327142351_bigint_migration_events_step1.rb' }
+    let(:table) { :events }
+    let(:insert_hash) do
+      {
+        guid: 'guid', timestamp: Time.now.utc, type: 'type',
+        actor: 'actor', actor_type: 'actor_type',
+        actee: 'actee', actee_type: 'actee_type'
+      }
+    end
+  end
+end

spec/migrations/bigint_migration_helpers_spec.rb

@@ -0,0 +1,46 @@
+require 'spec_helper'
+require_relative '../../db/helpers/bigint_migration'
+
+RSpec.describe 'bigint migration helpers', isolation: :truncation, type: :migration do
+  let(:skip_bigint_id_migration) { nil }
+
+  before do
+    allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:skip_bigint_id_migration).and_return(skip_bigint_id_migration)
+  end
+
+  describe '#opt_out?' do
+    context 'when skip_bigint_id_migration is false' do
+      let(:skip_bigint_id_migration) { false }
+
+      it 'returns false' do
+        expect(opt_out?).to be(false)
+      end
+    end
+
+    context 'when skip_bigint_id_migration is true' do
+      let(:skip_bigint_id_migration) { true }
+
+      it 'returns true' do
+        expect(opt_out?).to be(true)
+      end
+    end
+
+    context 'when skip_bigint_id_migration is nil' do
+      let(:skip_bigint_id_migration) { nil }
+
+      it 'returns false' do
+        expect(opt_out?).to be(false)
+      end
+    end
+
+    context 'when raising InvalidConfigPath error' do
+      before do
+        allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:skip_bigint_id_migration).and_raise(VCAP::CloudController::Config::InvalidConfigPath)
+      end
+
+      it 'returns false' do
+        expect(opt_out?).to be(false)
+      end
+    end
+  end
+end

spec/migrations/helpers/bigint_migration_step1_shared_context.rb

@@ -0,0 +1,127 @@
+require 'migrations/helpers/migration_shared_context'
+
+RSpec.shared_context 'bigint migration step1' do
+  subject(:run_migration) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index, allow_missing_migration_files: true) }
+
+  include_context 'migration'
+
+  let(:skip_bigint_id_migration) { nil }
+
+  before do
+    allow_any_instance_of(VCAP::CloudController::Config).to receive(:get).with(:skip_bigint_id_migration).and_return(skip_bigint_id_migration)
+  end
+
+  describe 'up' do
+    context 'when skip_bigint_id_migration is false' do
+      let(:skip_bigint_id_migration) { false }
+
+      context 'when the table is empty' do
+        before do
+          db[table].delete
+        end
+
+        it "changes the id column's type to bigint" do
+          expect(db).to have_table_with_column_and_type(table, :id, :integer)
+
+          run_migration
+
+          expect(db).to have_table_with_column_and_type(table, :id, :bigint)
+        end
+
+        it 'does not add the id_bigint column' do
+          expect(db).not_to have_table_with_column(table, :id_bigint)
+
+          run_migration
+
+          expect(db).not_to have_table_with_column(table, :id_bigint)
+        end
+      end
+
+      context 'when the table is not empty' do
+        before do
+          db[table].insert(insert_hash)
+        end
+
+        it "does not change the id column's type" do
+          expect(db).to have_table_with_column_and_type(table, :id, :integer)
+
+          run_migration
+
+          expect(db).to have_table_with_column_and_type(table, :id, :integer)
+        end
+
+        it 'adds the id_bigint column' do
+          expect(db).not_to have_table_with_column(table, :id_bigint)
+
+          run_migration
+
+          expect(db).to have_table_with_column_and_type(table, :id_bigint, :bigint)
+        end
+
+        it 'creates the trigger function' do
+          expect(db).not_to have_trigger_function_for_table(table) unless db.database_type == :mysql
+
+          run_migration
+
+          expect(db).to have_trigger_function_for_table(table) unless db.database_type == :mysql
+        end
+      end
+    end
+
+    context 'when skip_bigint_id_migration is true' do
+      let(:skip_bigint_id_migration) { true }
+
+      it "neither changes the id column's type, nor adds the id_bigint column" do
+        expect(db).to have_table_with_column_and_type(table, :id, :integer)
+        expect(db).not_to have_table_with_column(table, :id_bigint)
+
+        run_migration
+
+        expect(db).to have_table_with_column_and_type(table, :id, :integer)
+        expect(db).not_to have_table_with_column(table, :id_bigint)
+      end
+    end
+  end
+
+  describe 'down' do
+    subject(:run_rollback) { Sequel::Migrator.run(db, migrations_path, target: current_migration_index - 1, allow_missing_migration_files: true) }
+
+    context 'when the table is empty' do
+      before do
+        db[table].delete
+        run_migration
+      end
+
+      it "reverts the id column's type to integer" do
+        expect(db).to have_table_with_column_and_type(table, :id, :bigint)
+
+        run_rollback
+
+        expect(db).to have_table_with_column_and_type(table, :id, :integer)
+      end
+    end
+
+    context 'when the table is not empty' do
+      before do
+        db[table].insert(insert_hash)
+        run_migration
+      end
+
+      it 'drops the id_bigint column' do
+        expect(db).to have_table_with_column(table, :id_bigint)
+
+        run_rollback
+
+        expect(db).not_to have_table_with_column(table, :id_bigint)
+      end
+
+      it 'drops the trigger function' do
+        expect(db).to have_trigger_function_for_table(table) unless db.database_type == :mysql
+
+        run_rollback
+
+        expect(db).not_to have_trigger_function_for_table(table) unless db.database_type == :mysql
+      end
+    end
+  end
+end

spec/migrations/helpers/matchers.rb

@@ -15,3 +15,63 @@
     !options.delete(:name).nil? && (options - %i[if_exists concurrently]).empty?
   end
 end
+
+RSpec::Matchers.define :have_table_with_column do |table, column|
+  match do |db|
+    db[table].columns.include?(column)
+  end
+end
+
+RSpec::Matchers.define :have_table_with_column_and_type do |table, column, type|
+  match do |db|
+    expect(db).to have_table_with_column(table, column)
+
+    db.schema(table).find { |col, _| col == column }&.dig(1, :db_type) == db_type(db, type)
+  end
+end
+
+def db_type(db, type)
+  case type
+  when :integer
+    if db.database_type == :postgres
+      'integer'
+    elsif db.database_type == :mysql
+      'int'
+    else
+      raise "unsupported database type: #{db.database_type}"
+    end
+  when :bigint
+    'bigint'
+  else
+    raise "unsupported type: #{type}"
+  end
+end
+
+RSpec::Matchers.define :have_trigger_function_for_table do |table|
+  match do |db|
+    function_name = :"#{table}_set_id_bigint_on_insert"
+    trigger_name = :"trigger_#{function_name}"
+
+    if db.database_type == :postgres
+      function_exists = false
+      begin
+        db.fetch("select pg_get_functiondef('#{function_name}()'::regprocedure);") do
+          function_exists = true
+        end
+      rescue Sequel::DatabaseError => e
+        raise 'unexpected database error' unless e.message =~ /PG::UndefinedFunction/
+      end
+
+      trigger_exists = false
+      db.fetch("select 1 as one from information_schema.triggers WHERE trigger_name = '#{trigger_name}' limit 1;") do
+        trigger_exists = true
+      end
+
+      raise 'either function and trigger must exist or none of them' if function_exists != trigger_exists
+
+      function_exists && trigger_exists
+    elsif db.database_type == :mysql
+      # TODO
+    end
+  end
+end