Merge pull request #35299 from kamipo/fix_mismatched_foreign_key

Fix the regex that extract mismatched foreign key information

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -795,17 +795,27 @@ def arel_visitor
         end
 
         def mismatched_foreign_key(message, sql:, binds:)
-          parts = sql.scan(/`(\w+)`[ $)]/).flatten
-          MismatchedForeignKey.new(
-            self,
+          match = %r/
+            (?:CREATE|ALTER)\s+TABLE\s*(?:`?\w+`?\.)?`?(?<table>\w+)`?.+?
+            FOREIGN\s+KEY\s*\(`?(?<foreign_key>\w+)`?\)\s*
+            REFERENCES\s*(`?(?<target_table>\w+)`?)\s*\(`?(?<primary_key>\w+)`?\)
+          /xmi.match(sql)
+
+          options = {
             message: message,
             sql: sql,
             binds: binds,
-            table: parts[0],
-            foreign_key: parts[1],
-            target_table: parts[2],
-            primary_key: parts[3],
-          )
+          }
+
+          if match
+            options[:table] = match[:table]
+            options[:foreign_key] = match[:foreign_key]
+            options[:target_table] = match[:target_table]
+            options[:primary_key] = match[:primary_key]
+            options[:primary_key_column] = column_for(match[:target_table], match[:primary_key])
+          end
+
+          MismatchedForeignKey.new(options)
         end
 
         def integer_to_sql(limit) # :nodoc:

activerecord/lib/active_record/errors.rb

@@ -126,16 +126,26 @@ class InvalidForeignKey < WrappedDatabaseException
 
   # Raised when a foreign key constraint cannot be added because the column type does not match the referenced column type.
   class MismatchedForeignKey < StatementInvalid
-    def initialize(adapter = nil, message: nil, sql: nil, binds: nil, table: nil, foreign_key: nil, target_table: nil, primary_key: nil)
-      @adapter = adapter
+    def initialize(
+      message: nil,
+      sql: nil,
+      binds: nil,
+      table: nil,
+      foreign_key: nil,
+      target_table: nil,
+      primary_key: nil,
+      primary_key_column: nil
+    )
       if table
-        msg = +<<~EOM
-          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}`).
+        type = primary_key_column.bigint? ? :bigint : primary_key_column.type
+        msg = <<~EOM.squish
+          Column `#{foreign_key}` on table `#{table}` does not match column `#{primary_key}` on `#{target_table}`,
+          which has type `#{primary_key_column.sql_type}`.
+          To resolve this issue, change the type of the `#{foreign_key}` column on `#{table}` to be :#{type}.
+          (For example `t.#{type} :#{foreign_key}`).
         EOM
       else
-        msg = +<<~EOM
+        msg = <<~EOM.squish
           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
@@ -145,11 +155,6 @@ def initialize(adapter = nil, message: nil, sql: nil, binds: nil, table: nil, fo
       end
       super(msg, sql: sql, binds: binds)
     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 it would violate a not null constraint.

activerecord/test/cases/adapters/mysql2/mysql2_adapter_test.rb

@@ -56,7 +56,7 @@ def order.to_sql
       @conn.columns_for_distinct("posts.id", [order])
   end
 
-  def test_errors_for_bigint_fks_on_integer_pk_table
+  def test_errors_for_bigint_fks_on_integer_pk_table_in_alter_table
     # table old_cars has primary key of integer
 
     error = assert_raises(ActiveRecord::MismatchedForeignKey) do
@@ -64,9 +64,86 @@ def test_errors_for_bigint_fks_on_integer_pk_table
       @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_includes error.message, <<~MSG.squish
+      Column `old_car_id` on table `engines` does not match column `id` on `old_cars`,
+      which has type `int(11)`. To resolve this issue, change the type of the `old_car_id`
+      column on `engines` to be :integer. (For example `t.integer :old_car_id`).
+    MSG
     assert_not_nil error.cause
-    @conn.exec_query("ALTER TABLE engines DROP COLUMN old_car_id")
+  ensure
+    @conn.execute("ALTER TABLE engines DROP COLUMN old_car_id") rescue nil
+  end
+
+  def test_errors_for_bigint_fks_on_integer_pk_table_in_create_table
+    # table old_cars has primary key of integer
+
+    error = assert_raises(ActiveRecord::MismatchedForeignKey) do
+      @conn.execute(<<~SQL)
+        CREATE TABLE activerecord_unittest.foos (
+          id bigint NOT NULL AUTO_INCREMENT PRIMARY KEY,
+          old_car_id bigint,
+          INDEX index_foos_on_old_car_id (old_car_id),
+          CONSTRAINT fk_rails_ff771f3c96 FOREIGN KEY (old_car_id) REFERENCES old_cars (id)
+        )
+      SQL
+    end
+
+    assert_includes error.message, <<~MSG.squish
+      Column `old_car_id` on table `foos` does not match column `id` on `old_cars`,
+      which has type `int(11)`. To resolve this issue, change the type of the `old_car_id`
+      column on `foos` to be :integer. (For example `t.integer :old_car_id`).
+    MSG
+    assert_not_nil error.cause
+  ensure
+    @conn.drop_table :foos, if_exists: true
+  end
+
+  def test_errors_for_integer_fks_on_bigint_pk_table_in_create_table
+    # table old_cars has primary key of bigint
+
+    error = assert_raises(ActiveRecord::MismatchedForeignKey) do
+      @conn.execute(<<~SQL)
+        CREATE TABLE activerecord_unittest.foos (
+          id bigint NOT NULL AUTO_INCREMENT PRIMARY KEY,
+          car_id int,
+          INDEX index_foos_on_car_id (car_id),
+          CONSTRAINT fk_rails_ff771f3c96 FOREIGN KEY (car_id) REFERENCES cars (id)
+        )
+      SQL
+    end
+
+    assert_includes error.message, <<~MSG.squish
+      Column `car_id` on table `foos` does not match column `id` on `cars`,
+      which has type `bigint(20)`. To resolve this issue, change the type of the `car_id`
+      column on `foos` to be :bigint. (For example `t.bigint :car_id`).
+    MSG
+    assert_not_nil error.cause
+  ensure
+    @conn.drop_table :foos, if_exists: true
+  end
+
+  def test_errors_for_bigint_fks_on_string_pk_table_in_create_table
+    # table old_cars has primary key of string
+
+    error = assert_raises(ActiveRecord::MismatchedForeignKey) do
+      @conn.execute(<<~SQL)
+        CREATE TABLE activerecord_unittest.foos (
+          id bigint NOT NULL AUTO_INCREMENT PRIMARY KEY,
+          subscriber_id bigint,
+          INDEX index_foos_on_subscriber_id (subscriber_id),
+          CONSTRAINT fk_rails_ff771f3c96 FOREIGN KEY (subscriber_id) REFERENCES subscribers (nick)
+        )
+      SQL
+    end
+
+    assert_includes error.message, <<~MSG.squish
+      Column `subscriber_id` on table `foos` does not match column `nick` on `subscribers`,
+      which has type `varchar(255)`. To resolve this issue, change the type of the `subscriber_id`
+      column on `foos` to be :string. (For example `t.string :subscriber_id`).
+    MSG
+    assert_not_nil error.cause
+  ensure
+    @conn.drop_table :foos, if_exists: true
   end
 
   def test_errors_when_an_insert_query_is_called_while_preventing_writes