Merge pull request #2 from rails/master

merge master

16 files changed, 133 insertions, 75 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 8c5342edb2..6f08b1b8fe 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,13 @@
+*   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*
+
 *   Allow generated `create_table` migrations to include or skip timestamps.
 
      *Michael Duchemin*
diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb
index 220043c061..fd32eaaf3a 100644
--- a/activerecord/lib/active_record/attribute_methods.rb
+++ b/activerecord/lib/active_record/attribute_methods.rb
@@ -185,12 +185,14 @@ module ActiveRecord
       /ix
 
       def disallow_raw_sql!(args, permit: COLUMN_NAME) # :nodoc:
-        unexpected = args.reject do |arg|
-          Arel.arel_node?(arg) ||
+        unexpected = nil
+        args.each do |arg|
+          next if arg.is_a?(Symbol) || Arel.arel_node?(arg) ||
             arg.to_s.split(/\s*,\s*/).all? { |part| permit.match?(part) }
+          (unexpected ||= []) << arg
         end
 
-        return if unexpected.none?
+        return unless unexpected
 
         if allow_unsafe_raw_sql == :deprecated
           ActiveSupport::Deprecation.warn(
@@ -437,7 +439,7 @@ module ActiveRecord
       def attributes_for_update(attribute_names)
         attribute_names &= self.class.column_names
         attribute_names.delete_if do |name|
-          readonly_attribute?(name)
+          self.class.readonly_attribute?(name)
         end
       end
 
@@ -460,10 +462,6 @@ module ActiveRecord
         end
       end
 
-      def readonly_attribute?(name)
-        self.class.readonly_attributes.include?(name)
-      end
-
       def pk_attribute?(name)
         name == @primary_key
       end
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
index 4ff3cb0071..7628ef5537 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb
@@ -3,6 +3,7 @@
 require "thread"
 require "concurrent/map"
 require "monitor"
+require "weakref"
 
 module ActiveRecord
   # Raised when a connection could not be obtained within the connection
@@ -294,28 +295,37 @@ module ActiveRecord
           @frequency = frequency
         end
 
-        @@mutex = Mutex.new
-        @@pools = {}
+        @mutex = Mutex.new
+        @pools = {}
 
-        def self.register_pool(pool, frequency) # :nodoc:
-          @@mutex.synchronize do
-            if @@pools.key?(frequency)
-              @@pools[frequency] << pool
-            else
-              @@pools[frequency] = [pool]
+        class << self
+          def register_pool(pool, frequency) # :nodoc:
+            @mutex.synchronize do
+              unless @pools.key?(frequency)
+                @pools[frequency] = []
+                spawn_thread(frequency)
+              end
+              @pools[frequency] << WeakRef.new(pool)
+            end
+          end
+
+          private
+
+            def spawn_thread(frequency)
               Thread.new(frequency) do |t|
                 loop do
                   sleep t
-                  @@mutex.synchronize do
-                    @@pools[frequency].each do |p|
+                  @mutex.synchronize do
+                    @pools[frequency].select!(&:weakref_alive?)
+                    @pools[frequency].each do |p|
                       p.reap
                       p.flush
+                    rescue WeakRef::RefError
                     end
                   end
                 end
               end
             end
-          end
         end
 
         def run
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 688eea75e8..dbd533b4b3 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -264,8 +264,7 @@ module ActiveRecord
         if_not_exists: false,
         options: nil,
         as: nil,
-        comment: nil,
-        **
+        comment: nil
       )
         @conn = conn
         @columns_hash = {}
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 622e00fffb..fb56e712be 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 @@ module ActiveRecord
         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/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index f97842b3f5..cf57af5473 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -291,25 +291,25 @@ module ActiveRecord
       #     SELECT * FROM orders INNER JOIN line_items ON order_id=orders.id
       #
       # See also TableDefinition#column for details on how to create columns.
-      def create_table(table_name, **options)
-        td = create_table_definition(table_name, options)
+      def create_table(table_name, id: :primary_key, primary_key: nil, force: nil, **options)
+        td = create_table_definition(
+          table_name, options.extract!(:temporary, :if_not_exists, :options, :as, :comment)
+        )
 
