Support schemas in Postgresql enum_types

Follow up to rails#44896 and rails#45707

Author: ghiculescu

activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

@@ -452,15 +452,19 @@ def enum_types
           SELECT
             type.typname AS name,
             type.OID AS oid,
+            n.nspname AS schema,
             string_agg(enum.enumlabel, ',' ORDER BY enum.enumsortorder) AS value
           FROM pg_enum AS enum
-          JOIN pg_type AS type
-            ON (type.oid = enum.enumtypid)
-          GROUP BY type.OID, type.typname;
+          JOIN pg_type AS type ON (type.oid = enum.enumtypid)
+          JOIN pg_namespace n ON type.typnamespace = n.oid
+          GROUP BY type.OID, n.nspname, type.typname;
         SQL
 
         exec_query(query, "SCHEMA", allow_retry: true, uses_transaction: false).cast_values.each_with_object({}) do |row, memo|
-          memo[row.first] = row.last
+          name, schema = row[0], row[2]
+          schema = nil if schema == current_schema
+          full_name = [schema, name].compact.join(".")
+          memo[full_name] = row.last
         end.to_a
       end
 

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

@@ -101,7 +101,7 @@ def test_assigning_enum_to_nil
   def test_schema_dump
     @connection.add_column "postgresql_enums", "good_mood", :mood, default: "happy", null: false
 
-    output = dump_table_schema("postgresql_enums")
+    output = dump_all_table_schema
 
     assert output.include?("# Note that some types may not work with other database engines. Be careful if changing database."), output
 
@@ -158,26 +158,17 @@ def test_works_with_activerecord_enum
   end
 
   def test_enum_type_scoped_to_schemas
-    old_search_path = @connection.schema_search_path
-    @connection.create_schema("test_schema")
-    @connection.schema_search_path = "test_schema"
-    @connection.schema_cache.clear!
-    @connection.create_enum("mood_in_other_schema", ["sad", "ok", "happy"])
+    with_test_schema("test_schema") do
+      @connection.create_enum("mood_in_other_schema", ["sad", "ok", "happy"])
 
-    assert_nothing_raised do
-      @connection.create_table("postgresql_enums_in_other_schema") do |t|
-        t.column :current_mood, :mood_in_other_schema, default: "happy", null: false
+      assert_nothing_raised do
+        @connection.create_table("postgresql_enums_in_other_schema") do |t|
+          t.column :current_mood, :mood_in_other_schema, default: "happy", null: false
+        end
       end
-    end
 
-    assert_nothing_raised do
-      @connection.drop_table("postgresql_enums_in_other_schema")
-      @connection.drop_enum("mood_in_other_schema")
+      assert @connection.table_exists?("postgresql_enums_in_other_schema")
     end
-  ensure
-    @connection.drop_schema("test_schema")
-    @connection.schema_search_path = old_search_path
-    @connection.schema_cache.clear!
   end
 
   def test_enum_type_explicit_schema
@@ -197,4 +188,53 @@ def test_enum_type_explicit_schema
   ensure
     @connection.drop_schema("test_schema", if_exists: true)
   end
