[Rails] change_table bulk: true での index revert 処理の修正

背景

Rails 8.0 で導入された change_table の bulk: true オプションを使用した際、マイグレーションの revert（ロールバック）処理において index の削除が正しく動作しない問題が発見されました。この問題は #56573 で報告され、#56578 で部分的に対処されましたが、完全な修正には至っていませんでした。

bulk: true オプションは、複数のカラム追加や index 作成を単一の ALTER TABLE 文にまとめることで、マイグレーション実行時のパフォーマンスを向上させる機能です。しかし、revert 時に operations の実行順序が考慮されていなかったため、index を先に削除しようとしてエラーが発生していました。

技術的な変更点

1. operations の revert 順序の修正

最も重要な変更は、CommandRecorder における revert 時の operations 順序の反転です。

変更前:

@commands << [:change_table, [table_name], -> t { bulk_change_table(t.name, commands) }]

変更後:

@commands << [:change_table, [table_name], -> t { bulk_change_table(t.name, commands.reverse) }]

この変更により、revert 時には operations が逆順で実行されるようになります。例えば、以下のようなマイグレーションの場合：

change_table :testings, bulk: true do |t|
  t.column :foo, :string
  t.column :bar, :string
  t.index :foo
end

Revert 時には index の削除 → bar カラムの削除 → foo カラムの削除 の順序で実行され、依存関係が正しく保たれます。

2. bulk_change_table メソッドの引数処理の修正

SchemaStatements#bulk_change_table メソッドにおいて、引数の扱い方が改善されました。

変更前:

args.shift # remove table_name
method = :"#{command}_for_alter"

if respond_to?(method, true)
  sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) }
  sql_fragments.concat(sqls)
  non_combinable_operations.concat(procs)
else
  # ...
  send(command, table, *arguments)
end

変更後:

args.shift # remove table_name
method = :"#{command}_for_alter"

if respond_to?(method, true)
  sqls, procs = Array(send(method, table_name, *args)).partition { |v| v.is_a?(String) }
  sql_fragments.concat(sqls)
  non_combinable_operations.concat(procs)
else
  # ...
  send(command, table_name, *args)
end

変更点：
- table 変数への代入を削除し、table_name を直接使用
- arguments 変数を args に統一

これにより、変数の不要な代入が削除され、コードがより明確になりました。また、table_name を一貫して使用することで、メソッド呼び出し時の意図が明確になっています。

3. テストケースの更新

修正内容を検証するため、テストケースが更新されました。

def change
  change_table :testings, bulk: true do |t|
    t.column :foo, :string
    t.column :bar, :string
    t.index :foo
  end
end

このテストでは、カラム追加と index 作成を含む bulk: true のマイグレーションが正しく revert されることを確認しています。以前は assert_queries_count(1) でクエリ数を検証していましたが、この PR では revert の成功に焦点を当てたテストに変更されました。

影響範囲

この修正により、以下のケースで正しく動作するようになります：

Index を含む bulk マイグレーションの revert: カラムに依存する index が、カラム削除前に適切に削除される
複雑な dependencies を持つ operations: foreign key など、他の依存関係を持つ operations も正しい順序で revert される

既存のコードへの影響は最小限です。bulk: false（デフォルト）の場合は従来通りの動作となり、bulk: true を使用している場合のみ revert の挙動が改善されます。