Merge pull request rails#53232 from kamipo/extract_drop_constraint

Extract `remove_constraint` for SQL standard `DROP CONSTRAINT` syntax

Author: kamipo

activerecord/lib/active_record/connection_adapters/abstract/schema_creation.rb

@@ -28,6 +28,7 @@ def visit_AlterTable(o)
           sql << o.foreign_key_drops.map { |fk| visit_DropForeignKey fk }.join(" ")
           sql << o.check_constraint_adds.map { |con| visit_AddCheckConstraint con }.join(" ")
           sql << o.check_constraint_drops.map { |con| visit_DropCheckConstraint con }.join(" ")
+          sql << o.constraint_drops.map { |con| visit_DropConstraint con }.join(" ")
         end
 
         def visit_ColumnDefinition(o)
@@ -96,9 +97,11 @@ def visit_AddForeignKey(o)
           "ADD #{accept(o)}"
         end
 
-        def visit_DropForeignKey(name)
+        def visit_DropConstraint(name)
           "DROP CONSTRAINT #{quote_column_name(name)}"
         end
+        alias :visit_DropForeignKey :visit_DropConstraint
+        alias :visit_DropCheckConstraint :visit_DropConstraint
 
         def visit_CreateIndexDefinition(o)
           index = o.index
@@ -127,10 +130,6 @@ def visit_AddCheckConstraint(o)
           "ADD #{accept(o)}"
         end
 
-        def visit_DropCheckConstraint(name)
-          "DROP CONSTRAINT #{quote_column_name(name)}"
-        end
-
         def quoted_columns(o)
           String === o.columns ? o.columns : quoted_columns_for_index(o.columns, o.column_options)
         end

activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-
 module ActiveRecord
   module ConnectionAdapters # :nodoc:
     # Abstract representation of an index definition on a table. Instances of
@@ -622,6 +621,7 @@ class AlterTable # :nodoc:
       attr_reader :adds
       attr_reader :foreign_key_adds, :foreign_key_drops
       attr_reader :check_constraint_adds, :check_constraint_drops
+      attr_reader :constraint_drops
 
       def initialize(td)
         @td   = td
@@ -630,6 +630,7 @@ def initialize(td)
         @foreign_key_drops = []
         @check_constraint_adds = []
         @check_constraint_drops = []
+        @constraint_drops = []
       end
 
       def name; @td.name; end
@@ -650,6 +651,10 @@ def drop_check_constraint(constraint_name)
         @check_constraint_drops << constraint_name
       end
 
+      def drop_constraint(constraint_name)
+        @constraint_drops << constraint_name
+      end
+
       def add_column(name, type, **options)
         name = name.to_s
         type = type.to_sym

activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb

