Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow base for Table schema definition to be injected into #change_table #45669

Conversation

adrianna-chang-shopify
Copy link
Contributor

Summary

This PR is a part of the work to allow users to customize migration behaviour by swapping in their own migration execution strategy objects, which can be used to perform schema statement methods. (See #45324 for more context).

#change_table (when not used in bulk alter query mode) operates by building a Table schema definition, and then yielding that object to the #change_table block. Any methods called within the block are sent to the Table object, which then forwards them to @base. For example, #column:

def column(column_name, type, index: nil, **options)
raise_on_if_exist_options(options)
@base.add_column(name, column_name, type, **options)
if index
index_options = index.is_a?(Hash) ? index : {}
index(column_name, **index_options)
end
end

simply sends the #add_column message to @base, which is the connection adapter.

If apps are using a custom migration execution strategy and defining methods such as #add_column, #add_index, etc., they probably don't want #change_table to send commands to the connection adapter. Instead, it makes more sense that these methods would go to their strategy object. As such, I'm proposing that we simply allow base to be passed as an optional argument to #change_table, with the Table definition constructed using the supplied base. A custom strategy could then do something like this:

class CustomStrategy < ActiveRecord::Migration::DefaultStrategy
  def change_table(table_name, **options, &block)
    super(table_name, self, **options, &block)
  end

  def add_column(table_name, column_name, type, **options)
    # Custom add column
  end

  def add_index(table_name, column_name, **options)
    # Custom add index
  end
end

class TestChangeTable < ActiveRecord::Migration[7.1]
  def change
    change_table(:users, bulk: true) do |t|
      t.column(:name, :string) # This will use the strategy's #add_column method
      t.index(:name, unique: true) # This will use the strategy's #add_index method
    end
  end
end

@adrianna-chang-shopify adrianna-chang-shopify force-pushed the ac-allow-change-table-to-specify-base branch from 579695d to 1f77762 Compare July 27, 2022 13:57
@eileencodes eileencodes merged commit 2e8f4cb into rails:main Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants