Skip to content

Fix alphabetize_schema on Rails 8.2 - #322

Closed
zarqman wants to merge 1 commit into
ankane:masterfrom
zarqman:fix-alphabetize-columns-rails82
Closed

zarqman wants to merge 1 commit into
ankane:masterfrom
zarqman:fix-alphabetize-columns-rails82

Conversation

@zarqman

@zarqman zarqman commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

In rails/rails#58494, Rails 8.2 changed the #columns method to accept both a single table name (same as <= v8.1) and an array of table names (new). When an array, the returned value changes from [columns] to {table_name => [columns]}. This detects the two variants and sorts each appropriately.

Original error:

$ rails db:schema:dump
NoMethodError: undefined method 'name' for an instance of Array (NoMethodError)

      @connection.columns(...).sort_by(&:name)
                              ^^^^^^^^

Tasks: TOP => db:schema:dump

Additionally, the automatic alphabetization of columns introduced in v8.1 has been removed again in v8.2 (rails/rails#56565). This updates the test conditional to be strictly 8.1 instead of >= 8.1.


Note: When testing against rails main:HEAD today, I'm seeing a flaky test. I think this is unrelated to the changes in this PR and probably something else in Rails 8.2. It passes sometimes, so might be dependent on test run ordering.

  1) Error:
ChangeColumnTest#test_timestamps:
StrongMigrations::UnsafeMigration:
=== Dangerous operation detected #strong_migrations ===
[...]
    test/change_column_test.rb:125:in 'ChangeColumnTest#test_timestamps'

In rails/rails#58494, Rails 8.2 changed the #columns method to accept both
a single table name (same as <= v8.1) and an array of table names (new).
When an array, the returned value changes from `[columns]` to
`{table_name => [columns]}`. This detects the two variants and sorts each
appropriately.

Additionally, the automatic alphabetization of columns introduced in v8.1
has been removed again in v8.2 (rails/rails#56565). This updates the test
conditional to be strictly 8.1 instead of >= 8.1.
@ankane ankane closed this in d52e4ac Sep 22, 2026
@ankane

ankane commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Hi @zarqman, thanks for the PR. Merged a version of this in the commit above.

@ankane

ankane commented Sep 22, 2026

Copy link
Copy Markdown
Owner

Also, fixed the timestamp test in eb88bcf.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants