Merge pull request #10425 from ranjaykrishna/push_add_column_options_to_schema_creation

Push add column options to schema creation

4 files changed, 47 insertions, 38 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 c982d65d65..0be4b5cb19 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -25,6 +25,9 @@ module ActiveRecord
       end
     end
 
+    class ChangeColumnDefinition < Struct.new(:column, :type, :options) #:nodoc:
+    end
+
     # Represents the schema of an SQL table in an abstract way. This class
     # provides methods for manipulating the schema representation.
     #
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 2a4eda3622..3ac55a0f11 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb
@@ -694,17 +694,6 @@ module ActiveRecord
         end
       end
 
-      def add_column_options!(sql, options) #:nodoc:
-        sql << " DEFAULT #{quote(options[:default], options[:column])}" if options_include_default?(options)
-        # must explicitly check for :null to allow change_column to work on migrations
-        if options[:null] == false
-          sql << " NOT NULL"
-        end
-        if options[:auto_increment] == true
-          sql << " AUTO_INCREMENT"
-        end
-      end
-
       # SELECT DISTINCT clause for a given set of columns and a given ORDER BY clause.
       #
       #   distinct("posts.id", ["posts.created_at desc"])
diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
index 26586f0974..e232cad982 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb
@@ -116,6 +116,12 @@ module ActiveRecord
           send m, o
         end
 
+        def visit_AddColumn(o)
+          sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale)
+          sql = "ADD #{quote_column_name(o.name)} #{sql_type}"
+          add_column_options!(sql, column_options(o))
+        end
+
         private
 
         def visit_AlterTable(o)
@@ -123,12 +129,6 @@ module ActiveRecord
           sql << o.adds.map { |col| visit_AddColumn col }.join(' ')
         end
 
-        def visit_AddColumn(o)
-          sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale)
-          sql = "ADD #{quote_column_name(o.name)} #{sql_type}"
-          add_column_options!(sql, column_options(o))
-        end
-
         def visit_ColumnDefinition(o)
           sql_type = type_to_sql(o.type.to_sym, o.limit, o.precision, o.scale)
           column_sql = "#{quote_column_name(o.name)} #{sql_type}"
@@ -149,6 +149,8 @@ module ActiveRecord
           column_options[:null] = o.null unless o.null.nil?
           column_options[:default] = o.default unless o.default.nil?
           column_options[:column] = o
+          column_options[:first] = o.first
+          column_options[:after] = o.after
           column_options
         end
 
@@ -164,9 +166,20 @@ module ActiveRecord
           @conn.type_to_sql type.to_sym, limit, precision, scale
         end
 
-        def add_column_options!(column_sql, column_options)
-          @conn.add_column_options! column_sql, column_options
-          column_sql
+        def add_column_options!(sql, options)
+          sql << " DEFAULT #{@conn.quote(options[:default], options[:column])}" if options_include_default?(options)
+          # must explicitly check for :null to allow change_column to work on migrations
+          if options[:null] == false
+            sql << " NOT NULL"
+          end
+          if options[:auto_increment] == true
+            sql << " AUTO_INCREMENT"
+          end
+          sql
+        end
+
+        def options_include_default?(options)
+          options.include?(:default) && !(options[:null] == false && options[:default].nil?)
         end
       end
 
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 d098ded77c..5b25b26164 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -4,17 +4,26 @@ module ActiveRecord
   module ConnectionAdapters
     class AbstractMysqlAdapter < AbstractAdapter
       class SchemaCreation < AbstractAdapter::SchemaCreation
-        private
 
         def visit_AddColumn(o)
-          add_column_position!(super, o)
+          add_column_position!(super, column_options(o))
+        end
+
+        private
+        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)
+          add_column_position!(change_column_sql, options)
         end
 
-        def add_column_position!(sql, column)
-          if column.first
+        def add_column_position!(sql, options)
+          if options[:first]
             sql << " FIRST"
-          elsif column.after
-            sql << " AFTER #{quote_column_name(column.after)}"
+          elsif options[:after]
+            sql << " AFTER #{quote_column_name(options[:after])}"
           end
           sql
         end
@@ -661,10 +670,9 @@ module ActiveRecord
       end
 
       def add_column_sql(table_name, column_name, type, options = {})
-        add_column_sql = "ADD #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
-        add_column_options!(add_column_sql, options)
-        add_column_position!(add_column_sql, options)
-        add_column_sql
+        td = create_table_definition table_name, options[:temporary], options[:options]
+        cd = td.new_column_definition(column_name, type, options)
+        schema_creation.visit_AddColumn cd
       end
 
       def change_column_sql(table_name, column_name, type, options = {})
@@ -678,14 +686,12 @@ module ActiveRecord
           options[:null] = column.null
         end
 
-        change_column_sql = "CHANGE #{quote_column_name(column_name)} #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}"
-        add_column_options!(change_column_sql, options)
-        add_column_position!(change_column_sql, options)
-        change_column_sql
+        options[:name] = column.name
+        schema_creation.accept ChangeColumnDefinition.new column, type, options
       end
 
       def rename_column_sql(table_name, column_name, new_column_name)
-        options = {}
+        options = { name: new_column_name }
 
         if column = columns(table_name).find { |c| c.name == column_name.to_s }
           options[:default] = column.default
@@ -696,9 +702,7 @@ module ActiveRecord
         end
 
         current_type = select_one("SHOW COLUMNS FROM #{quote_table_name(table_name)} LIKE '#{column_name}'", 'SCHEMA')["Type"]
-        rename_column_sql = "CHANGE #{quote_column_name(column_name)} #{quote_column_name(new_column_name)} #{current_type}"
-        add_column_options!(rename_column_sql, options)
-        rename_column_sql
+        schema_creation.accept ChangeColumnDefinition.new column, current_type, options
       end
 
       def remove_column_sql(table_name, column_name, type = nil, options = {})