Merge pull request #24054 from kamipo/extract_default_primary_key

Extract `default_primary_key?` to refactor `column_spec_for_primary_key`

5 files changed, 22 insertions, 22 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
index 9e653cd7c4..4880d216d6 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
@@ -14,7 +14,7 @@ module ActiveRecord
       end
 
       def column_spec_for_primary_key(column)
-        return if column.type == :integer
+        return {} if default_primary_key?(column)
         spec = { id: schema_type(column).inspect }
         spec.merge!(prepare_column_options(column))
       end
@@ -56,6 +56,10 @@ module ActiveRecord
 
       private
 
+      def default_primary_key?(column)
+        schema_type(column) == :integer
+      end
+
       def schema_type(column)
         if column.bigint?
           :bigint
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb
index 90e5420e71..be40df4101 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_dumper.rb
@@ -3,18 +3,13 @@ module ActiveRecord
     module MySQL
       module ColumnDumper
         def column_spec_for_primary_key(column)
-          spec = {}
           if column.bigint?
-            spec[:id] = ':bigint'
+            spec = { id: :bigint.inspect }
             spec[:default] = schema_default(column) || 'nil' unless column.auto_increment?
-            spec[:unsigned] = 'true' if column.unsigned?
-          elsif column.auto_increment?
-            spec[:unsigned] = 'true' if column.unsigned?
-            return if spec.empty?
           else
-            spec[:id] = schema_type(column).inspect
-            spec.merge!(prepare_column_options(column).delete_if { |key, _| key == :null })
+            spec = super.except!(:null)
           end
+          spec[:unsigned] = 'true' if column.unsigned?
           spec
         end
 
@@ -30,6 +25,10 @@ module ActiveRecord
 
         private
 
+        def default_primary_key?(column)
+          super && column.auto_increment?
+        end
+
         def schema_type(column)
           if column.sql_type == 'tinyblob'
             :blob
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb
index 19761618cf..1047ba8cac 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_dumper.rb
@@ -3,16 +3,9 @@ module ActiveRecord
     module PostgreSQL
       module ColumnDumper
         def column_spec_for_primary_key(column)
-          spec = {}
-          if column.serial?
-            return unless column.bigint?
-            spec[:id] = ':bigserial'
-          elsif column.type == :uuid
-            spec[:id] = ':uuid'
-            spec[:default] = schema_default(column) || 'nil'
-          else
-            spec[:id] = schema_type(column).inspect
-            spec.merge!(prepare_column_options(column).delete_if { |key, _| key == :null })
+          spec = super.except!(:null)
+          if schema_type(column) == :uuid
+            spec[:default] ||= 'nil'
           end
           spec
         end
@@ -31,6 +24,10 @@ module ActiveRecord
 
         private
 
+        def default_primary_key?(column)
+          schema_type(column) == :serial
+        end
+
         def schema_type(column)
           return super unless column.serial?
 
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index 8c76c5f62d..affcd9aed1 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -123,7 +123,7 @@ HEADER
             tbl.print ", primary_key: #{pk.inspect}" unless pk == 'id'
             pkcol = columns.detect { |c| c.name == pk }
             pkcolspec = @connection.column_spec_for_primary_key(pkcol)
-            if pkcolspec
+            if pkcolspec.present?
               pkcolspec.each do |key, value|
                 tbl.print ", #{key}: #{value}"
               end
diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb
index 6aaee8de7c..32bccce2ed 100644
--- a/activerecord/test/cases/primary_keys_test.rb
+++ b/activerecord/test/cases/primary_keys_test.rb
@@ -351,9 +351,9 @@ if current_adapter?(:PostgreSQLAdapter, :Mysql2Adapter)
     test "schema dump primary key with bigserial" do
       schema = dump_table_schema "widgets"
       if current_adapter?(:PostgreSQLAdapter)
-        assert_match %r{create_table "widgets", id: :bigserial}, schema
+        assert_match %r{create_table "widgets", id: :bigserial, force: :cascade}, schema
       else
-        assert_match %r{create_table "widgets", id: :bigint}, schema
+        assert_match %r{create_table "widgets", id: :bigint, force: :cascade}, schema
       end
     end