Merge pull request #26154 from kamipo/remove_text_default_treated_as_empty_string

Remove text default treated as an empty string in non-strict mode

6 files changed, 70 insertions, 84 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 0655fa3aa7..0fd22ed614 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,35 @@
+*   Remove text default treated as an empty string in non-strict mode for
+    consistency with other types.
+
+    Strict mode controls how MySQL handles invalid or missing values in
+    data-change statements such as INSERT or UPDATE. If strict mode is not
+    in effect, MySQL inserts adjusted values for invalid or missing values
+    and produces warnings.
+
+        def test_mysql_not_null_defaults_non_strict
+          using_strict(false) do
+            with_mysql_not_null_table do |klass|
+              record = klass.new
+              assert_nil record.non_null_integer
+              assert_nil record.non_null_string
+              assert_nil record.non_null_text
+              assert_nil record.non_null_blob
+
+              record.save!
+              record.reload
+
+              assert_equal 0,  record.non_null_integer
+              assert_equal "", record.non_null_string
+              assert_equal "", record.non_null_text
+              assert_equal "", record.non_null_blob
+            end
+          end
+        end
+
+    https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict
+
+    *Ryuta Kamizono*
+
 *   Sqlite3 migrations to add a column to an existing table can now be
     successfully rolled back when the column was given and invalid column
     type.
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 f09c9c9e7f..3a28879c15 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb
@@ -709,7 +709,7 @@ module ActiveRecord
         end
 
         def fetch_type_metadata(sql_type, extra = "")
-          MySQL::TypeMetadata.new(super(sql_type), extra: extra, strict: strict_mode?)
+          MySQL::TypeMetadata.new(super(sql_type), extra: extra)
         end
 
         def add_index_length(quoted_columns, **options)
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/column.rb b/activerecord/lib/active_record/connection_adapters/mysql/column.rb
index 5452e44c84..72b57f4b8f 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/column.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/column.rb
@@ -2,17 +2,7 @@ module ActiveRecord
   module ConnectionAdapters
     module MySQL
       class Column < ConnectionAdapters::Column # :nodoc:
-        delegate :strict, :extra, to: :sql_type_metadata, allow_nil: true
-
-        def initialize(*)
-          super
-          extract_default
-        end
-
-        def has_default?
-          return false if blob_or_text_column? # MySQL forbids defaults on blob and text columns
-          super
-        end
+        delegate :extra, to: :sql_type_metadata, allow_nil: true
 
         def blob_or_text_column?
           /\A(?:tiny|medium|long)?blob\b/ === sql_type || type == :text
@@ -29,14 +19,6 @@ module ActiveRecord
         def auto_increment?
           extra == "auto_increment"
         end
-
-        private
-
-          def extract_default
-            if blob_or_text_column?
-              @default = null || strict ? nil : ""
-            end
-          end
       end
     end
   end
diff --git a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb
index 1be5cb4740..24dcf852e1 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb
@@ -2,13 +2,12 @@ module ActiveRecord
   module ConnectionAdapters
     module MySQL
       class TypeMetadata < DelegateClass(SqlTypeMetadata) # :nodoc:
-        attr_reader :extra, :strict
+        attr_reader :extra
 
-        def initialize(type_metadata, extra: "", strict: false)
+        def initialize(type_metadata, extra: "")
           super(type_metadata)
           @type_metadata = type_metadata
           @extra = extra
-          @strict = strict
         end
 
         def ==(other)
@@ -24,7 +23,7 @@ module ActiveRecord
         protected
 
           def attributes_for_hash
-            [self.class, @type_metadata, extra, strict]
+            [self.class, @type_metadata, extra]
           end
       end
     end
diff --git a/activerecord/test/cases/column_definition_test.rb b/activerecord/test/cases/column_definition_test.rb
index 989beaa5c8..a65bb89052 100644
--- a/activerecord/test/cases/column_definition_test.rb
+++ b/activerecord/test/cases/column_definition_test.rb
@@ -60,14 +60,13 @@ module ActiveRecord
         end
 
         def test_should_not_set_default_for_blob_and_text_data_types
-          text_type = MySQL::TypeMetadata.new(
-            SqlTypeMetadata.new(type: :text))
+          text_type = MySQL::TypeMetadata.new(SqlTypeMetadata.new(type: :text))
 
           text_column = MySQL::Column.new("title", nil, text_type)