-        if options[:id] != false && !options[:as]
-          pk = options.fetch(:primary_key) do
-            Base.get_primary_key table_name.to_s.singularize
-          end
+        if id && !td.as
+          pk = primary_key || Base.get_primary_key(table_name.to_s.singularize)
 
           if pk.is_a?(Array)
             td.primary_keys pk
           else
-            td.primary_key pk, options.fetch(:id, :primary_key), options
+            td.primary_key pk, id, options
           end
         end
 
         yield td if block_given?
 
-        if options[:force]
-          drop_table(table_name, options.merge(if_exists: true))
+        if force
+          drop_table(table_name, force: force, if_exists: true)
         end
 
         result = execute schema_creation.accept td
@@ -321,7 +321,7 @@ module ActiveRecord
         end
 
         if supports_comments? && !supports_comments_in_create?
-          if table_comment = options[:comment].presence
+          if table_comment = td.comment.presence
             change_table_comment(table_name, table_comment)
           end
 
@@ -518,14 +518,15 @@ module ActiveRecord
       # Available options are (none of these exists by default):
       # * <tt>:limit</tt> -
       #   Requests a maximum column length. This is the number of characters for a <tt>:string</tt> column
-      #   and number of bytes for <tt>:text</tt>, <tt>:binary</tt> and <tt>:integer</tt> columns.
+      #   and number of bytes for <tt>:text</tt>, <tt>:binary</tt>, and <tt>:integer</tt> columns.
       #   This option is ignored by some backends.
       # * <tt>:default</tt> -
       #   The column's default value. Use +nil+ for +NULL+.
       # * <tt>:null</tt> -
       #   Allows or disallows +NULL+ values in the column.
       # * <tt>:precision</tt> -
-      #   Specifies the precision for the <tt>:decimal</tt> and <tt>:numeric</tt> columns.
+      #   Specifies the precision for the <tt>:decimal</tt>, <tt>:numeric</tt>,
+      #   <tt>:datetime</tt>, and <tt>:time</tt> columns.
       # * <tt>:scale</tt> -
       #   Specifies the scale for the <tt>:decimal</tt> and <tt>:numeric</tt> columns.
       # * <tt>:collation</tt> -
diff --git a/activerecord/lib/active_record/integration.rb b/activerecord/lib/active_record/integration.rb
index 573a823dbc..4a97061731 100644
--- a/activerecord/lib/active_record/integration.rb
+++ b/activerecord/lib/active_record/integration.rb
@@ -93,7 +93,7 @@ module ActiveRecord
     # cache_version, but this method can be overwritten to return something else.
     #
     # Note, this method will return nil if ActiveRecord::Base.cache_versioning is set to
-    # +false+ (which it is by default until Rails 6.0).
+    # +false+.
     def cache_version
       return unless cache_versioning
 
diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb
index a58c5fd48a..b7bc4b3b8e 100644
--- a/activerecord/lib/active_record/persistence.rb
+++ b/activerecord/lib/active_record/persistence.rb
@@ -939,7 +939,7 @@ module ActiveRecord
     end
 
     def verify_readonly_attribute(name)
-      raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attributes.include?(name)
+      raise ActiveRecordError, "#{name} is marked as readonly" if self.class.readonly_attribute?(name)
     end
 
     def _raise_record_not_destroyed
diff --git a/activerecord/lib/active_record/readonly_attributes.rb b/activerecord/lib/active_record/readonly_attributes.rb
index 7bc26993d5..c851ed52c3 100644
--- a/activerecord/lib/active_record/readonly_attributes.rb
+++ b/activerecord/lib/active_record/readonly_attributes.rb
@@ -19,6 +19,10 @@ module ActiveRecord
       def readonly_attributes
         _attr_readonly
       end
+
+      def readonly_attribute?(name) # :nodoc:
+        _attr_readonly.include?(name)
+      end
     end
   end
 end
diff --git a/activerecord/lib/active_record/timestamp.rb b/activerecord/lib/active_record/timestamp.rb
index 04a1c03474..a5862ae06b 100644
--- a/activerecord/lib/active_record/timestamp.rb
+++ b/activerecord/lib/active_record/timestamp.rb
@@ -59,19 +59,26 @@ module ActiveRecord
         attribute_names.index_with(time || current_time_from_proper_timezone)
       end
 