@@ -1334,7 +1334,6 @@ def remove_check_constraint(table_name, expression = nil, if_exists: false, **op
         execute schema_creation.accept(at)
       end
 
-
       # Checks to see if a check constraint exists on a table for a given check constraint definition.
       #
       #   check_constraint_exists?(:products, name: "price_check")
@@ -1346,6 +1345,13 @@ def check_constraint_exists?(table_name, **options)
         check_constraint_for(table_name, **options).present?
       end
 
+      def remove_constraint(table_name, constraint_name) # :nodoc:
+        at = create_alter_table(table_name)
+        at.drop_constraint(constraint_name)
+
+        execute schema_creation.accept(at)
+      end
+
       def dump_schema_information # :nodoc:
         versions = pool.schema_migration.versions
         insert_versions_sql(versions) if versions.any?

activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb

@@ -11,9 +11,7 @@ def visit_AlterTable(o)
             sql = super
             sql << o.constraint_validations.map { |fk| visit_ValidateConstraint fk }.join(" ")
             sql << o.exclusion_constraint_adds.map { |con| visit_AddExclusionConstraint con }.join(" ")
-            sql << o.exclusion_constraint_drops.map { |con| visit_DropExclusionConstraint con }.join(" ")
             sql << o.unique_constraint_adds.map { |con| visit_AddUniqueConstraint con }.join(" ")
-            sql << o.unique_constraint_drops.map { |con| visit_DropUniqueConstraint con }.join(" ")
           end
 
           def visit_AddForeignKey(o)
@@ -72,18 +70,10 @@ def visit_AddExclusionConstraint(o)
             "ADD #{accept(o)}"
           end
 
-          def visit_DropExclusionConstraint(name)
-            "DROP CONSTRAINT #{quote_column_name(name)}"
-          end
-
           def visit_AddUniqueConstraint(o)
             "ADD #{accept(o)}"
           end
 
-          def visit_DropUniqueConstraint(name)
-            "DROP CONSTRAINT #{quote_column_name(name)}"
-          end
-
           def visit_ChangeColumnDefinition(o)
             column = o.column
             column.sql_type = type_to_sql(column.type, **column.options)

activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb

@@ -356,15 +356,13 @@ def validate_check_constraint(*args)
 
       # = Active Record PostgreSQL Adapter Alter \Table
       class AlterTable < ActiveRecord::ConnectionAdapters::AlterTable
-        attr_reader :constraint_validations, :exclusion_constraint_adds, :exclusion_constraint_drops, :unique_constraint_adds, :unique_constraint_drops
+        attr_reader :constraint_validations, :exclusion_constraint_adds, :unique_constraint_adds
 
         def initialize(td)
           super
           @constraint_validations = []
           @exclusion_constraint_adds = []
-          @exclusion_constraint_drops = []
           @unique_constraint_adds = []
-          @unique_constraint_drops = []
         end
 
         def validate_constraint(name)
@@ -375,17 +373,9 @@ def add_exclusion_constraint(expression, options)
           @exclusion_constraint_adds << @td.new_exclusion_constraint_definition(expression, options)
         end
 
-        def drop_exclusion_constraint(constraint_name)
-          @exclusion_constraint_drops << constraint_name
-        end
-
         def add_unique_constraint(column_name, options)
           @unique_constraint_adds << @td.new_unique_constraint_definition(column_name, options)
         end
-
-        def drop_unique_constraint(unique_constraint_name)
-          @unique_constraint_drops << unique_constraint_name
-        end
       end
     end
   end

activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb

@@ -764,10 +764,7 @@ def exclusion_constraint_options(table_name, expression, options) # :nodoc:
         def remove_exclusion_constraint(table_name, expression = nil, **options)
           excl_name_to_delete = exclusion_constraint_for!(table_name, expression: expression, **options).name
 
-          at = create_alter_table(table_name)
-          at.drop_exclusion_constraint(excl_name_to_delete)
-
-          execute schema_creation.accept(at)
+          remove_constraint(table_name, excl_name_to_delete)
         end
 
         # Adds a new unique constraint to the table.
@@ -819,10 +816,7 @@ def unique_constraint_options(table_name, column_name, options) # :nodoc:
         def remove_unique_constraint(table_name, column_name = nil, **options)
           unique_name_to_delete = unique_constraint_for!(table_name, column: column_name, **options).name
 
-          at = create_alter_table(table_name)
-          at.drop_unique_constraint(unique_name_to_delete)
-
-          execute schema_creation.accept(at)
+          remove_constraint(table_name, unique_name_to_delete)
         end
 
         # Maps logical Rails types to PostgreSQL-specific data types.

activerecord/test/cases/migration/check_constraint_test.rb

@@ -242,6 +242,16 @@ def test_check_constraint_exists_ensures_required_options
           assert_equal "At least one of :name or :expression must be supplied", error.message
         end
 
+        if supports_sql_standard_drop_constraint?
+          def test_remove_constraint
+            @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check"
+
+            assert_equal 1, @connection.check_constraints("trades").size
+            @connection.remove_constraint :trades, "quantity_check"
+            assert_equal 0, @connection.check_constraints("trades").size
+          end
+        end
+
         def test_remove_check_constraint
           @connection.add_check_constraint :trades, "price > 0", name: "price_check"
           @connection.add_check_constraint :trades, "quantity > 0", name: "quantity_check"
@@ -313,7 +323,7 @@ def test_remove_constraint_from_change_table_with_options
 else
   module ActiveRecord
     class Migration
-      class NoForeignKeySupportTest < ActiveRecord::TestCase
+      class NoCheckConstraintSupportTest < ActiveRecord::TestCase
         setup do
           @connection = ActiveRecord::Base.lease_connection
         end

activerecord/test/cases/migration/foreign_key_test.rb

@@ -380,6 +380,16 @@ def test_foreign_key_exists_in_change_table
           end
         end
 
+        if supports_sql_standard_drop_constraint?
+          def test_remove_constraint
+            @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk"
+
+            assert_equal 1, @connection.foreign_keys("astronauts").size
+            @connection.remove_constraint :astronauts, "fancy_named_fk"
+            assert_equal [], @connection.foreign_keys("astronauts")
+          end
+        end
+
         def test_remove_foreign_key_inferes_column
           @connection.add_foreign_key :astronauts, :rockets
 

activerecord/test/cases/test_case.rb

@@ -324,7 +324,6 @@ def self.run(*args)
     end
   end
 
-
   class SQLite3TestCase < TestCase
     def self.run(*args)
       super if current_adapter?(:SQLite3Adapter)

activerecord/test/support/adapter_helper.rb

@@ -48,6 +48,21 @@ def supports_text_column_with_default?
     end
   end
 
+  def supports_sql_standard_drop_constraint?
+    if current_adapter?(:SQLite3Adapter)
+      false
+    elsif current_adapter?(:Mysql2Adapter, :TrilogyAdapter)
+      conn = ActiveRecord::Base.lease_connection
+      if conn.mariadb?
+        conn.database_version >= "10.3.13"
+      else
+        conn.database_version >= "8.0.19"
+      end
+    else
+      true
+    end
+  end
+
   %w[
     supports_savepoints?
     supports_partial_index?