Merge pull request #6069 from mipearson/use_strict_mode_in_mysql

Use strict mode in mysql

6 files changed, 54 insertions, 1 deletions

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index a661a44f1f..31886c8212 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,5 +1,11 @@
 ## Rails 4.0.0 (unreleased) ##
 
+*   `mysql` and `mysql2` connections will set `SQL_MODE=STRICT_ALL_TABLES` by
+    default to avoid silent data loss. This can be disabled by specifying
+    `strict: false` in your `database.yml`.
+
+    *Michael Pearson*
+
 *   Added default order to `first` to assure consistent results among
     diferent database engines. Introduced `take` as a replacement to
     the old behavior of `first`.
diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
index 92908d9599..350ccce03d 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb
@@ -253,6 +253,14 @@ module ActiveRecord
         # By default, MySQL 'where id is null' selects the last inserted id.
         # Turn this off. http://dev.rubyonrails.org/ticket/6778
         variable_assignments = ['SQL_AUTO_IS_NULL=0']
+
+        # 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
+
         encoding = @config[:encoding]
 
         # make sure we set the encoding
diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
index 724dbff1f0..0b6734b010 100644
--- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb
@@ -53,6 +53,7 @@ module ActiveRecord
     # * <tt>:database</tt> - The name of the database. No default, must be provided.
     # * <tt>:encoding</tt> - (Optional) Sets the client encoding by executing "SET NAMES <encoding>" after connection.
     # * <tt>:reconnect</tt> - Defaults to false (See MySQL documentation: http://dev.mysql.com/doc/refman/5.0/en/auto-reconnect.html).
+    # * <tt>:strict</tt> - Defaults to true. Enable STRICT_ALL_TABLES. (See MySQL documentation: http://dev.mysql.com/doc/refman/5.5/en/server-sql-mode.html)
     # * <tt>:sslca</tt> - Necessary to use MySQL with an SSL connection.
     # * <tt>:sslkey</tt> - Necessary to use MySQL with an SSL connection.
     # * <tt>:sslcert</tt> - Necessary to use MySQL with an SSL connection.
@@ -404,6 +405,13 @@ module ActiveRecord
         # By default, MySQL 'where id is null' selects the last inserted id.
         # Turn this off. http://dev.rubyonrails.org/ticket/6778
         execute("SET SQL_AUTO_IS_NULL=0", :skip_logging)
+
+        # 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
       end
 
       def select(sql, name = nil, binds = [])
diff --git a/activerecord/test/cases/adapters/mysql/connection_test.rb b/activerecord/test/cases/adapters/mysql/connection_test.rb
index fa2ba8d592..5e1c52c9ba 100644
--- a/activerecord/test/cases/adapters/mysql/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql/connection_test.rb
@@ -120,6 +120,19 @@ class MysqlConnectionTest < ActiveRecord::TestCase
     end
   end
 
+  def test_mysql_default_in_strict_mode
+    result = @connection.exec_query "SELECT @@SESSION.sql_mode"
+    assert_equal [["STRICT_ALL_TABLES"]], result.rows
+  end
+
+  def test_mysql_strict_mode_disabled
+    run_without_connection do |orig_connection|
+      ActiveRecord::Model.establish_connection(orig_connection.merge({:strict => false}))
+      result = ActiveRecord::Model.connection.exec_query "SELECT @@SESSION.sql_mode"
+      assert_equal [['']], result.rows
+    end
+  end
+
   private
 
   def run_without_connection
diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb
index 8e2b9ca9a5..684c7f5929 100644
--- a/activerecord/test/cases/adapters/mysql2/connection_test.rb
+++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb
@@ -29,6 +29,22 @@ class MysqlConnectionTest < ActiveRecord::TestCase
     assert @connection.active?
   end
 
+  # TODO: Below is a straight up copy/paste from mysql/connection_test.rb
+  # I'm not sure what the correct way is to share these tests between
+  # adapters in minitest.
+  def test_mysql_default_in_strict_mode
+    result = @connection.exec_query "SELECT @@SESSION.sql_mode"
+    assert_equal [["STRICT_ALL_TABLES"]], result.rows
+  end
+
+  def test_mysql_strict_mode_disabled
+    run_without_connection do |orig_connection|
+      ActiveRecord::Model.establish_connection(orig_connection.merge({:strict => false}))
+      result = ActiveRecord::Model.connection.exec_query "SELECT @@SESSION.sql_mode"
+      assert_equal [['']], result.rows
+    end
+  end
+
   private
 
   def run_without_connection
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 7082d5dc86..cef08cd99c 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -38,7 +38,9 @@ ActiveRecord::Schema.define do
   create_table :admin_users, :force => true do |t|
     t.string :name
     t.text :settings, :null => true
-    t.text :preferences, :null => false, :default => ""
+    # MySQL does not allow default values for blobs. Fake it out with a
+    # big varchar below.
+    t.string :preferences, :null => false, :default => '', :limit => 1024
     t.references :account
   end