-      private
-        def timestamp_attributes_for_create_in_model
-          timestamp_attributes_for_create.select { |c| column_names.include?(c) }
-        end
+      def timestamp_attributes_for_create_in_model
+        @timestamp_attributes_for_create_in_model ||=
+          (timestamp_attributes_for_create & column_names).freeze
+      end
 
-        def timestamp_attributes_for_update_in_model
-          timestamp_attributes_for_update.select { |c| column_names.include?(c) }
-        end
+      def timestamp_attributes_for_update_in_model
+        @timestamp_attributes_for_update_in_model ||=
+          (timestamp_attributes_for_update & column_names).freeze
+      end
 
-        def all_timestamp_attributes_in_model
-          timestamp_attributes_for_create_in_model + timestamp_attributes_for_update_in_model
-        end
+      def all_timestamp_attributes_in_model
+        @all_timestamp_attributes_in_model ||=
+          (timestamp_attributes_for_create_in_model + timestamp_attributes_for_update_in_model).freeze
+      end
 
+      def current_time_from_proper_timezone
+        default_timezone == :utc ? Time.now.utc : Time.now
+      end
+
+      private
         def timestamp_attributes_for_create
           ["created_at", "created_on"]
         end
@@ -80,8 +87,11 @@ module ActiveRecord
           ["updated_at", "updated_on"]
         end
 
-        def current_time_from_proper_timezone
-          default_timezone == :utc ? Time.now.utc : Time.now
+        def reload_schema_from_cache
+          @timestamp_attributes_for_create_in_model = nil
+          @timestamp_attributes_for_update_in_model = nil
+          @all_timestamp_attributes_in_model = nil
+          super
         end
     end
 
@@ -124,19 +134,19 @@ module ActiveRecord
     end
 
     def timestamp_attributes_for_create_in_model
-      self.class.send(:timestamp_attributes_for_create_in_model)
+      self.class.timestamp_attributes_for_create_in_model
     end
 
     def timestamp_attributes_for_update_in_model
-      self.class.send(:timestamp_attributes_for_update_in_model)
+      self.class.timestamp_attributes_for_update_in_model
     end
 
     def all_timestamp_attributes_in_model
-      self.class.send(:all_timestamp_attributes_in_model)
+      self.class.all_timestamp_attributes_in_model
     end
 
     def current_time_from_proper_timezone
-      self.class.send(:current_time_from_proper_timezone)
+      self.class.current_time_from_proper_timezone
     end
 
     def max_updated_column_timestamp
diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index 49f754be63..cbe48a374f 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -37,8 +37,8 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
 
   def test_eager_association_loading_with_hmt_does_not_table_name_collide_when_joining_associations
     authors = Author.joins(:posts).eager_load(:comments).where(posts: { tags_count: 1 }).order(:id).to_a
-    assert_equal 3, assert_no_queries { authors.size }
-    assert_equal 10, assert_no_queries { authors[0].comments.size }
+    assert_equal 3, assert_queries(0) { authors.size }
+    assert_equal 10, assert_queries(0) { authors[0].comments.size }
   end
 
   def test_eager_association_loading_grafts_stashed_associations_to_correct_parent
@@ -103,14 +103,14 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     firms = Firm.all.merge!(includes: { account: { firm: :account } }, order: "companies.id").to_a
     assert_equal 2, firms.size
     assert_equal firms.first.account, firms.first.account.firm.account
-    assert_equal companies(:first_firm).account, assert_no_queries { firms.first.account.firm.account }
-    assert_equal companies(:first_firm).account.firm.account, assert_no_queries { firms.first.account.firm.account }
+    assert_equal companies(:first_firm).account, assert_queries(0) { firms.first.account.firm.account }
+    assert_equal companies(:first_firm).account.firm.account, assert_queries(0) { firms.first.account.firm.account }
   end
 
   def test_eager_association_loading_with_has_many_sti
     topics = Topic.all.merge!(includes: :replies, order: "topics.id").to_a
     first, second, = topics(:first).replies.size, topics(:second).replies.size
-    assert_no_queries do
+    assert_queries(0) do
       assert_equal first, topics[0].replies.size
       assert_equal second, topics[1].replies.size
     end
@@ -131,13 +131,13 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
     replies = Reply.all.merge!(includes: :topic, order: "topics.id").to_a
     assert_includes replies, topics(:second)
     assert_not_includes replies, topics(:first)
