Merge pull request rails#45735 from ghiculescu/drop-enum

Add `drop_enum` command for Postgres

Author: yahonda

activerecord/CHANGELOG.md

@@ -1,3 +1,10 @@
+*   Add `drop_enum` migration command for PostgreSQL
+
+    This does the inverse of `create_enum`. Before dropping an enum, ensure you have
+    dropped columns that depend on it.
+
+    *Alex Ghiculescu*
+
 *   Adds support for `if_exists` option when removing a check constraint.
 
     The `remove_check_constraint` method now accepts an `if_exists` option. If set

activerecord/lib/active_record/connection_adapters/abstract_adapter.rb

@@ -535,6 +535,10 @@ def enable_extension(name, **)
       def create_enum(*) # :nodoc:
       end
 
+      # This is meant to be implemented by the adapters that support custom enum types
+      def drop_enum(*) # :nodoc:
+      end
+
       def advisory_locks_enabled? # :nodoc:
         supports_advisory_locks? && @advisory_locks_enabled
       end

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -486,6 +486,17 @@ def create_enum(name, values)
         exec_query(query)
       end
 
+      # Drops an enum type.
+      # If the `if_exists: true` option is provided, the enum is only dropped if it exists.
+      # Otherwise, if the enum doesn't exist, an error is raised.
+      def drop_enum(name, *args)
+        options = args.extract_options!
+        query = <<~SQL
+          DROP TYPE#{' IF EXISTS' if options[:if_exists]} #{quote_table_name(name)};
+        SQL
+        exec_query(query)
+      end
+
       # Returns the configured supported identifier length supported by PostgreSQL
       def max_identifier_length
         @max_identifier_length ||= query_value("SHOW max_identifier_length", "SCHEMA").to_i

activerecord/lib/active_record/migration/command_recorder.rb

@@ -45,7 +45,8 @@ class CommandRecorder
         :add_foreign_key, :remove_foreign_key,
         :change_column_comment, :change_table_comment,
         :add_check_constraint, :remove_check_constraint,
-        :add_exclusion_constraint, :remove_exclusion_constraint
+        :add_exclusion_constraint, :remove_exclusion_constraint,
+        :create_enum, :drop_enum
       ]
       include JoinTable
 
@@ -144,7 +145,8 @@ module StraightReversions # :nodoc:
               add_foreign_key:   :remove_foreign_key,
               add_check_constraint: :remove_check_constraint,
               add_exclusion_constraint: :remove_exclusion_constraint,
-              enable_extension:  :disable_extension
+              enable_extension:  :disable_extension,
+              create_enum:       :drop_enum
             }.each do |cmd, inv|
               [[inv, cmd], [cmd, inv]].uniq.each do |method, inverse|
                 class_eval <<-EOV, __FILE__, __LINE__ + 1

activerecord/test/cases/adapters/postgresql/enum_test.rb

@@ -30,8 +30,9 @@ def setup
   end
 
   teardown do
+    reset_connection
     @connection.drop_table "postgresql_enums", if_exists: true
-    @connection.execute "DROP TYPE IF EXISTS mood"
+    @connection.drop_enum "mood", if_exists: true
     reset_connection
   end
 
@@ -126,6 +127,22 @@ def test_schema_load
     $stdout = original
   end
 
+  def test_drop_enum
+    @connection.create_enum :unused, []
+
+    assert_nothing_raised do
+      @connection.drop_enum "unused"
+    end
+
+    assert_nothing_raised do
+      @connection.drop_enum "unused", if_exists: true
+    end
+
+    assert_raises ActiveRecord::StatementInvalid do
+      @connection.drop_enum "unused"
+    end
+  end
+
   def test_works_with_activerecord_enum
     model = PostgresqlEnum.create!
     model.current_mood_okay!
@@ -145,13 +162,18 @@ def test_enum_type_scoped_to_schemas
     @connection.create_schema("test_schema")
     @connection.schema_search_path = "test_schema"
     @connection.schema_cache.clear!
-    @connection.create_enum("mood", ["sad", "ok", "happy"])
+    @connection.create_enum("mood_in_other_schema", ["sad", "ok", "happy"])
 
     assert_nothing_raised do
