The default value of a text/blob in mysql strict mode should be nil

In non-strict mode it is '', but if someone is in strict mode then we
should honour the strict semantics.

Also, this removes the need for a completely horrible hack in dirty.rb.

Closes #7780

6 files changed, 70 insertions, 48 deletions

diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb
index 7a5bb9e863..3417e3af17 100644
--- a/activerecord/lib/active_record/attribute_methods/dirty.rb
+++ b/activerecord/lib/active_record/attribute_methods/dirty.rb
@@ -68,17 +68,7 @@ module ActiveRecord
       end
 
       def create(*)
-        if partial_updates?
-          keys = keys_for_partial_update
-
-          # This is an extremely bloody annoying necessity to work around mysql being crap.
-          # See test_mysql_text_not_null_defaults
-          keys.concat self.class.columns.select(&:explicit_default?).map(&:name)
-
-          super keys
-        else
-          super
-        end
+        partial_updates? ? super(keys_for_partial_update) : super
       end
 
       # Serialized attributes should always be written in case they've been
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 8c83c4f5db..b0e7bd7e82 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,19 @@ module ActiveRecord
   module ConnectionAdapters
     class AbstractMysqlAdapter < AbstractAdapter
       class Column < ConnectionAdapters::Column # :nodoc:
-        attr_reader :collation
+        attr_reader :collation, :strict
 
-        def initialize(name, default, sql_type = nil, null = true, collation = nil)
-          super(name, default, sql_type, null)
+        def initialize(name, default, sql_type = nil, null = true, collation = nil, strict = false)
+          @strict    = strict
           @collation = collation
+
+          super(name, default, sql_type, null)
         end
 
         def extract_default(default)
           if sql_type =~ /blob/i || type == :text
             if default.blank?
-              return null ? nil : ''
+              null || strict ? nil : ''
             else
               raise ArgumentError, "#{type} columns cannot have a default value: #{default.inspect}"
             end
@@ -30,10 +32,6 @@ module ActiveRecord
           super
         end
 
-        def explicit_default?
-          !null && (sql_type =~ /blob/i || type == :text)
-        end
-
         # Must return the relevant concrete adapter
         def adapter
           raise NotImplementedError
@@ -571,6 +569,10 @@ module ActiveRecord
         where_sql
       end
 
+      def strict_mode?
+        @config.fetch(:strict, true)
+      end
+
       protected
 
       # MySQL is too stupid to create a temporary table for use subquery, so we have
diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb
index 2028abf6f0..816b5e17c1 100644
--- a/activerecord/lib/active_record/connection_adapters/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/column.rb
@@ -53,10 +53,6 @@ module ActiveRecord
         !default.nil?
       end
 
-      def explicit_default?
-        false
-      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/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
index 328d080687..879eec7fcf 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -53,7 +53,7 @@ module ActiveRecord
       end
 
       def new_column(field, default, type, null, collation) # :nodoc:
-        Column.new(field, default, type, null, collation)
+        Column.new(field, default, type, null, collation, strict_mode?)
       end
 
       def error_number(exception)
@@ -259,9 +259,7 @@ module ActiveRecord
         # Make MySQL reject illegal values rather than truncating or
         # blanking them. See
         # http://dev.mysql.com/doc/refman/5.5/en/server-sql-mode.html#sqlmode_strict_all_tables
-        if @config.fetch(:strict, true)
-          variable_assignments << "SQL_MODE='STRICT_ALL_TABLES'"
-        end
+        variable_assignments << "SQL_MODE='STRICT_ALL_TABLES'" if strict_mode?
 
         encoding = @config[:encoding]
 
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 0b936bbf39..76667616a1 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -150,7 +150,7 @@ module ActiveRecord
       end
 
       def new_column(field, default, type, null, collation) # :nodoc:
-        Column.new(field, default, type, null, collation)
+        Column.new(field, default, type, null, collation, strict_mode?)
       end
 
       def error_number(exception) # :nodoc:
@@ -546,9 +546,7 @@ module ActiveRecord
         # Make MySQL reject illegal values rather than truncating or
         # blanking them. See
         # http://dev.mysql.com/doc/refman/5.5/en/server-sql-mode.html#sqlmode_strict_all_tables
-        if @config.fetch(:strict, true)
-          execute("SET SQL_MODE='STRICT_ALL_TABLES'", :skip_logging)
-        end
+        execute("SET SQL_MODE='STRICT_ALL_TABLES'", :skip_logging) if strict_mode?
       end
 
       def select(sql, name = nil, binds = [])
diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb
index 72f1c99ca0..0df872ff10 100644
--- a/activerecord/test/cases/defaults_test.rb
+++ b/activerecord/test/cases/defaults_test.rb
@@ -51,11 +51,60 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter)
     # We don't want that to happen, so we disable transactional fixtures here.
     self.use_transactional_fixtures = false
 
-    # 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
+    def using_strict(strict)
+      connection = ActiveRecord::Model.remove_connection
+      ActiveRecord::Model.establish_connection connection.merge(strict: strict)
+      yield
+    ensure
+      ActiveRecord::Model.remove_connection
+      ActiveRecord::Model.establish_connection connection
+    end
+
+    # MySQL cannot have defaults on text/blob columns. It reports the
+    # default value as null.
+    #
+    # Despite this, in non-strict mode, MySQL will use an empty string
+    # as the default value of the field, if no other value is
+    # specified.
+    #
+    # Therefore, in non-strict mode, we want column.default to report
+    # an empty string as its default, to be consistent with that.
+    #
+    # In strict mode, column.default should be nil.
+    def test_mysql_text_not_null_defaults_non_strict
+      using_strict(false) do
+        with_text_blob_not_null_table do |klass|
+          assert_equal '', klass.columns_hash['non_null_blob'].default
+          assert_equal '', klass.columns_hash['non_null_text'].default
+
+          assert_nil klass.columns_hash['null_blob'].default
+          assert_nil klass.columns_hash['null_text'].default
+
+          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
+      end
+    end
+
+    def test_mysql_text_not_null_defaults_strict
+      using_strict(true) do
+        with_text_blob_not_null_table do |klass|
+          assert_nil klass.columns_hash['non_null_blob'].default
+          assert_nil klass.columns_hash['non_null_text'].default
+          assert_nil klass.columns_hash['null_blob'].default
+          assert_nil klass.columns_hash['null_text'].default
+
+          assert_raises(ActiveRecord::StatementInvalid) { klass.create }
+        end
+      end
+    end
+
+    def with_text_blob_not_null_table
       klass = Class.new(ActiveRecord::Base)
       klass.table_name = 'test_mysql_text_not_null_defaults'
       klass.connection.create_table klass.table_name do |t|
@@ -64,19 +113,8 @@ if current_adapter?(:MysqlAdapter) or current_adapter?(:Mysql2Adapter)
         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_nil klass.columns_hash['null_blob'].default
-      assert_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
+      yield klass
     ensure
       klass.connection.drop_table(klass.table_name) rescue nil
     end