-    assert_equal topics(:first), assert_no_queries { replies.first.topic }
+    assert_equal topics(:first), assert_queries(0) { replies.first.topic }
   end
 
   def test_eager_association_loading_with_multiple_stis_and_order
     author = Author.all.merge!(includes: { posts: [ :special_comments, :very_special_comment ] }, order: ["authors.name", "comments.body", "very_special_comments_posts.body"], where: "posts.id = 4").first
     assert_equal authors(:david), author
-    assert_no_queries do
+    assert_queries(0) do
       author.posts.first.special_comments
       author.posts.first.very_special_comment
     end
@@ -146,7 +146,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
   def test_eager_association_loading_of_stis_with_multiple_references
     authors = Author.all.merge!(includes: { posts: { special_comments: { post: [ :special_comments, :very_special_comment ] } } }, order: "comments.body, very_special_comments_posts.body", where: "posts.id = 4").to_a
     assert_equal [authors(:david)], authors
-    assert_no_queries do
+    assert_queries(0) do
       authors.first.posts.first.special_comments.first.post.special_comments
       authors.first.posts.first.special_comments.first.post.very_special_comment
     end
@@ -155,14 +155,14 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
   def test_eager_association_loading_where_first_level_returns_nil
     authors = Author.all.merge!(includes: { post_about_thinking: :comments }, order: "authors.id DESC").to_a
     assert_equal [authors(:bob), authors(:mary), authors(:david)], authors
-    assert_no_queries do
+    assert_queries(0) do
       authors[2].post_about_thinking.comments.first
     end
   end
 
   def test_preload_through_missing_records
     post = Post.where.not(author_id: Author.select(:id)).preload(author: { comments: :post }).first!
-    assert_no_queries { assert_nil post.author }
+    assert_queries(0) { assert_nil post.author }
   end
 
   def test_eager_association_loading_with_missing_first_record
@@ -172,12 +172,12 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
 
   def test_eager_association_loading_with_recursive_cascading_four_levels_has_many_through
     source = Vertex.all.merge!(includes: { sinks: { sinks: { sinks: :sinks } } }, order: "vertices.id").first
-    assert_equal vertices(:vertex_4), assert_no_queries { source.sinks.first.sinks.first.sinks.first }
+    assert_equal vertices(:vertex_4), assert_queries(0) { source.sinks.first.sinks.first.sinks.first }
   end
 
   def test_eager_association_loading_with_recursive_cascading_four_levels_has_and_belongs_to_many
     sink = Vertex.all.merge!(includes: { sources: { sources: { sources: :sources } } }, order: "vertices.id DESC").first
-    assert_equal vertices(:vertex_1), assert_no_queries { sink.sources.first.sources.first.sources.first.sources.first }
+    assert_equal vertices(:vertex_1), assert_queries(0) { sink.sources.first.sources.first.sources.first.sources.first }
   end
 
   def test_eager_association_loading_with_cascaded_interdependent_one_level_and_two_levels
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 9ed25ca7c2..f7aad9d775 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -1245,7 +1245,7 @@ class EagerAssociationTest < ActiveRecord::TestCase
       Post.all.merge!(select: "posts.*, authors.name as author_name", includes: :comments, joins: :author, order: "posts.id").to_a
     end
     assert_equal "David", posts[0].author_name
-    assert_equal posts(:welcome).comments, assert_no_queries { posts[0].comments }
+    assert_equal posts(:welcome).comments.sort_by(&:id), assert_no_queries { posts[0].comments.sort_by(&:id) }
   end
 
   def test_eager_loading_with_conditions_on_join_model_preloads
@@ -1257,8 +1257,8 @@ class EagerAssociationTest < ActiveRecord::TestCase
   end
 
   def test_preload_belongs_to_uses_exclusive_scope
-    people = Person.males.merge(includes: :primary_contact).to_a
-    assert_not_equal people.length, 0
+    people = Person.males.includes(:primary_contact).to_a
+    assert_equal 2, people.length
     people.each do |person|
       assert_no_queries { assert_not_nil person.primary_contact }
       assert_equal Person.find(person.id).primary_contact, person.primary_contact
