Introduce versions formatter for the schema dumper

[fatkodima]

Follow up to #44363 (cc @ghiculescu).

Previously, the versions were sorted in decreasing order and new added to the top of the list. Within large-enough teams, this can cause many merge conflicts near the top of this list. Within my company, we have lots of developers pushing lots of schema changes and half of the time someone merges a PR, lots of existing PRs now have conflicts with the master.

Now, the versions are sorted by their reverse representations (20241201183002 -> 20038110214202), in decreasing (can be increasing, does not matter, but to be consistent with the previous implementation) order. This will greatly reduce the likelihood of merge conflicts as now new versions can be inserted at any place in the existing list. I did a simple calculation and while before we had a likelihood of conflict within a list of, say 5 top elements (20%), now - within a whole list of 600+ items (0.1%).

I also added a comment with reversed version representation for each version within dumped structure.sql file for people to more easily solve the merge conflicts. But we can remove this and add a comment at the top of the INSERT describing the algorithm.

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

[ghiculescu]

I love this idea!

IMO the comment is a bit confusing unless you already know about the algorithm.

[fatkodima]

Agree, changed to print only a comment at the top of the list.

[byroot]

I'm not too sure how I feel about that.

I'm sympathetic to the desire of reducing conflicts, but it feels weird to have these versions ordered in a seemingly arbitrary fashion. I'll try to seek a second opinion.

[fatkodima]

Another alternative is the ability for users to provide a custom proc, which will be used for sorting these versions.

[jjb]

Suggested change

	-- Versions are sorted by reverse representations of numbers (abc -> cba) in descending order.
	-- Versions are sorted by reverse representations of numbers (20241225000000 -> 00000052214202) in descending order.

[eileencodes]

I'm sympathetic to the problem but I really don't like this solution. Reversing the strings is just confusing and I can imagine both beginners and seasoned Rails devs getting stuck on this. If we merge this we'll get endless bug reports and complaints.

[fatkodima]

Wdyt about my alternative idea

Another alternative is the ability for users to provide a custom proc, which will be used for sorting these versions.

so people can be able to configure their custom logic? And we preserve the original implementation by default.

[ghiculescu]

If we merge this we'll get endless bug reports and complaints.

I'm skeptical. I am going to monkey patch this into our codebase, and see if anyone (~50 devs) notices or asks.

[ghiculescu]

^ A week later nobody has asked about it (as far as I'm aware) and we're definitely seeing less merge conflicts.

[ghiculescu]

[byroot]

This was the reactions when the order was reversed: #44363

[fatkodima]

If this PR is implemented in alternative way, for example, providing users a proc to configure, for example config.active_record.structure_versions_sort = ->(a, b) { b <=> a }, we can preserve the current behaviour, for not happy people from the linked PR to use ->(a, b) { a <=> b } to return to the previous behavior, and for the proposal in this PR to use the behaviour we want ->(a, b) { b.to_s.reverse <=> a.to_s.reverse }.

[byroot]

If this PR is implemented in alternative way, for example, providing users a proc to configure,

@eileencodes @matthewd thoughts? I'm not against it, but it's yet another super obscure config.

[byroot]

for example, providing users a proc to configure, for example config.active_record.structure_versions_sort = ->(a, b) { b <=> a }

I can be convinced to offer some way to customize this, but I wonder if rather than just exposing a sort proc, we might not as well refactor insert_versions_sql out so it's implemented by a class that you can substitue, e.g (untested pseudo-code).

def insert_versions_sql
  self.class.schema_version_inserter.generate(pool.schema_migration)
end

Then you get the flexibility to order however you want, but also include some comments, and even fetch the list of versions differently.

[fatkodima]

Updated with the suggestion for a separate class.

[chaadow]

I've just tried this snippet and it's missing

   private attr_reader :connection
   def initialize(connection)
        @connection = connection
   end

[chaadow]

cc @fatkodima

[fatkodima]

oops, #54054