Deal with MySQL's quirky handling of defaults and blob/text columns

[#1043 state:committed]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

6 files changed, 49 insertions, 3 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index d31e63017e..ff2b064e1c 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *Edge*
 
+* MySQL: cope with quirky default values for not-null text columns.  #1043 [Frederick Cheung]
+
 * Multiparameter attributes skip time zone conversion for time-only columns [#1030 state:resolved] [Geoff Buesing]
 
 * Base.skip_time_zone_conversion_for_attributes uses class_inheritable_accessor, so that subclasses don't overwrite Base [#346 state:resolved] [miloops]
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 22304edfc9..58992f91da 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb
@@ -40,6 +40,10 @@ module ActiveRecord
         type == :integer || type == :float || type == :decimal
       end
 
+      def has_default?
+        !default.nil?
+      end
+
       # Returns the Ruby class that corresponds to the abstract data type.
       def klass
         case type
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index c2a0fb72bf..a26fd02b90 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -80,7 +80,7 @@ module ActiveRecord
       def extract_default(default)
         if type == :binary || type == :text
           if default.blank?
-            nil
+            return null ? nil : ''
           else
             raise ArgumentError, "#{type} columns cannot have a default value: #{default.inspect}"
           end
@@ -91,6 +91,11 @@ module ActiveRecord
         end
       end
 
+      def has_default?
+        return false if type == :binary || type == :text #mysql forbids defaults on blob and text columns
+        super
+      end
+
       private
         def simplified_type(field_type)
           return :boolean if MysqlAdapter.emulate_booleans && field_type.downcase.index("tinyint(1)")
diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb
index b90ed88c6b..4f96e225c1 100644
--- a/activerecord/lib/active_record/schema_dumper.rb
+++ b/activerecord/lib/active_record/schema_dumper.rb
@@ -102,7 +102,7 @@ HEADER
             spec[:precision] = column.precision.inspect if !column.precision.nil?
             spec[:scale]     = column.scale.inspect if !column.scale.nil?
             spec[:null]      = 'false' if !column.null
-            spec[:default]   = default_string(column.default) if !column.default.nil?
+            spec[:default]   = default_string(column.default) if column.has_default?
             (spec.keys - [:name, :type]).each{ |k| spec[k].insert(0, "#{k.inspect} => ")}
             spec
           end.compact
diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb
index 3473b846a0..ee84cb8af8 100644
--- a/activerecord/test/cases/defaults_test.rb
+++ b/activerecord/test/cases/defaults_test.rb
@@ -19,6 +19,37 @@ class DefaultTest < ActiveRecord::TestCase
   end
 
   if current_adapter?(:MysqlAdapter)
+
+    #MySQL 5 and higher is quirky with not null text/blob columns.
+    #With MySQL Text/blob columns cannot have defaults. If the column is not null MySQL will report that the column has a null default
+    #but it behaves as though the column had a default of ''
+    def test_mysql_text_not_null_defaults
+      klass = Class.new(ActiveRecord::Base)
+      klass.table_name = 'test_mysql_text_not_null_defaults'
+      klass.connection.create_table klass.table_name do |t|
+        t.column :non_null_text, :text, :null => false
+        t.column :non_null_blob, :blob, :null => false
+        t.column :null_text, :text, :null => true
+        t.column :null_blob, :blob, :null => true
+      end
+      assert_equal '', klass.columns_hash['non_null_blob'].default
+      assert_equal '', klass.columns_hash['non_null_text'].default
+
+      assert_equal nil, klass.columns_hash['null_blob'].default
+      assert_equal nil, klass.columns_hash['null_text'].default
+
+      assert_nothing_raised do
+        instance = klass.create!
+        assert_equal '', instance.non_null_text
+        assert_equal '', instance.non_null_blob
+        assert_nil instance.null_text
+        assert_nil instance.null_blob
+      end
+    ensure
+      klass.connection.drop_table(klass.table_name) rescue nil
+    end
+
+
     # MySQL uses an implicit default 0 rather than NULL unless in strict mode.
     # We use an implicit NULL so schema.rb is compatible with other databases.
     def test_mysql_integer_not_null_defaults
diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb
index c1a8da2270..ac44dd7ffe 100644
--- a/activerecord/test/cases/migration_test.rb
+++ b/activerecord/test/cases/migration_test.rb
@@ -1133,7 +1133,11 @@ if ActiveRecord::Base.connection.supports_migrations?
       columns = Person.connection.columns(:binary_testings)
       data_column = columns.detect { |c| c.name == "data" }
 
-      assert_nil data_column.default
+      if current_adapter?(:MysqlAdapter)
+        assert_equal '', data_column.default
+      else
+        assert_nil data_column.default
+      end
 
       Person.connection.drop_table :binary_testings rescue nil
     end