Skip to content

Commit

Permalink
*_for_alter methods should also takes keyword arguments
Browse files Browse the repository at this point in the history
Follow up 5572df9.
  • Loading branch information
kamipo committed Jan 20, 2020
1 parent 9a400d8 commit 6e1f852
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 36 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ def remove_columns(table_name, *column_names)
# In that case, +type+ and +options+ will be used by #add_column.
# Indexes on the column are automatically removed.
def remove_column(table_name, column_name, type = nil, **options)
execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, options)}"
execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, **options)}"
end

# Changes the column's definition according to the new options.
Expand Down Expand Up @@ -1436,20 +1436,37 @@ def bulk_change_table(table_name, operations)
non_combinable_operations.each(&:call)
end

def add_column_for_alter(table_name, column_name, type, options = {})
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)
schema_creation.accept(AddColumnDefinition.new(cd))
end

def remove_column_for_alter(table_name, column_name, type = nil, options = {})
def remove_column_for_alter(table_name, column_name, type = nil, **options)
"DROP COLUMN #{quote_column_name(column_name)}"
end

def remove_columns_for_alter(table_name, *column_names)
def remove_columns_for_alter(table_name, *column_names, **options)
column_names.map { |column_name| remove_column_for_alter(table_name, column_name) }
end

def add_timestamps_for_alter(table_name, **options)
options[:null] = false if options[:null].nil?

if !options.key?(:precision) && supports_datetime_with_precision?
options[:precision] = 6
end

[
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_columns_for_alter(table_name, :updated_at, :created_at)
end

def insert_versions_sql(versions)
sm_table = quote_table_name(schema_migration.table_name)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,20 +685,6 @@ def remove_index_for_alter(table_name, options = {})
"DROP INDEX #{quote_column_name(index_name)}"
end

def add_timestamps_for_alter(table_name, options = {})
options[:null] = false if options[:null].nil?

if !options.key?(:precision) && supports_datetime_with_precision?
options[:precision] = 6
end

[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 supports_rename_index?
mariadb? ? false : database_version >= "5.7.6"
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,7 @@ def extract_foreign_key_action(specifier)
end
end

def add_column_for_alter(table_name, column_name, type, options = {})
def add_column_for_alter(table_name, column_name, type, **options)
return super unless options.key?(:comment)
[super, Proc.new { change_column_comment(table_name, column_name, options[:comment]) }]
end
Expand Down Expand Up @@ -711,20 +711,6 @@ def change_column_null_for_alter(table_name, column_name, null, default = nil)
"ALTER COLUMN #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL"
end

def add_timestamps_for_alter(table_name, options = {})
options[:null] = false if options[:null].nil?

if !options.key?(:precision) && supports_datetime_with_precision?
options[:precision] = 6
end

[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 @@ -218,7 +218,7 @@ def test_raises_type_mismatch_with_namespaced_class
assert_nil defined?(Region), "This test requires that there is no top-level Region class"

ActiveRecord::Base.connection.instance_eval do
create_table(:admin_regions) { |t| t.string :name }
create_table(:admin_regions, force: true) { |t| t.string :name }
add_column :admin_users, :region_id, :integer
end
Admin.const_set "RegionalUser", Class.new(Admin::User) { belongs_to(:region) }
Expand Down
12 changes: 10 additions & 2 deletions activerecord/test/cases/migration/change_table_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -283,14 +283,22 @@ def test_change_default_changes_column

def test_remove_drops_single_column
with_change_table do |t|
@connection.expect :remove_columns, nil, [:delete_me, :bar]
if RUBY_VERSION < "2.7"
@connection.expect :remove_columns, nil, [:delete_me, :bar, {}]
else
@connection.expect :remove_columns, nil, [:delete_me, :bar]
end
t.remove :bar
end
end

def test_remove_drops_multiple_columns
with_change_table do |t|
@connection.expect :remove_columns, nil, [:delete_me, :bar, :baz]
if RUBY_VERSION < "2.7"
@connection.expect :remove_columns, nil, [:delete_me, :bar, :baz, {}]
else
@connection.expect :remove_columns, nil, [:delete_me, :bar, :baz]
end
t.remove :bar, :baz
end
end
Expand Down

0 comments on commit 6e1f852

Please sign in to comment.