Merge pull request #17822 from kamipo/refactor_visit_change_column_definition

Refactor `visit_ChangeColumnDefinition`

4 files changed, 14 insertions, 16 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
index 1cf1600d81..68d8a2cd6a 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -22,7 +22,7 @@ module ActiveRecord
       end
     end
 
-    class ChangeColumnDefinition < Struct.new(:column, :type, :options) #:nodoc:
+    class ChangeColumnDefinition < Struct.new(:column, :name) #:nodoc:
     end
 
     class ForeignKeyDefinition < Struct.new(:from_table, :to_table, :options) #:nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
index 24afd9c5da..34d60493ea 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -981,12 +981,12 @@ module ActiveRecord
         end
 
       private
-      def create_table_definition(name, temporary, options, as = nil)
+      def create_table_definition(name, temporary = false, options = nil, as = nil)
         TableDefinition.new native_database_types, name, temporary, options, as
       end
 
       def create_alter_table(name)
-        AlterTable.new create_table_definition(name, false, {})
+        AlterTable.new create_table_definition(name)
       end
 
       def foreign_key_name(table_name, options) # :nodoc:
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
index 83eb5e3ef1..e9a3c26c32 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -38,12 +38,8 @@ module ActiveRecord
         end
 
         def visit_ChangeColumnDefinition(o)
-          column = o.column
-          options = o.options
-          sql_type = type_to_sql(o.type, options[:limit], options[:precision], options[:scale])
-          change_column_sql = "CHANGE #{quote_column_name(column.name)} #{quote_column_name(options[:name])} #{sql_type}"
-          add_column_options!(change_column_sql, options.merge(column: column))
-          add_column_position!(change_column_sql, options)
+          change_column_sql = "CHANGE #{quote_column_name(o.name)} #{accept(o.column)}"
+          add_column_position!(change_column_sql, column_options(o.column))
         end
 
         def add_column_position!(sql, options)
@@ -768,7 +764,7 @@ module ActiveRecord
       end
 
       def add_column_sql(table_name, column_name, type, options = {})
-        td = create_table_definition table_name, options[:temporary], options[:options]
+        td = create_table_definition(table_name)
         cd = td.new_column_definition(column_name, type, options)
         schema_creation.visit_AddColumn cd
       end
@@ -784,21 +780,23 @@ module ActiveRecord
           options[:null] = column.null
         end
 
-        options[:name] = column.name
-        schema_creation.accept ChangeColumnDefinition.new column, type, options
+        td = create_table_definition(table_name)
+        cd = td.new_column_definition(column.name, type, options)
+        schema_creation.accept(ChangeColumnDefinition.new(cd, column.name))
       end
 
       def rename_column_sql(table_name, column_name, new_column_name)
         column  = column_for(table_name, column_name)
         options = {
-          name: new_column_name,
           default: column.default,
           null: column.null,
           auto_increment: column.extra == "auto_increment"
         }
 
         current_type = select_one("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE '#{column_name}'", 'SCHEMA')["Type"]
-        schema_creation.accept ChangeColumnDefinition.new column, current_type, options
+        td = create_table_definition(table_name)
+        cd = td.new_column_definition(new_column_name, current_type, options)
+        schema_creation.accept(ChangeColumnDefinition.new(cd, column.name))
       end
 
       def remove_column_sql(table_name, column_name, type = nil, options = {})
@@ -892,7 +890,7 @@ module ActiveRecord
         end
       end
 
-      def create_table_definition(name, temporary, options, as = nil) # :nodoc:
+      def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc:
         TableDefinition.new(native_database_types, name, temporary, options, as)
       end
 
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 6053e33a20..13bb5c187e 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -761,7 +761,7 @@ module ActiveRecord
           $1.strip if $1
         end
 
-        def create_table_definition(name, temporary, options, as = nil) # :nodoc:
+        def create_table_definition(name, temporary = false, options = nil, as = nil) # :nodoc:
           PostgreSQL::TableDefinition.new native_database_types, name, temporary, options, as
         end
     end