Make pg adapter use bigserial for pk by default

11 files changed, 262 insertions, 3 deletions

diff --git a/.gitignore b/.gitignore
index d48828ea03..9268977c2f 100644
--- a/.gitignore
+++ b/.gitignore
@@ -19,4 +19,3 @@ pkg
 /railties/doc
 /railties/tmp
 /guides/output
-/*/.byebug_history
\ No newline at end of file
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 96d0ad3b30..2182ee78bb 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,7 @@
+*   PostgreSQL & MySQL: Use big integer as primary key type for new tables
+
+    *Jon McCartie*, *Pavel Pravosud*
+
 *   Change the type argument of `ActiveRecord::Base#attribute` to be optional.
     The default is now `ActiveRecord::Type::Value.new`, which provides no type
     casting behavior.
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 6eb7e0ae52..971b274265 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -39,7 +39,7 @@ module ActiveRecord
       self.emulate_booleans = true
 
       NATIVE_DATABASE_TYPES = {
-        primary_key: "BIGINT(8) UNSIGNED DEFAULT NULL auto_increment PRIMARY KEY",
+        primary_key: "BIGINT(8) UNSIGNED auto_increment PRIMARY KEY",
         string:      { name: "varchar", limit: 255 },
         text:        { name: "text", limit: 65535 },
         integer:     { name: "int", limit: 4 },
@@ -736,6 +736,8 @@ module ActiveRecord
         ER_NO_REFERENCED_ROW_2  = 1452
         ER_DATA_TOO_LONG        = 1406
         ER_LOCK_DEADLOCK        = 1213
+        ER_CANNOT_ADD_FOREIGN   = 1215
+        ER_CANNOT_CREATE_TABLE  = 1005
 
         def translate_exception(exception, message)
           case error_number(exception)
@@ -743,6 +745,14 @@ module ActiveRecord
             RecordNotUnique.new(message)
           when ER_NO_REFERENCED_ROW_2
             InvalidForeignKey.new(message)
+          when ER_CANNOT_ADD_FOREIGN
+            mismatched_foreign_key(message)
+          when ER_CANNOT_CREATE_TABLE
+            if message.include?("errno: 150")
+              mismatched_foreign_key(message)
+            else
+              super
+            end
           when ER_DATA_TOO_LONG
             ValueTooLong.new(message)
           when ER_LOCK_DEADLOCK
@@ -914,6 +924,18 @@ module ActiveRecord
           MySQL::TableDefinition.new(*args)
         end
 
+        def mismatched_foreign_key(message)
+          parts = message.scan(/`(\w+)`[ $)]/).flatten
+          MismatchedForeignKey.new(
+            self,
+            message: message,
+            table: parts[0],
+            foreign_key: parts[1],
+            target_table: parts[2],
+            primary_key: parts[3],
+          )
+        end
+
         def extract_schema_qualified_name(string) # :nodoc:
           schema, name = string.to_s.scan(/[^`.\s]+|`[^`]*`/)
           schema, name = @config[:database], schema unless name
diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb
index 6464d40c94..7c8be37326 100644
--- a/activerecord/lib/active_record/errors.rb
+++ b/activerecord/lib/active_record/errors.rb
@@ -123,6 +123,34 @@ module ActiveRecord
   class InvalidForeignKey < WrappedDatabaseException
   end
 
+  # Raised when a foreign key constraint cannot be added because the column type does not match the referenced column type.
+  class MismatchedForeignKey < WrappedDatabaseException
+    def initialize(adapter = nil, message: nil, table: nil, foreign_key: nil, target_table: nil, primary_key: nil)
+      @adapter = adapter
+      if table
+        msg = <<-EOM.strip_heredoc
+          Column `#{foreign_key}` on table `#{table}` has a type of `#{column_type(table, foreign_key)}`.
+          This does not match column `#{primary_key}` on `#{target_table}`, which has type `#{column_type(target_table, primary_key)}`.
+          To resolve this issue, change the type of the `#{foreign_key}` column on `#{table}` to be :integer. (For example `t.integer #{foreign_key}`).
+        EOM
+      else
+        msg = <<-EOM
+          There is a mismatch between the foreign key and primary key column types.
+          Verify that the foreign key column type and the primary key of the associated table match types.
+        EOM
+      end
+      if message
+        msg << "\nOriginal message: #{message}"
+      end
+      super(msg)
+    end
+
+    private
+      def column_type(table, column)
+        @adapter.columns(table).detect { |c| c.name == column }.sql_type
+      end
+  end
+
   # Raised when a record cannot be inserted or updated because a value too long for a column type.
   class ValueTooLong < StatementInvalid
   end
diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb
index ae45ac7157..9d3652d63b 100644
--- a/activerecord/lib/active_record/migration/compatibility.rb
+++ b/activerecord/lib/active_record/migration/compatibility.rb
@@ -104,11 +104,21 @@ module ActiveRecord
 
       class V5_0 < V5_1
         def create_table(table_name, options = {})
-          if ActiveRecord::Base.connection.adapter_name == "PostgreSQL"
+          connection_name = self.connection.adapter_name
+          if connection_name == "PostgreSQL"
             if options[:id] == :uuid && !options[:default]
               options[:default] = "uuid_generate_v4()"
             end
           end
+
+          # Since 5.1 Postgres adapter uses bigserial type for primary
+          # keys by default and MySQL uses bigint. This compat layer makes old migrations utilize
+          # serial/int type instead -- the way it used to work before 5.1.
+          if options[:id].blank?
+            options[:id] = :integer
+            options[:auto_increment] = true
+          end
+
           super
         end
       end
diff --git a/activerecord/test/cases/adapters/mysql2/legacy_migration_test.rb b/activerecord/test/cases/adapters/mysql2/legacy_migration_test.rb
new file mode 100644
index 0000000000..5d3125c2be
--- /dev/null
+++ b/activerecord/test/cases/adapters/mysql2/legacy_migration_test.rb
@@ -0,0 +1,60 @@
+require "cases/helper"
+
+class MysqlLegacyMigrationTest < ActiveRecord::Mysql2TestCase
+  self.use_transactional_tests = false
+
+  class GenerateTableWithoutBigint < ActiveRecord::Migration[5.0]
+    def change
+      create_table :legacy_integer_pk do |table|
+        table.string :foo
+      end
+
+      create_table :override_pk, id: :bigint do |table|
+        table.string :bar
+      end
+    end
+  end
+
+  def setup
+    super
+    @connection = ActiveRecord::Base.connection
+
+    @migration_verbose_old = ActiveRecord::Migration.verbose
+    ActiveRecord::Migration.verbose = false
+
+    migrations = [GenerateTableWithoutBigint.new(nil, 1)]
+
+    ActiveRecord::Migrator.new(:up, migrations).migrate
+  end
+
+  def teardown
+    ActiveRecord::Migration.verbose = @migration_verbose_old
+    @connection.drop_table("legacy_integer_pk")
+    @connection.drop_table("override_pk")
+    ActiveRecord::SchemaMigration.delete_all rescue nil
+    super
+  end
+
+  def test_create_table_uses_integer_as_pkey_by_default
+    col = column(:legacy_integer_pk, :id)
+    assert_equal "int(11)", sql_type_for(col)
+    assert col.auto_increment?
+  end
+
+  def test_create_tables_respects_pk_column_type_override
+    col = column(:override_pk, :id)
+    assert_equal "bigint(20)", sql_type_for(col)
+  end
+
+  private
+
+    def column(table_name, column_name)
+      ActiveRecord::Base.connection
+        .columns(table_name.to_s)
+        .detect { |c| c.name == column_name.to_s }
+    end
+
+    def sql_type_for(col)
+      col && col.sql_type
+    end
+end
diff --git a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
index 69336eb906..aab3dcb724 100644
--- a/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb
@@ -65,6 +65,19 @@ class Mysql2AdapterTest < ActiveRecord::Mysql2TestCase
       @conn.columns_for_distinct("posts.id", [order])
   end
 
+  def test_errors_for_bigint_fks_on_integer_pk_table
+    # table old_cars has primary key of integer
+
+    error = assert_raises(ActiveRecord::MismatchedForeignKey) do
+      @conn.add_reference :engines, :old_car
+      @conn.add_foreign_key :engines, :old_cars
+    end
+
+    assert_match "Column `old_car_id` on table `engines` has a type of `bigint(20)`", error.message
+    assert_not_nil error.cause
+    @conn.exec_query("ALTER TABLE engines DROP COLUMN old_car_id")
+  end
+
   private
 
     def with_example_table(definition = "id int auto_increment primary key, number int, data varchar(255)", &block)
diff --git a/activerecord/test/cases/adapters/postgresql/legacy_migration_test.rb b/activerecord/test/cases/adapters/postgresql/legacy_migration_test.rb
new file mode 100644
index 0000000000..082fe95053
--- /dev/null
+++ b/activerecord/test/cases/adapters/postgresql/legacy_migration_test.rb
@@ -0,0 +1,54 @@
+require "cases/helper"
+
+class PostgresqlLegacyMigrationTest < ActiveRecord::PostgreSQLTestCase
+  class GenerateTableWithoutBigserial < ActiveRecord::Migration[5.0]
+    def change
+      create_table :legacy_integer_pk do |table|
+        table.string :foo
+      end
+
+      create_table :override_pk, id: :bigint do |table|
+        table.string :bar
+      end
+    end
+  end
+
+  def setup
+    super
+
+    @migration_verbose_old = ActiveRecord::Migration.verbose
+    ActiveRecord::Migration.verbose = false
+
+    migrations = [GenerateTableWithoutBigserial.new(nil, 1)]
+    ActiveRecord::Migrator.new(:up, migrations).migrate
+  end
+
+  def teardown
+    ActiveRecord::Migration.verbose = @migration_verbose_old
+
+    super
+  end
+
+  def test_create_table_uses_serial_as_pkey_by_default
+    col = column(:legacy_integer_pk, :id)
+    assert_equal "integer", sql_type_for(col)
+    assert col.serial?
+  end
+
+  def test_create_tables_respects_pk_column_type_override
+    col = column(:override_pk, :id)
+    assert_equal "bigint", sql_type_for(col)
+  end
+
+  private
+
+    def column(table_name, column_name)
+      ActiveRecord::Base.connection.
+        columns(table_name.to_s).
+        detect { |c| c.name == column_name.to_s }
+    end
+
+    def sql_type_for(col)
+      col && col.sql_type
+    end
+end
diff --git a/activerecord/test/cases/adapters/sqlite3/legacy_migration_test.rb b/activerecord/test/cases/adapters/sqlite3/legacy_migration_test.rb
new file mode 100644
index 0000000000..fcca8d66b5
--- /dev/null
+++ b/activerecord/test/cases/adapters/sqlite3/legacy_migration_test.rb
@@ -0,0 +1,59 @@
+require "cases/helper"
+
+class SqliteLegacyMigrationTest < ActiveRecord::SQLite3TestCase
+  self.use_transactional_tests = false
+
+  class GenerateTableWithoutBigint < ActiveRecord::Migration[5.0]
+    def change
+      create_table :legacy_integer_pk do |table|
+        table.string :foo
+      end
+
+      create_table :override_pk, id: :bigint do |table|
+        table.string :bar
+      end
+    end
+  end
+
+  def setup
+    super
+    @connection = ActiveRecord::Base.connection
+
+    @migration_verbose_old = ActiveRecord::Migration.verbose
+    ActiveRecord::Migration.verbose = false
+
+    migrations = [GenerateTableWithoutBigint.new(nil, 1)]
+
+    ActiveRecord::Migrator.new(:up, migrations).migrate
+  end
+
+  def teardown
+    ActiveRecord::Migration.verbose = @migration_verbose_old
+    @connection.drop_table("legacy_integer_pk")
+    @connection.drop_table("override_pk")
+    ActiveRecord::SchemaMigration.delete_all rescue nil
+    super
+  end
+
+  def test_create_table_uses_integer_as_pkey_by_default
+    col = column(:legacy_integer_pk, :id)
+    assert_equal "INTEGER", sql_type_for(col)
+    assert primary_key?(:legacy_integer_pk, "id"), "id is not primary key"
+  end
+
+  private
+
+    def column(table_name, column_name)
+      ActiveRecord::Base.connection
+        .columns(table_name.to_s)
+        .detect { |c| c.name == column_name.to_s }
+    end
+
+    def sql_type_for(col)
+      col && col.sql_type
+    end
+
+    def primary_key?(table_name, column)
+      ActiveRecord::Base.connection.execute("PRAGMA table_info(#{table_name})").find { |col| col["name"] == column }["pk"] == 1
+    end
+end
diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb
index 2af2a2df89..30c161cd99 100644
--- a/activerecord/test/cases/primary_keys_test.rb
+++ b/activerecord/test/cases/primary_keys_test.rb
@@ -361,6 +361,13 @@ class PrimaryKeyIntegerTest < ActiveRecord::TestCase
     Widget.reset_column_information
   end
 
+  if current_adapter?(:PostgreSQLAdapter, :Mysql2Adapter)
+    test "schema dump primary key with bigserial" do
+      schema = dump_table_schema "widgets"
+      assert_match %r{create_table "widgets", force: :cascade}, schema
+    end
+  end
+
   test "primary key column type" do
     column_type = Widget.type_for_attribute(Widget.primary_key)
     assert_equal :integer, column_type.type
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index dcf29d36f4..658591b6ec 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -126,6 +126,9 @@ ActiveRecord::Schema.define do
     t.timestamps null: false
   end
 
+  create_table :old_cars, id: :integer, force: true do |t|
+  end
+
   create_table :carriers, force: true
 
   create_table :categories, force: true do |t|