From 49f31043be571381fd66dbc22b123d5625af64de Mon Sep 17 00:00:00 2001
From: Guilherme Goettems Schneider <guigs81@gmail.com>
Date: Mon, 3 Jun 2019 08:05:36 -0300
Subject: [PATCH] Fix invalid schema dump when primary key column has a comment

Before this fix it would either generate an invalid schema, passing `comment` option twice to `create_table`, or it move the comment from primary key column to the table if table had no comment when the dump was generated.

The situation now is that a comment on primary key will be ignored (not present on schema).

Fixes #29966
---
 activerecord/CHANGELOG.md                     |  6 +++++
 .../abstract/schema_dumper.rb                 |  2 +-
 activerecord/test/cases/comment_test.rb       | 22 ++++++++++++++++++-
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 2b5fc1c2f8905..6f08b1b8fe4d0 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,9 @@
+*   Fix invalid schema when primary key column has a comment
+
+    Fixes #29966
+
+    *Guilherme Goettems Schneider*
+
 *   Fix table comment also being applied to the primary key column
 
     *Guilherme Goettems Schneider*
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 622e00fffba9b..fb56e712be64b 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_dumper.rb
@@ -15,7 +15,7 @@ def column_spec(column)
         def column_spec_for_primary_key(column)
           return {} if default_primary_key?(column)
           spec = { id: schema_type(column).inspect }
-          spec.merge!(prepare_column_options(column).except!(:null))
+          spec.merge!(prepare_column_options(column).except!(:null, :comment))
           spec[:default] ||= "nil" if explicit_primary_key_default?(column)
           spec
         end
diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb
index f2f28462d1ba1..25e2f20676438 100644
--- a/activerecord/test/cases/comment_test.rb
+++ b/activerecord/test/cases/comment_test.rb
@@ -14,6 +14,9 @@ class Commented < ActiveRecord::Base
     class BlankComment < ActiveRecord::Base
     end
 
+    class PkCommented < ActiveRecord::Base
+    end
+
     setup do
       @connection = ActiveRecord::Base.connection
 
@@ -35,8 +38,13 @@ class BlankComment < ActiveRecord::Base
         t.index :absent_comment
       end
 
+      @connection.create_table("pk_commenteds", comment: "Table comment", id: false, force: true) do |t|
+        t.integer :id, comment: "Primary key comment", primary_key: true
+      end
+
       Commented.reset_column_information
       BlankComment.reset_column_information
+      PkCommented.reset_column_information
     end
 
     teardown do
@@ -44,7 +52,7 @@ class BlankComment < ActiveRecord::Base
       @connection.drop_table "blank_comments", if_exists: true
     end
 
-    def test_primary_key_comment
+    def test_default_primary_key_comment
       column = Commented.columns_hash["id"]
       assert_nil column.comment
     end
@@ -169,5 +177,17 @@ def test_change_column_comment_to_nil
       column = Commented.columns_hash["name"]
       assert_nil column.comment
     end
+
+    def test_comment_on_primary_key
+      column = PkCommented.columns_hash["id"]
+      assert_equal "Primary key comment", column.comment
+      assert_equal "Table comment", @connection.table_comment("pk_commenteds")
+    end
+
+    def test_schema_dump_with_primary_key_comment
+      output = dump_table_schema "pk_commenteds"
+      assert_match %r[create_table "pk_commenteds",.*\s+comment: "Table comment"], output
+      assert_no_match %r[create_table "pk_commenteds",.*\s+comment: "Primary key comment"], output
+    end
   end
 end