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

Add bulk alter support for PostgreSQL #31331

Merged
merged 1 commit into from
Jan 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,31 @@ 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 << procs if procs.present?
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 Expand Up @@ -394,7 +419,7 @@ def add_column(table_name, column_name, type, options = {}) #:nodoc:

def change_column(table_name, column_name, type, options = {}) #:nodoc:
clear_cache!
sqls, procs = change_column_for_alter(table_name, column_name, type, options)
sqls, procs = Array(change_column_for_alter(table_name, column_name, type, options)).partition { |v| v.is_a?(String) }
execute "ALTER TABLE #{quote_table_name(table_name)} #{sqls.join(", ")}"
procs.each(&:call)
end
Expand Down Expand Up @@ -665,12 +690,10 @@ def change_column_sql(table_name, column_name, type, options = {})

def change_column_for_alter(table_name, column_name, type, options = {})
sqls = [change_column_sql(table_name, column_name, type, options)]
procs = []
sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default)
sqls << change_column_null_for_alter(table_name, column_name, options[:null], options[:default]) if options.key?(:null)
procs << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment)

[sqls, procs]
sqls << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment)
sqls
end


Expand All @@ -694,6 +717,14 @@ def change_column_null_for_alter(table_name, column_name, null, default = nil) #
"ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL"
end

def add_timestamps_for_alter(table_name, options = {})
[add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)]
end

def remove_timestamps_for_alter(table_name, options = {})
[remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)]
end

def add_index_opclass(quoted_columns, **options)
opclasses = options_for_index_columns(options[:opclass])
quoted_columns.each do |name, column|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ class PostgreSQLAdapter < AbstractAdapter
include PostgreSQL::SchemaStatements
include PostgreSQL::DatabaseStatements

def supports_bulk_alter?
true
end

def supports_index_sort_order?
true
end
Expand Down
34 changes: 27 additions & 7 deletions activerecord/test/cases/migration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -801,8 +801,15 @@ def test_adding_indexes
t.integer :age
end

# Adding an index fires a query every time to check if an index already exists or not
assert_queries(3) do
classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
expected_query_count = {
"Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not
"PostgreSQLAdapter" => 2,
}.fetch(classname) {
raise "need an expected query count for #{classname}"
}

assert_queries(expected_query_count) do
with_bulk_change_table do |t|
t.index :username, unique: true, name: :awesome_username_index
t.index [:name, :age]
Expand All @@ -826,7 +833,15 @@ def test_removing_index

assert index(:index_delete_me_on_name)

assert_queries(3) do
classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
expected_query_count = {
"Mysql2Adapter" => 3, # Adding an index fires a query every time to check if an index already exists or not
"PostgreSQLAdapter" => 2,
}.fetch(classname) {
raise "need an expected query count for #{classname}"
}

assert_queries(expected_query_count) do
with_bulk_change_table do |t|
t.remove_index :name
t.index :name, name: :new_name_index, unique: true
Expand All @@ -848,10 +863,15 @@ def test_changing_columns
assert ! column(:name).default
assert_equal :date, column(:birthdate).type

# One query for columns (delete_me table)
# One query for primary key (delete_me table)
# One query to do the bulk change
assert_queries(3, ignore_none: true) do
classname = ActiveRecord::Base.connection.class.name[/[^:]*$/]
expected_query_count = {
"Mysql2Adapter" => 3, # one query for columns, one query for primary key, one query to do the bulk change
"PostgreSQLAdapter" => 2, # one query for columns, one for bulk change
}.fetch(classname) {
raise "need an expected query count for #{classname}"
}

assert_queries(expected_query_count, ignore_none: true) do
with_bulk_change_table do |t|
t.change :name, :string, default: "NONAME"
t.change :birthdate, :datetime
Expand Down
2 changes: 1 addition & 1 deletion activerecord/test/cases/test_case.rb
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ def clear_log; self.log = []; self.log_all = []; end
# instead examining the SQL content.
oracle_ignored = [/^select .*nextval/i, /^SAVEPOINT/, /^ROLLBACK TO/, /^\s*select .* from all_triggers/im, /^\s*select .* from all_constraints/im, /^\s*select .* from all_tab_cols/im, /^\s*select .* from all_sequences/im]
mysql_ignored = [/^SHOW FULL TABLES/i, /^SHOW FULL FIELDS/, /^SHOW CREATE TABLE /i, /^SHOW VARIABLES /, /^\s*SELECT (?:column_name|table_name)\b.*\bFROM information_schema\.(?:key_column_usage|tables)\b/im]
postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i]
postgresql_ignored = [/^\s*select\b.*\bfrom\b.*pg_namespace\b/im, /^\s*select tablename\b.*from pg_tables\b/im, /^\s*select\b.*\battname\b.*\bfrom\b.*\bpg_attribute\b/im, /^SHOW search_path/i, /^\s*SELECT\b.*::regtype::oid\b/im]
sqlite3_ignored = [/^\s*SELECT name\b.*\bFROM sqlite_master/im, /^\s*SELECT sql\b.*\bFROM sqlite_master/im]

[oracle_ignored, mysql_ignored, postgresql_ignored, sqlite3_ignored].each do |db_ignored_sql|
Expand Down