Update AddColumnsToWideTables cop

This commit updates our existing cop to prevent adding columns
on large tables.

db/migrate/20240820125541_add_snoozed_until_to_todos.rb

@@ -4,6 +4,6 @@ class AddSnoozedUntilToTodos < Gitlab::Database::Migration[2.2]
   milestone '17.4'
 
   def change
-    add_column :todos, :snoozed_until, :datetime_with_timezone
+    add_column :todos, :snoozed_until, :datetime_with_timezone # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
   end
 end

db/migrate/20240821043019_add_project_id_to_ci_build_needs.rb

@@ -4,6 +4,6 @@ class AddProjectIdToCiBuildNeeds < Gitlab::Database::Migration[2.2]
   milestone '17.4'
 
   def change
-    add_column :ci_build_needs, :project_id, :bigint
+    add_column :ci_build_needs, :project_id, :bigint # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
   end
 end

db/migrate/20240821074355_add_project_id_to_ci_job_variables.rb

@@ -4,6 +4,6 @@ class AddProjectIdToCiJobVariables < Gitlab::Database::Migration[2.2]
   milestone '17.4'
 
   def change
-    add_column(:ci_job_variables, :project_id, :bigint)
+    add_column(:ci_job_variables, :project_id, :bigint) # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
   end
 end

db/migrate/20240822195511_add_notification_cols_to_personal_access_tokens.rb

@@ -4,9 +4,15 @@ class AddNotificationColsToPersonalAccessTokens < Gitlab::Database::Migration[2.
   milestone '17.4'
 
   def up
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :personal_access_tokens, :seven_days_notification_sent_at, :datetime_with_timezone
+    # rubocop:enable Migration/PreventAddingColumns
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :personal_access_tokens, :thirty_days_notification_sent_at, :datetime_with_timezone
+    # rubocop:enable Migration/PreventAddingColumns
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :personal_access_tokens, :sixty_days_notification_sent_at, :datetime_with_timezone
+    # rubocop:enable Migration/PreventAddingColumns
   end
 
   def down

db/migrate/20240909171100_add_has_vulnerability_resolution_to_vulnerability_reads.rb

@@ -4,6 +4,8 @@ class AddHasVulnerabilityResolutionToVulnerabilityReads < Gitlab::Database::Migr
   milestone '17.4'
 
   def change
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :vulnerability_reads, :has_vulnerability_resolution, :boolean, default: false
+    # rubocop:enable Migration/PreventAddingColumns
   end
 end

db/migrate/20240909230052_add_personal_namespace_id_to_events_self_managed.rb

@@ -11,7 +11,7 @@ def up
     return if column_exists?(:events, :personal_namespace_id)
 
     with_lock_retries(raise_on_exhaustion: true) do
-      add_column :events, :personal_namespace_id, :bigint
+      add_column :events, :personal_namespace_id, :bigint # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     end
   end
 

db/migrate/20240924213521_add_correct_work_item_type_id_to_issues.rb

@@ -6,6 +6,8 @@ class AddCorrectWorkItemTypeIdToIssues < Gitlab::Database::Migration[2.2]
   def change
     # Defaulting to 0 here to avoid validating the not null constraint afterwards as done in
     # https://gitlab.com/gitlab-org/gitlab/-/blob/a24ea906d46589c3397660eaf3223d5af6ad9708/lib/gitlab/database/migration_helpers.rb#L1182-1182
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :issues, :correct_work_item_type_id, :bigint, null: false, default: 0
+    # rubocop:enable Migration/PreventAddingColumns
   end
 end

db/migrate/20241014081022_add_project_id_to_ci_pipeline_messages.rb

@@ -4,6 +4,6 @@ class AddProjectIdToCiPipelineMessages < Gitlab::Database::Migration[2.2]
   milestone '17.6'
 
   def change
-    add_column(:ci_pipeline_messages, :project_id, :bigint)
+    add_column(:ci_pipeline_messages, :project_id, :bigint) # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
   end
 end

db/migrate/20241014114621_add_project_id_to_security_findings.rb

@@ -4,6 +4,6 @@ class AddProjectIdToSecurityFindings < Gitlab::Database::Migration[2.2]
   milestone '17.6'
 
   def change
-    add_column :security_findings, :project_id, :bigint
+    add_column :security_findings, :project_id, :bigint # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
   end
 end

db/migrate/20241021141853_add_auto_resolved_to_vulnerabilities.rb

@@ -4,6 +4,8 @@ class AddAutoResolvedToVulnerabilities < Gitlab::Database::Migration[2.2]
   milestone '17.6'
 
   def change
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :vulnerabilities, :auto_resolved, :boolean, null: false, default: false, if_not_exists: true
+    # rubocop:enable Migration/PreventAddingColumns
   end
 end

db/migrate/20241021143317_add_auto_resolved_to_vulnerability_reads.rb

@@ -4,6 +4,8 @@ class AddAutoResolvedToVulnerabilityReads < Gitlab::Database::Migration[2.2]
   milestone '17.6'
 
   def change
+    # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     add_column :vulnerability_reads, :auto_resolved, :boolean, null: false, default: false, if_not_exists: true
+    # rubocop:enable Migration/PreventAddingColumns
   end
 end

db/post_migrate/20240905022445_add_personal_namespace_id_to_events_2.rb

@@ -15,7 +15,7 @@ def up
       # Doing DDL in post-deployment migration is discouraged in general,
       # this is done as a workaround to prevent production incidents when
       # changing the schema for very high-traffic table
-      add_column :events, :personal_namespace_id, :bigint
+      add_column :events, :personal_namespace_id, :bigint # rubocop:disable Migration/PreventAddingColumns -- Legacy migration
     end
   end
 

rubocop/cop/migration/add_columns_to_wide_tables.rb → rubocop/cop/migration/prevent_adding_columns.rb

@@ -6,11 +6,15 @@ module RuboCop
   module Cop
     module Migration
       # Cop that prevents adding columns to wide tables.
-      class AddColumnsToWideTables < RuboCop::Cop::Base
+      class PreventAddingColumns < RuboCop::Cop::Base
         include MigrationHelpers
 
-        MSG = '`%s` is a wide table with several columns, adding more should be avoided unless absolutely necessary. ' \
-              'Consider storing the column in a different table or creating a new one.'
+        MSG = <<~MSG
+          `%s` is a large table with several columns, adding more should be avoided unless absolutely necessary.
+          Consider storing the column in a different table or creating a new one.
+          The list of large tables is defined in rubocop/rubocop-migrations.yml.
+          See https://docs.gitlab.com/ee/development/database/large_tables_limitations.html
+        MSG
 
         DENYLISTED_METHODS = %i[
           add_column
@@ -18,8 +22,11 @@ class AddColumnsToWideTables < RuboCop::Cop::Base
           add_timestamps_with_timezone
         ].freeze
 
+        MIGRATION_METHODS = %i[change up].freeze
+
         def on_send(node)
           return unless in_migration?(node)
+          return unless in_up?(node)
 
           method_name = node.children[1]
           table_name = node.children[2]
@@ -31,14 +38,29 @@ def on_send(node)
 
         private
 
+        def in_up?(node)
+          node.each_ancestor(:def).any? do |def_node|
+            MIGRATION_METHODS.include?(def_node.method_name)
+          end
+        end
+
         def offense?(method_name, table_name)
-          wide_table?(table_name) &&
-            DENYLISTED_METHODS.include?(method_name)
+          table_matches?(table_name) && DENYLISTED_METHODS.include?(method_name)
+        end
+
+        def table_matches?(table_name)
+          return false unless valid_table_node?(table_name)
+
+          table_value = table_name.value
+          wide_or_large_table?(table_value)
+        end
+
+        def wide_or_large_table?(table_value)
+          WIDE_TABLES.include?(table_value) || large_tables.include?(table_value)
         end
 
-        def wide_table?(table_name)
-          table_name && table_name.type == :sym &&
-            WIDE_TABLES.include?(table_name.value)
+        def valid_table_node?(table_name)
+          table_name && table_name.type == :sym
         end
       end
     end

rubocop/migration_helpers.rb

@@ -9,7 +9,7 @@ module MigrationHelpers
       plan_limits
     ].freeze
 
