Run DROP DEFAULT for NULL value; Add text type checks

This commit does two things.

1. Reverts a recent change that was calling SET DEFAULT NULL
when altering a table. This PR revert that back to instead call DROP
DEFAULT in the case when the column is not nullable.
2. Adds a check to ensure that only nonbinary data types are eligible
for having the `collation` carry through when performing a change_column
migration.

activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb

@@ -398,8 +398,8 @@ module ActiveRecord
           options[:comment] = column.comment
         end
 
-        unless options.key?(:collation) || type == :binary
-          options[:collation] = column.collation
+        unless options.key?(:collation)
+          options[:collation] = column.collation if text_type?(type)
         end
 
         unless options.key?(:auto_increment)
@@ -683,6 +683,10 @@ module ActiveRecord
       EMULATE_BOOLEANS_TRUE = { emulate_booleans: true }.freeze
 
       private
+        def text_type?(type)
+          TYPE_MAP.lookup(type).is_a?(Type::String) || TYPE_MAP.lookup(type).is_a?(Type::Text)
+        end
+
         def extended_type_map_key
           if @default_timezone
             { default_timezone: @default_timezone, emulate_booleans: emulate_booleans }

activerecord/lib/active_record/connection_adapters/mysql/schema_creation.rb

@@ -25,11 +25,11 @@ module ActiveRecord
           end
 
           def visit_ChangeColumnDefaultDefinition(o)
-            sql = +"ALTER COLUMN #{quote_column_name(o.column.name)} SET DEFAULT "
-            if o.default.nil?
-              sql << "NULL"
+            sql = +"ALTER COLUMN #{quote_column_name(o.column.name)} "
+            if o.default.nil? && !o.column.null
+              sql << "DROP DEFAULT"
             else
-              sql << quote_default_expression(o.default, o.column)
+              sql << "SET DEFAULT #{quote_default_expression(o.default, o.column)}"
             end
           end
 

activerecord/test/cases/adapters/mysql2/charset_collation_test.rb

@@ -48,7 +48,7 @@ class Mysql2CharsetCollationTest < ActiveRecord::Mysql2TestCase
     assert_equal "utf8mb4_general_ci", column.collation
   end
 
-  test "change column ensures binary column type are set to nil" do
+  test "change column doesn't preserve collation for string to binary types" do
     @connection.add_column :charset_collations, :description, :string, charset: "utf8mb4", collation: "utf8mb4_unicode_ci"
     @connection.change_column :charset_collations, :description, :binary
 
@@ -58,7 +58,17 @@ class Mysql2CharsetCollationTest < ActiveRecord::Mysql2TestCase
     assert_nil column.collation
   end
 
-  test "change column preserves collation" do
+  test "change column doesn't preserve collation for string to non-string types" do
+    @connection.add_column :charset_collations, :description, :string, charset: "utf8mb4", collation: "utf8mb4_unicode_ci"
+    @connection.change_column :charset_collations, :description, :int
+
+    column = @connection.columns(:charset_collations).find { |c| c.name == "description" }
+
+    assert_equal :integer, column.type
+    assert_nil column.collation
+  end
+
+  test "change column preserves collation for string to text" do
     @connection.add_column :charset_collations, :description, :string, charset: "utf8mb4", collation: "utf8mb4_unicode_ci"
     @connection.change_column :charset_collations, :description, :text
 

activerecord/test/cases/migration/columns_test.rb

@@ -277,6 +277,19 @@ module ActiveRecord
         assert_nil TestModel.new.first_name
       end
 
+      def test_change_column_default_to_null_with_not_null
+        add_column "test_models", "first_name", :string, null: false
+        add_column "test_models", "age", :integer, null: false
+
+        connection.change_column_default "test_models", "first_name", nil
+
+        assert_nil TestModel.new.first_name
+
+        connection.change_column_default "test_models", "age", nil
+
+        assert_nil TestModel.new.age
+      end
+
       def test_change_column_default_with_from_and_to
         add_column "test_models", "first_name", :string
         connection.change_column_default "test_models", "first_name", from: nil, to: "Tester"