Skip to content

Commit

Permalink
use PostgreSQL's bulk_alter_table implementation
Browse files Browse the repository at this point in the history
Running this migration on mysql at current master fails
because `add_references_for_alter` is missing.

```
change_table :users, bulk: true do |t|
  t.references :article
end
```

This is also true for postgresql adapter,
but its `bulk_alter_table` implementation can fallback in such case.
postgresql's implementation is desirable to prevent unknown failure like this.
  • Loading branch information
yskkin committed Apr 13, 2019
1 parent 713f624 commit 53f1b3e
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 48 deletions.
Expand Up @@ -1379,6 +1379,31 @@ def can_remove_index_by_name?(options)
options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty?
end

def bulk_change_table(table_name, operations)
sql_fragments = []
non_combinable_operations = []

operations.each do |command, args|
table, arguments = args.shift, args
method = :"#{command}_for_alter"

if respond_to?(method, true)
sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) }
sql_fragments << sqls
non_combinable_operations.concat(procs)
else
execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
non_combinable_operations.each(&:call)
sql_fragments = []
non_combinable_operations = []
send(command, table, *arguments)
end
end

execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
non_combinable_operations.each(&:call)
end

def add_column_for_alter(table_name, column_name, type, options = {})
td = create_table_definition(table_name)
cd = td.new_column_definition(column_name, type, options)
Expand Down
Expand Up @@ -63,7 +63,7 @@ def mariadb? # :nodoc:
/mariadb/i.match?(full_version)
end

def supports_bulk_alter? #:nodoc:
def supports_bulk_alter?
true
end

Expand Down Expand Up @@ -285,21 +285,6 @@ def table_comment(table_name) # :nodoc:
SQL
end

def bulk_change_table(table_name, operations) #:nodoc:
sqls = operations.flat_map do |command, args|
table, arguments = args.shift, args
method = :"#{command}_for_alter"

if respond_to?(method, true)
send(method, table, *arguments)
else
raise "Unknown method called : #{method}(#{arguments.inspect})"
end
end.join(", ")

execute("ALTER TABLE #{quote_table_name(table_name)} #{sqls}")
end

def change_table_comment(table_name, comment) #:nodoc:
comment = "" if comment.nil?
execute("ALTER TABLE #{quote_table_name(table_name)} COMMENT #{quote(comment)}")
Expand Down
Expand Up @@ -368,31 +368,6 @@ def primary_keys(table_name) # :nodoc:
SQL
end

def bulk_change_table(table_name, operations)
sql_fragments = []
non_combinable_operations = []

operations.each do |command, args|
table, arguments = args.shift, args
method = :"#{command}_for_alter"

if respond_to?(method, true)
sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) }
sql_fragments << sqls
non_combinable_operations.concat(procs)
else
execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
non_combinable_operations.each(&:call)
sql_fragments = []
non_combinable_operations = []
send(command, table, *arguments)
end
end

execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty?
non_combinable_operations.each(&:call)
end

# Renames a table.
# Also renames a table's primary key sequence if the sequence name exists and
# matches the Active Record default.
Expand Down
24 changes: 17 additions & 7 deletions activerecord/test/cases/adapters/mysql2/active_schema_test.rb
Expand Up @@ -10,7 +10,15 @@ def setup
ActiveRecord::Base.connection.send(:default_row_format)
ActiveRecord::Base.connection.singleton_class.class_eval do
alias_method :execute_without_stub, :execute
def execute(sql, name = nil) sql end
def execute(sql, name = nil)
ActiveSupport::Notifications.instrumenter.instrument(
"sql.active_record",
sql: sql,
name: name,
connection: self) do
sql
end
end
end
end

Expand Down Expand Up @@ -89,17 +97,19 @@ def (ActiveRecord::Base.connection).index_name_exists?(*); false; end

%w(SPATIAL FULLTEXT UNIQUE).each do |type|
expected = "ALTER TABLE `people` ADD #{type} INDEX `index_people_on_last_name` (`last_name`)"
actual = ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t|
t.index :last_name, type: type
assert_sql(expected) do
ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t|
t.index :last_name, type: type
end
end
assert_equal expected, actual
end

expected = "ALTER TABLE `people` ADD INDEX `index_people_on_last_name` USING btree (`last_name`(10)), ALGORITHM = COPY"
actual = ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t|
t.index :last_name, length: 10, using: :btree, algorithm: :copy
assert_sql(expected) do
ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t|
t.index :last_name, length: 10, using: :btree, algorithm: :copy
end
end
assert_equal expected, actual
end

def test_drop_table
Expand Down

0 comments on commit 53f1b3e

Please sign in to comment.