Skip to content

Commit

Permalink
Reverse the order of INSERT statements in structure.sql dumps
Browse files Browse the repository at this point in the history
This was suggested as a better fix in #43414. The goal is to decrease merge conflicts that often come at the bottom due to the semi colon placement.

We've been running with a monkey patch for it for about a month, and can confirm, it's definitely an improvement. So I'm making this as an alternative suggestion to #43414

Adding @mlarraz as a co-author - thanks for the original inspiration.

Co-authored-by: @mlarraz <mlarraz@users.noreply.github.com>
  • Loading branch information
ghiculescu and mlarraz committed Feb 8, 2022
1 parent b923f70 commit 7c8ac6c
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 5 deletions.
10 changes: 10 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,13 @@
* Reversed the order of `INSERT` statements in `structure.sql` dumps

This should decrease the likelihood of merge conflicts. New migrations
will now be added at the top of the list.

For existing apps, there will be a large diff the next time `structure.sql`
is generated.

*Alex Ghiculescu*, *Matt Larraz*

* Fix PG.connect keyword arguments deprecation warning on ruby 2.7

Fixes #44307.
Expand Down
Expand Up @@ -1675,7 +1675,7 @@ def insert_versions_sql(versions)

if versions.is_a?(Array)
sql = +"INSERT INTO #{sm_table} (version) VALUES\n"
sql << versions.map { |v| "(#{quote(v)})" }.join(",\n")
sql << versions.reverse.map { |v| "(#{quote(v)})" }.join(",\n")
sql << ";\n\n"
sql
else
Expand Down
14 changes: 10 additions & 4 deletions activerecord/test/cases/schema_dumper_test.rb
Expand Up @@ -25,15 +25,21 @@ def test_dump_schema_information_with_empty_versions
assert_no_match(/INSERT INTO/, schema_info)
end

def test_dump_schema_information_outputs_lexically_ordered_versions
def test_dump_schema_information_outputs_lexically_reverse_ordered_versions_regardless_of_database_order
versions = %w{ 20100101010101 20100201010101 20100301010101 }
versions.reverse_each do |v|
versions.shuffle.each do |v|
ActiveRecord::SchemaMigration.create!(version: v)
end

schema_info = ActiveRecord::Base.connection.dump_schema_information
assert_match(/20100201010101.*20100301010101/m, schema_info)
assert_includes schema_info, "20100101010101"
expected = <<~STR
INSERT INTO #{ActiveRecord::Base.connection.quote_table_name("schema_migrations")} (version) VALUES
('20100301010101'),
('20100201010101'),
('20100101010101');
STR
assert_equal expected, schema_info
ensure
ActiveRecord::SchemaMigration.delete_all
end
Expand Down

0 comments on commit 7c8ac6c

Please sign in to comment.