Merge pull request #19185 from kamipo/correctly_dump_serial_and_bigserial

Correctly dump `serial` and `bigserial`

8 files changed, 100 insertions, 27 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 824484e065..0f99f907e8 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   Correctly dump `serial` and `bigserial`.
+
+    *Ryuta Kamizono*
+
 *   Fix default `format` value in `ActiveRecord::Tasks::DatabaseTasks#schema_file`.
 
     *James Cox*
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 af7ef7cbaa..999cb0ec5a 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
@@ -24,7 +24,7 @@ module ActiveRecord
       def prepare_column_options(column)
         spec = {}
         spec[:name]      = column.name.inspect
-        spec[:type]      = column.type.to_s
+        spec[:type]      = schema_type(column)
         spec[:null]      = 'false' unless column.null
 
         limit = column.limit || native_database_types[column.type][:limit]
@@ -45,6 +45,10 @@ module ActiveRecord
 
       private
 
+      def schema_type(column)
+        column.type.to_s
+      end
+
       def schema_default(column)
         type = lookup_cast_type_from_column(column)
         default = type.deserialize(column.default)
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index c084431588..9625fcf9b8 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -86,8 +86,8 @@ module ActiveRecord
       def column_spec_for_primary_key(column)
         spec = {}
         if column.auto_increment?
-          return unless column.limit == 8
-          spec[:id] = ':bigint'
+          spec[:id] = ':bigint' if column.bigint?
+          return if spec.empty?
         else
           spec[:id] = column.type.inspect
           spec.merge!(prepare_column_options(column).delete_if { |key, _| [:name, :type, :null].include?(key) })
diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb
index fa5ed07b8a..a67127bd71 100644
--- a/activerecord/lib/active_record/connection_adapters/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/column.rb
@@ -31,7 +31,11 @@ module ActiveRecord
       end
 
       def has_default?
-        !default.nil?
+        !default.nil? || default_function
+      end
+
+      def bigint?
+        /bigint/ === sql_type
       end
 
       # Returns the human name of the column name.
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb
index 0eb4fb468c..be13ead120 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb
@@ -6,7 +6,9 @@ module ActiveRecord
       alias :array? :array
 
       def serial?
-        default_function && default_function =~ /\Anextval\(.*\)\z/
+        return unless default_function
+
+        %r{\Anextval\('(?<table_name>.+)_#{name}_seq'::regclass\)\z} === default_function
       end
     end
   end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 6d25b53b21..92f470ae70 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -128,7 +128,7 @@ module ActiveRecord
       def column_spec_for_primary_key(column)
         spec = {}
         if column.serial?
-          return unless column.sql_type == 'bigint'
+          return unless column.bigint?
           spec[:id] = ':bigserial'
         elsif column.type == :uuid
           spec[:id] = ':uuid'
@@ -145,7 +145,6 @@ module ActiveRecord
       def prepare_column_options(column) # :nodoc:
         spec = super
         spec[:array] = 'true' if column.array?
-        spec[:default] = "\"#{column.default_function}\"" if column.default_function
         spec
       end
 
@@ -154,6 +153,26 @@ module ActiveRecord
         super + [:array]
       end
 
+      def schema_type(column)
+        return super unless column.serial?
+
+        if column.bigint?
+          'bigserial'
+        else
+          'serial'
+        end
+      end
+      private :schema_type
+
+      def schema_default(column)
+        if column.default_function
+          column.default_function.inspect unless column.serial?
+        else
+          super
+        end
+      end
+      private :schema_default
+
       # Returns +true+, since this connection adapter supports prepared statement
       # caching.
       def supports_statement_cache?
diff --git a/activerecord/test/cases/adapters/postgresql/serial_test.rb b/activerecord/test/cases/adapters/postgresql/serial_test.rb
new file mode 100644
index 0000000000..458a8dae6c
--- /dev/null
+++ b/activerecord/test/cases/adapters/postgresql/serial_test.rb
@@ -0,0 +1,60 @@
+require "cases/helper"
+require 'support/schema_dumping_helper'
+
+class PostgresqlSerialTest < ActiveRecord::TestCase
+  include SchemaDumpingHelper
+
+  class PostgresqlSerial < ActiveRecord::Base; end
+
+  setup do
+    @connection = ActiveRecord::Base.connection
+    @connection.create_table "postgresql_serials", force: true do |t|
+      t.serial :seq
+    end
+  end
+
+  teardown do
+    @connection.drop_table "postgresql_serials", if_exists: true
+  end
+
+  def test_serial_column
+    column = PostgresqlSerial.columns_hash["seq"]
+    assert_equal :integer, column.type
+    assert_equal "integer", column.sql_type
+    assert column.serial?
+  end
+
+  def test_schema_dump_with_shorthand
+    output = dump_table_schema "postgresql_serials"
+    assert_match %r{t\.serial\s+"seq"}, output
+  end
+end
+
+class PostgresqlBigSerialTest < ActiveRecord::TestCase
+  include SchemaDumpingHelper
+
+  class PostgresqlBigSerial < ActiveRecord::Base; end
+
+  setup do
+    @connection = ActiveRecord::Base.connection
+    @connection.create_table "postgresql_big_serials", force: true do |t|
+      t.bigserial :seq
+    end
+  end
+
+  teardown do
+    @connection.drop_table "postgresql_big_serials", if_exists: true
+  end
+
+  def test_bigserial_column
+    column = PostgresqlBigSerial.columns_hash["seq"]
+    assert_equal :integer, column.type
+    assert_equal "bigint", column.sql_type
+    assert column.serial?
+  end
+
+  def test_schema_dump_with_shorthand
+    output = dump_table_schema "postgresql_big_serials"
+    assert_match %r{t\.bigserial\s+"seq"}, output
+  end
+end
diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb
index 1219e197ab..e9379a1019 100644
--- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb
+++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb
@@ -135,26 +135,6 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase
   end
 end
 
-class PostgresqlLargeKeysTest < ActiveRecord::TestCase
-  include PostgresqlUUIDHelper
-  include SchemaDumpingHelper
-
-  def setup
-    connection.create_table('big_serials', id: :bigserial) do |t|
-      t.string 'name'
-    end
-  end
-
-  def test_omg
-    schema = dump_table_schema "big_serials"
-    assert_match "create_table \"big_serials\", id: :bigserial", schema
-  end
-
-  def teardown
-    drop_table "big_serials"
-  end
-end
-
 class PostgresqlUUIDGenerationTest < ActiveRecord::TestCase
   include PostgresqlUUIDHelper
   include SchemaDumpingHelper