@@ -1267,16 +1267,17 @@ class EagerAssociationTest < ActiveRecord::TestCase
 
   def test_preload_has_many_uses_exclusive_scope
     people = Person.males.includes(:agents).to_a
+    assert_equal 2, people.length
     people.each do |person|
-      assert_equal Person.find(person.id).agents, person.agents
+      assert_equal Person.find(person.id).agents.sort_by(&:id), person.agents.sort_by(&:id)
     end
   end
 
   def test_preload_has_many_using_primary_key
-    expected = Firm.first.clients_using_primary_key.to_a
+    expected = Firm.first.clients_using_primary_key.sort_by(&:id)
     firm = Firm.includes(:clients_using_primary_key).first
     assert_no_queries do
-      assert_equal expected, firm.clients_using_primary_key
+      assert_equal expected, firm.clients_using_primary_key.sort_by(&:id)
     end
   end
 
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 0e9dafeee6..25cfa0a723 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -550,7 +550,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
     developer = project.developers.first
 
-    assert_no_queries do
+    assert_queries(0) do
       assert_predicate project.developers, :loaded?
       assert_includes project.developers, developer
     end
@@ -745,7 +745,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
   def test_get_ids_for_loaded_associations
     developer = developers(:david)
     developer.projects.reload
-    assert_no_queries do
+    assert_queries(0) do
       developer.project_ids
       developer.project_ids
     end
@@ -873,7 +873,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
 
   def test_has_and_belongs_to_many_associations_on_new_records_use_null_relations
     projects = Developer.new.projects
-    assert_no_queries do
+    assert_queries(0) do
       assert_equal [], projects
       assert_equal [], projects.where(title: "omg")
       assert_equal [], projects.pluck(:title)
diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb
index 0d13174e12..e42af3686e 100644
--- a/activerecord/test/cases/calculations_test.rb
+++ b/activerecord/test/cases/calculations_test.rb
@@ -837,7 +837,7 @@ class CalculationsTest < ActiveRecord::TestCase
 
   def test_pluck_columns_with_same_name
     expected = [["The First Topic", "The Second Topic of the day"], ["The Third Topic of the day", "The Fourth Topic of the day"]]
-    actual = Topic.joins(:replies)
+    actual = Topic.joins(:replies).order(:id)
       .pluck("topics.title", "replies_topics.title")
     assert_equal expected, actual
   end
diff --git a/activerecord/test/cases/comment_test.rb b/activerecord/test/cases/comment_test.rb
index 584e03d196..25e2f20676 100644
--- a/activerecord/test/cases/comment_test.rb
+++ b/activerecord/test/cases/comment_test.rb
@@ -14,6 +14,9 @@ if ActiveRecord::Base.connection.supports_comments?
     class BlankComment < ActiveRecord::Base
     end
 
+    class PkCommented < ActiveRecord::Base
+    end
+
     setup do
       @connection = ActiveRecord::Base.connection
 
@@ -35,8 +38,13 @@ if ActiveRecord::Base.connection.supports_comments?
         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,6 +52,11 @@ if ActiveRecord::Base.connection.supports_comments?
       @connection.drop_table "blank_comments", if_exists: true
     end
 
+    def test_default_primary_key_comment
+      column = Commented.columns_hash["id"]
+      assert_nil column.comment
+    end
+
     def test_column_created_in_block
       column = Commented.columns_hash["name"]
       assert_equal :string, column.type
@@ -164,5 +177,17 @@ if ActiveRecord::Base.connection.supports_comments?
       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
diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb
index e900fd40fb..45ccc442ba 100644
--- a/activerecord/test/models/face.rb
+++ b/activerecord/test/models/face.rb
@@ -6,7 +6,7 @@ class Face < ActiveRecord::Base
   belongs_to :polymorphic_man, polymorphic: true, inverse_of: :polymorphic_face
   # Oracle identifier length is limited to 30 bytes or less, `polymorphic` renamed `poly`
   belongs_to :poly_man_without_inverse, polymorphic: true
-  # These is a "broken" inverse_of for the purposes of testing
+  # These are "broken" inverse_of associations for the purposes of testing
   belongs_to :horrible_man, class_name: "Man", inverse_of: :horrible_face
   belongs_to :horrible_polymorphic_man, polymorphic: true, inverse_of: :horrible_polymorphic_face