+
+  def test_schema_dump_scoped_to_schemas
+    with_test_schema("test_schema") do
+      @connection.create_enum("mood_in_test_schema", ["sad", "ok", "happy"])
+      @connection.create_table("postgresql_enums_in_test_schema") do |t|
+        t.column :current_mood, :mood_in_test_schema
+      end
+
+      output = dump_all_table_schema
+
+      assert output.include?('create_enum "public.mood", ["sad", "ok", "happy"]'), output
+      assert output.include?('create_enum "mood_in_test_schema", ["sad", "ok", "happy"]'), output
+      assert output.include?('t.enum "current_mood", enum_type: "mood_in_test_schema"'), output
+    end
+  end
+
+  def test_schema_load_scoped_to_schemas
+    silence_stream($stdout) do
+      with_test_schema("test_schema", drop: false) do
+        ActiveRecord::Schema.define do
+          create_enum "mood_in_test_schema", ["sad", "ok", "happy"]
+          create_enum "public.mood", ["sad", "ok", "happy"]
+
+          create_table "postgresql_enums_in_test_schema", force: :cascade do |t|
+            t.enum "current_mood", enum_type: "mood_in_test_schema"
+          end
+        end
+
+        assert @connection.column_exists?(:postgresql_enums_in_test_schema, :current_mood, sql_type: "mood_in_test_schema")
+      end
+
+      # This is outside `with_test_schema`, so we need to explicitly specify which schema we query
+      assert @connection.column_exists?("test_schema.postgresql_enums_in_test_schema", :current_mood, sql_type: "test_schema.mood_in_test_schema")
+    end
+  ensure
+    @connection.drop_schema("test_schema")
+  end
+
+  private
+    def with_test_schema(name, drop: true)
+      old_search_path = @connection.schema_search_path
+      @connection.create_schema(name)
+      @connection.schema_search_path = name
+      yield
+    ensure
+      @connection.drop_schema(name) if drop
+      @connection.schema_search_path = old_search_path
+      @connection.schema_cache.clear!
+    end
 end

activerecord/test/cases/schema_dumper_test.rb

@@ -12,11 +12,7 @@ class SchemaDumperTest < ActiveRecord::TestCase
   end
 
   def standard_dump
-    @@standard_dump ||= perform_schema_dump
-  end
-
-  def perform_schema_dump
-    dump_all_table_schema []
+    @@standard_dump ||= dump_all_table_schema
   end
 
   def test_dump_schema_information_with_empty_versions
@@ -354,13 +350,13 @@ def test_schema_dump_includes_extensions
       connection = ActiveRecord::Base.connection
 
       connection.stub(:extensions, ["hstore"]) do
-        output = perform_schema_dump
+        output = dump_all_table_schema
         assert_match "# These are extensions that must be enabled", output
         assert_match %r{enable_extension "hstore"}, output
       end
 
       connection.stub(:extensions, []) do
-        output = perform_schema_dump
+        output = dump_all_table_schema
         assert_no_match "# These are extensions that must be enabled", output
         assert_no_match %r{enable_extension}, output
       end
@@ -370,13 +366,13 @@ def test_schema_dump_includes_extensions_in_alphabetic_order
       connection = ActiveRecord::Base.connection
 
       connection.stub(:extensions, ["hstore", "uuid-ossp", "xml2"]) do
-        output = perform_schema_dump
+        output = dump_all_table_schema
         enabled_extensions = output.scan(%r{enable_extension "(.+)"}).flatten
         assert_equal ["hstore", "uuid-ossp", "xml2"], enabled_extensions
       end
 
       connection.stub(:extensions, ["uuid-ossp", "xml2", "hstore"]) do
-        output = perform_schema_dump
+        output = dump_all_table_schema
         enabled_extensions = output.scan(%r{enable_extension "(.+)"}).flatten
         assert_equal ["hstore", "uuid-ossp", "xml2"], enabled_extensions
       end
@@ -427,7 +423,7 @@ def test_do_not_dump_foreign_keys_when_bypassed_by_config
           }
         )
 
-        output = perform_schema_dump
+        output = dump_all_table_schema
         assert_no_match(/^\s+add_foreign_key "fk_test_has_fk"[^\n]+\n\s+add_foreign_key "lessons_students"/, output)
       ensure
         ActiveRecord::Base.establish_connection(:arunit)
@@ -461,7 +457,7 @@ def test_schema_dump_with_table_name_prefix_and_suffix
     migration = CreateDogMigration.new
     migration.migrate(:up)
 
-    output = perform_schema_dump
+    output = dump_all_table_schema
     assert_no_match %r{create_table "foo_.+_bar"}, output
     assert_no_match %r{add_index "foo_.+_bar"}, output
     assert_no_match %r{create_table "schema_migrations"}, output
@@ -486,7 +482,7 @@ def test_schema_dump_with_table_name_prefix_and_suffix_regexp_escape
     migration = CreateDogMigration.new
     migration.migrate(:up)
 