-          assert_equal nil, text_column.default
+          assert_nil text_column.default
 
           not_null_text_column = MySQL::Column.new("title", nil, text_type, false)
-          assert_equal "", not_null_text_column.default
+          assert_nil not_null_text_column.default
         end
 
         def test_has_default_should_return_false_for_blob_and_text_data_types
diff --git a/activerecord/test/cases/defaults_test.rb b/activerecord/test/cases/defaults_test.rb
index f94815d34c..fcaff38f82 100644
--- a/activerecord/test/cases/defaults_test.rb
+++ b/activerecord/test/cases/defaults_test.rb
@@ -127,92 +127,66 @@ if current_adapter?(:Mysql2Adapter)
       ActiveRecord::Base.establish_connection connection
     end
 
-    # MySQL cannot have defaults on text/blob columns. It reports the
-    # default value as null.
+    # Strict mode controls how MySQL handles invalid or missing values
+    # in data-change statements such as INSERT or UPDATE. A value can be
+    # invalid for several reasons. For example, it might have the wrong
+    # data type for the column, or it might be out of range. A value is
+    # missing when a new row to be inserted does not contain a value for
+    # a non-NULL column that has no explicit DEFAULT clause in its definition.
+    # (For a NULL column, NULL is inserted if the value is missing.)
     #
-    # 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.
+    # If strict mode is not in effect, MySQL inserts adjusted values for
+    # invalid or missing values and produces warnings. In strict mode,
+    # you can produce this behavior by using INSERT IGNORE or UPDATE IGNORE.
     #
-    # 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
+    # https://dev.mysql.com/doc/refman/5.7/en/sql-mode.html#sql-mode-strict
+    def test_mysql_not_null_defaults_non_strict
       using_strict(false) do
-        with_text_blob_not_null_table do |klass|
+        with_mysql_not_null_table do |klass|
           record = klass.new
-          assert_equal "", record.non_null_blob
-          assert_equal "", record.non_null_text
-
-          assert_nil record.null_blob
-          assert_nil record.null_text
+          assert_nil record.non_null_integer
+          assert_nil record.non_null_string
+          assert_nil record.non_null_text
+          assert_nil record.non_null_blob
 
           record.save!
           record.reload
 
+          assert_equal 0,  record.non_null_integer
+          assert_equal "", record.non_null_string
           assert_equal "", record.non_null_text
           assert_equal "", record.non_null_blob
-
-          assert_nil record.null_text
-          assert_nil record.null_blob
         end
       end
     end
 
-    def test_mysql_text_not_null_defaults_strict
+    def test_mysql_not_null_defaults_strict
       using_strict(true) do
-        with_text_blob_not_null_table do |klass|
+        with_mysql_not_null_table do |klass|
           record = klass.new
-          assert_nil record.non_null_blob
+          assert_nil record.non_null_integer
+          assert_nil record.non_null_string
           assert_nil record.non_null_text
-          assert_nil record.null_blob
-          assert_nil record.null_text
+          assert_nil record.non_null_blob
 
           assert_raises(ActiveRecord::StatementInvalid) { klass.create }
         end
       end
     end
 
-    def with_text_blob_not_null_table
+    def with_mysql_not_null_table
       klass = Class.new(ActiveRecord::Base)
-      klass.table_name = "test_mysql_text_not_null_defaults"
+      klass.table_name = "test_mysql_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
+        t.integer :non_null_integer, null: false
+        t.string  :non_null_string,  null: false
+        t.text    :non_null_text,    null: false
+        t.blob    :non_null_blob,    null: false
       end
 
       yield klass
     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
-      klass = Class.new(ActiveRecord::Base)
-      klass.table_name = "test_integer_not_null_default_zero"
-      klass.connection.create_table klass.table_name do |t|
-        t.column :zero, :integer, null: false, default: 0
-        t.column :omit, :integer, null: false
-      end
-
-      assert_equal "0", klass.columns_hash["zero"].default
-      assert !klass.columns_hash["zero"].null
-      assert_equal nil, klass.columns_hash["omit"].default
-      assert !klass.columns_hash["omit"].null
-
-      assert_raise(ActiveRecord::StatementInvalid) { klass.create! }
-
-      assert_nothing_raised do
-        instance = klass.create!(omit: 1)
-        assert_equal 0, instance.zero
-        assert_equal 1, instance.omit
-      end
-    ensure
-      klass.connection.drop_table(klass.table_name) rescue nil
-    end
   end
 end