SchemaDumper mixes table comment and primary key comment

[yskkin]

Steps to reproduce

begin
  require "bundler/inline"
rescue LoadError => e
  $stderr.puts "Bundler version 1.10 or later is required. Please update your Bundler"
  raise e
end

gemfile(true) do
  source "https://rubygems.org"
  gem "rails", "~>5.1"
  gem "arel", github: "rails/arel"
  gem "pg"
end

require "active_record"
require "minitest/autorun"
require "logger"

# Ensure backward compatibility with Minitest 4
Minitest::Test = MiniTest::Unit::TestCase unless defined?(Minitest::Test)

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: "postgresql", database: "test")
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :payments, force: true, id: false do |t|
    t.bigint :pk, comment: "primary key comment", primary_key: true
    t.decimal :amount, precision: 10, scale: 0, default: 0, null: false
  end
  change_table_comment :payments, "table comment"
end

class Payment < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_comment_should_appear_only_once_in_create_table_line
    stream = StringIO.new
    ActiveRecord::SchemaDumper.dump(ActiveRecord::Base.connection, stream)

    create_table_line = stream.string[/^\s*create_table.+$/]
    puts stream.string
    assert_equal 1, create_table_line.scan("comment:").length
  end
end

Expected behavior

dumped schema should be:

  create_table "payments", id: false, force: :cascade, comment: "table comment" do |t|
    t.bigint "pk", comment: "primary key comment"
    t.decimal "amount", precision: 10, default: "0", null: false
  end

Actual behavior

dumped schema is:

  create_table "payments", primary_key: "pk", id: :bigint, comment: "primary key comment", default: nil, force: :cascade, comment: "table comment" do |t|
    t.decimal "amount", precision: 10, default: "0", null: false
  end

System configuration

Rails version: master

Ruby version: 2.4.0-p0

[yskkin]

I was checking against 5.1.
This is fixed on master.
sorry for noise.

[guigs]

Hey, I'm still having this issue with Rails 5.2.3 and also on master.

I just had to modify the test to create pk with type integer instead of bigint.

Looks like if pk is type bigint it ignores all other options for column, including the comment.

[guigs]

The problem is that a :comment option is added for the pk column (except if it is bigint) in:

rails/activerecord/lib/active_record/schema_dumper.rb

Lines 127 to 130 in 1da0e25

	pkcolspec = column_spec_for_primary_key(pkcol)
	if pkcolspec.present?
	tbl.print ", #{format_colspec(pkcolspec)}"
	end

Then another :comment option is added for the table in

rails/activerecord/lib/active_record/schema_dumper.rb

Lines 137 to 140 in 1da0e25

	table_options = @connection.table_options(table)
	if table_options.present?
	tbl.print ", #{format_options(table_options)}"
	end