-    output = perform_schema_dump
+    output = dump_all_table_schema
     assert_no_match %r{create_table "foo\$.+\$bar"}, output
     assert_no_match %r{add_index "foo\$.+\$bar"}, output
     assert_no_match %r{create_table "schema_migrations"}, output
@@ -554,7 +550,7 @@ def down
       end
       migration.migrate(:up)
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.datetime "this_should_remain_datetime"')
       assert output.include?('t.datetime "this_is_an_alias_of_datetime"')
       assert output.include?('t.datetime "without_time_zone"')
@@ -583,7 +579,7 @@ def down
         end
         migration.migrate(:up)
 
-        output = perform_schema_dump
+        output = dump_all_table_schema
         assert output.include?('t.datetime "this_should_remain_datetime"')
         assert output.include?('t.datetime "this_is_an_alias_of_datetime"')
         assert output.include?('t.timestamp "without_time_zone"')
@@ -611,7 +607,7 @@ def down
       end
       migration.migrate(:up)
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.datetime "this_should_remain_datetime"')
       assert output.include?('t.datetime "this_is_an_alias_of_datetime"')
       assert output.include?('t.datetime "this_is_also_an_alias_of_datetime"')
@@ -638,7 +634,7 @@ def down
         end
         migration.migrate(:up)
 
-        output = perform_schema_dump
+        output = dump_all_table_schema
         # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type`
         # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns
         # are still created as a `:timestamp` we need to change what is written to the schema dump.
@@ -672,15 +668,15 @@ def down
       end
       migration.migrate(:up)
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.datetime "default_format"')
       assert output.include?('t.datetime "without_time_zone"')
       assert output.include?('t.timestamptz "with_time_zone"')
 
       datetime_type_was = ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type
       ActiveRecord::ConnectionAdapters::PostgreSQLAdapter.datetime_type = :timestamptz
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.timestamp "default_format"')
       assert output.include?('t.timestamp "without_time_zone"')
       assert output.include?('t.datetime "with_time_zone"')
@@ -708,7 +704,7 @@ def down
       end
       migration.migrate(:up)
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.datetime "default_format"')
       assert output.include?('t.datetime "without_time_zone"')
       assert output.include?('t.datetime "also_without_time_zone"')
@@ -736,7 +732,7 @@ def down
       end
       migration.migrate(:up)
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.datetime "default_format"')
       assert output.include?('t.datetime "without_time_zone"')
       assert output.include?('t.datetime "also_without_time_zone"')
@@ -763,7 +759,7 @@ def down
       end
       migration.migrate(:up)
 
-      output = perform_schema_dump
+      output = dump_all_table_schema
       assert output.include?('t.datetime "default_format"')
       assert output.include?('t.datetime "without_time_zone"')
       assert output.include?('t.datetime "also_without_time_zone"')
@@ -789,7 +785,7 @@ def down
         end
         migration.migrate(:up)
 
-        output = perform_schema_dump
+        output = dump_all_table_schema
         # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type`
         # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns
         # are still created as a `:timestamp` we need to change what is written to the schema dump.
@@ -822,7 +818,7 @@ def down
         end
         migration.migrate(:up)
 
-        output = perform_schema_dump
+        output = dump_all_table_schema
         # Normally we'd write `t.datetime` here. But because you've changed the `datetime_type`
         # to something else, `t.datetime` now means `:timestamptz`. To ensure that old columns
         # are still created as a `:timestamp` we need to change what is written to the schema dump.

activerecord/test/support/schema_dumping_helper.rb

@@ -11,7 +11,7 @@ def dump_table_schema(table, connection = ActiveRecord::Base.connection)
     ActiveRecord::SchemaDumper.ignore_tables = old_ignore_tables
   end
 
-  def dump_all_table_schema(ignore_tables)
+  def dump_all_table_schema(ignore_tables = [])
     old_ignore_tables, ActiveRecord::SchemaDumper.ignore_tables = ActiveRecord::SchemaDumper.ignore_tables, ignore_tables
     stream = StringIO.new
     ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)