Permalink
Browse files

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
  • Loading branch information...
1 parent eb72e62 commit af8c8b432abfe478f478fee35c5624c8c04d65e4 @jonleighton jonleighton committed Oct 19, 2012
@@ -68,17 +68,7 @@ def update(*)
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
@@ -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 @@ def has_default?
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 @@ def limited_update_conditions(where_sql, quoted_table_name, quoted_primary_key)
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
@@ -53,10 +53,6 @@ def has_default?
!default.nil?
end
- def explicit_default?
- false
- end
-
# Returns the Ruby class that corresponds to the abstract data type.
def klass
case type
@@ -53,7 +53,7 @@ def each_hash(result) # :nodoc:
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 @@ def configure_connection
# 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]
@@ -150,7 +150,7 @@ def each_hash(result) # :nodoc:
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 @@ def configure_connection
# 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 = [])
@@ -51,11 +51,60 @@ class DefaultsTestWithoutTransactionalFixtures < ActiveRecord::TestCase
# 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 @@ def test_mysql_text_not_null_defaults
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

0 comments on commit af8c8b4

Please sign in to comment.