-      @connection.create_table("postgresql_enums") do |t|
-        t.column :current_mood, :mood, default: "happy", null: false
+      @connection.create_table("postgresql_enums_in_other_schema") do |t|
+        t.column :current_mood, :mood_in_other_schema, default: "happy", null: false
       end
     end
+
+    assert_nothing_raised do
+      @connection.drop_table("postgresql_enums_in_other_schema")
+      @connection.drop_enum("mood_in_other_schema")
+    end
   ensure
     @connection.drop_schema("test_schema")
     @connection.schema_search_path = old_search_path
@@ -160,13 +182,18 @@ def test_enum_type_scoped_to_schemas
 
   def test_enum_type_explicit_schema
     @connection.create_schema("test_schema")
-    @connection.create_enum("test_schema.mood", ["sad", "ok", "happy"])
+    @connection.create_enum("test_schema.mood_in_other_schema", ["sad", "ok", "happy"])
 
-    @connection.create_table("test_schema.postgresql_enums") do |t|
-      t.column :current_mood, "test_schema.mood"
+    @connection.create_table("test_schema.postgresql_enums_in_other_schema") do |t|
+      t.column :current_mood, "test_schema.mood_in_other_schema"
     end
 
-    assert @connection.table_exists?("test_schema.postgresql_enums")
+    assert @connection.table_exists?("test_schema.postgresql_enums_in_other_schema")
+
+    assert_nothing_raised do
+      @connection.drop_table("test_schema.postgresql_enums_in_other_schema")
+      @connection.drop_enum("test_schema.mood_in_other_schema")
+    end
   ensure
     @connection.drop_schema("test_schema", if_exists: true)
   end

activerecord/test/cases/adapters/postgresql/invertible_migration_test.rb

@@ -20,6 +20,15 @@ def change
     end
   end
 
+  class CreateEnumMigration < SilentMigration
+    def change
+      create_enum :color, ["blue", "green"]
+      create_table :enums do |t|
+        t.enum :best_color, enum_type: "color", default: "blue", null: false
+      end
+    end
+  end
+
   self.use_transactional_tests = false
 
   setup do
@@ -28,6 +37,7 @@ def change
 
   teardown do
     @connection.drop_table "settings", if_exists: true
+    @connection.drop_table "enums", if_exists: true
   end
 
   def test_migrate_revert_add_index_with_expression
@@ -43,4 +53,16 @@ def test_migrate_revert_add_index_with_expression
     assert_not @connection.index_exists?(:settings, nil, name: "index_settings_data_foo"),
                "index index_settings_data_foo should not exist"
   end
+
+  def test_revert_create_enum
+    CreateEnumMigration.new.migrate(:up)
+
+    assert @connection.column_exists?(:enums, :best_color, sql_type: "color", default: "blue", null: false)
+    assert_equal [["color", "blue,green"]], @connection.enum_types
+
+    CreateEnumMigration.new.migrate(:down)
+
+    assert_not @connection.table_exists?(:enums)
+    assert_equal [], @connection.enum_types
+  end
 end

activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb

@@ -396,12 +396,12 @@ def test_raise_error_when_cannot_translate_exception
       end
 
       def test_reload_type_map_for_newly_defined_types
-        @connection.execute "CREATE TYPE feeling AS ENUM ('good', 'bad')"
+        @connection.create_enum "feeling", ["good", "bad"]
         result = @connection.select_all "SELECT 'good'::feeling"
         assert_instance_of(PostgreSQLAdapter::OID::Enum,
                            result.column_types["feeling"])
       ensure
-        @connection.execute "DROP TYPE IF EXISTS feeling"
+        @connection.drop_enum "feeling", if_exists: true
         reset_connection
       end
 

guides/source/active_record_postgresql.md

@@ -261,14 +261,12 @@ def up
   end
 end
 
-# There's no built in support for dropping enums, but you can do it manually.
-# You should first drop any table that depends on them.
+# The above migration is reversible (using #change), but you can
+# also define a #down method:
 def down
   drop_table :articles
 
-  execute <<-SQL
-    DROP TYPE article_status;
-  SQL
+  drop_enum :article_status
 end
 ```