-    # Tables with large number of columns (> 50 on GitLab.com as of 01/2021)
+    # Tables with large number of columns (> 50 on GitLab.com as of 2024-11-20)
     WIDE_TABLES = %i[
       ci_builds
       p_ci_builds

spec/rubocop/cop/migration/add_columns_to_wide_tables_spec.rb → spec/rubocop/cop/migration/prevent_adding_columns_spec.rb

 # frozen_string_literal: true
 
 require 'rubocop_spec_helper'
-require_relative '../../../../rubocop/cop/migration/add_columns_to_wide_tables'
+require_relative '../../../../rubocop/cop/migration/prevent_adding_columns'
 
-RSpec.describe RuboCop::Cop::Migration::AddColumnsToWideTables do
+RSpec.describe RuboCop::Cop::Migration::PreventAddingColumns, feature_category: :database do
   context 'when outside of a migration' do
     it 'does not register any offenses' do
       expect_no_offenses(<<~RUBY)
@@ -21,7 +21,7 @@ def up
 
     context 'with wide tables' do
       it 'registers an offense when adding a column to a wide table' do
-        offense = '`projects` is a wide table with several columns, [...]'
+        offense = '`projects` is a large table with several columns, [...]'
 
         expect_offense(<<~RUBY)
           def up
@@ -32,7 +32,7 @@ def up
       end
 
       it 'registers an offense when adding a column with default to a wide table' do
-        offense = '`users` is a wide table with several columns, [...]'
+        offense = '`users` is a large table with several columns, [...]'
 
         expect_offense(<<~RUBY)
           def up
@@ -43,7 +43,7 @@ def up
       end
 
       it 'registers an offense when adding a reference' do
-        offense = '`ci_builds` is a wide table with several columns, [...]'
+        offense = '`ci_builds` is a large table with several columns, [...]'
 
         expect_offense(<<~RUBY)
           def up
@@ -54,7 +54,7 @@ def up
       end
 
       it 'registers an offense when adding timestamps' do
-        offense = '`projects` is a wide table with several columns, [...]'
+        offense = '`projects` is a large table with several columns, [...]'
 
         expect_offense(<<~RUBY)
           def up
@@ -74,10 +74,33 @@ def up
     end
 
     context 'with a regular table' do
-      it 'registers no offense for notes' do
+      it 'registers no offense for licenses' do
         expect_no_offenses(<<~RUBY)
           def up
-            add_column(:notes, :another_column, :boolean)
+            add_column(:licenses, :another_column, :boolean)
+          end
+        RUBY
+      end
+    end
+
+    context 'when targeting a large table' do
+      it 'registers an offense for audit_events' do
+        offense = '`audit_events` is a large table with several columns, [...]'
+
+        expect_offense(<<~RUBY)
+          def up
+            add_column(:audit_events, :another_column, :boolean, default: false)
+            ^^^^^^^^^^ #{offense}
+          end
+        RUBY
+      end
+    end
+
+    context 'when rolling back migration' do
+      it 'registers no offense' do
+        expect_no_offenses(<<~RUBY)
+          def down
+            add_column(:notes, :another_column, :boolean, default: false)
           end
         RUBY
       end