Set strict mode for compatibility with Mojo::Pg and to avoid data loss

[shadowcat-mst]

https://metacpan.org/pod/DBIx::Class::Storage::DBI::mysql#set_strict_mode hugely reduces mysql's tendency to eat data, and would make Mojo::mysql much more of a drop-in replacement than Mojo::Pg - otherwise the migration path is "switch to Mojo::mysql and add validation code before every INSERT/UPDATE/DELETE to work around mysql's lossy defaults".

At the very least it'd be nice to have it as an option, documented for people migrating from a saner database - I wish I'd turned it on by default in DBIx::Class but it's too late for me now; your choice as to whether it's also too late for you.

[jhthorsen]

I threw together this:

diff --git a/lib/Mojo/mysql.pm b/lib/Mojo/mysql.pm
index e374841..4aa99fc 100644
--- a/lib/Mojo/mysql.pm
+++ b/lib/Mojo/mysql.pm
@@ -73,6 +73,17 @@ sub from_string {

 sub new { @_ > 1 ? shift->SUPER::new->from_string(@_) : shift->SUPER::new }

+sub strict_mode {
+  my $self = shift;
+  my $strict = shift // 1;
+
+  warn "[Mojo::mysql] strict_mode($strict)\n" if $ENV{DBI_TRACE};
+  delete @$self{qw(pid queue)};
+  $self->unsubscribe(connection => \&_set_strict_mode);
+  $self->on(connection => \&_set_strict_mode) if $strict;
+  $self;
+}
+
 sub _dequeue {
   my $self = shift;
   my $dbh;
@@ -101,6 +112,11 @@ sub _enqueue {
   shift @{$self->{queue}} while @{$self->{queue}} > $self->max_connections;
 }

+sub _set_strict_mode {
+  $_[1]->do(q[SET SQL_MODE = CONCAT('ANSI,TRADITIONAL,ONLY_FULL_GROUP_BY,', @@sql_mode)]);
+  $_[1]->do(q[SET SQL_AUTO_IS_NULL = 0]);
+}
+
 1;

 =encoding utf8
diff --git a/t/strict-mode.t b/t/strict-mode.t
new file mode 100644
index 0000000..ccda7b6
--- /dev/null
+++ b/t/strict-mode.t
@@ -0,0 +1,32 @@
+BEGIN { $ENV{MOJO_REACTOR} = 'Mojo::Reactor::Poll' }
+use Mojo::Base -strict;
+use Test::More;
+use Mojo::IOLoop;
+use Mojo::mysql;
+
+plan skip_all => 'TEST_ONLINE=mysql://root@/test' unless $ENV{TEST_ONLINE};
+
+my $mysql = Mojo::mysql->new($ENV{TEST_ONLINE});
+ok $mysql->db->ping, 'connected';
+
+my $db = $mysql->db;
+
+$db->query('drop table if exists strict_mode_test_table');
+$db->query('create table strict_mode_test_table (foo varchar(4))');
+
+$db->query('SET SQL_MODE = ""');    # make sure this fails, even in mysql 5.7
+$db->insert(strict_mode_test_table => {foo => 'y' x 10});
+is $db->select('strict_mode_test_table')->hash->{foo}, 'yyyy', 'fetch invalid data';
+
+is $mysql->strict_mode, $mysql, 'enabled strict mode';
+is @{$mysql->subscribers('connection')}, 1, 'add connection event handler';
+$db = $mysql->db;
+eval { $db->insert(strict_mode_test_table => {foo => 'too_long'}) };
+like $@, qr{Data too long.*foo}, 'too long string';
+
+is $mysql->strict_mode(0), $mysql, 'disable strict mode';
+is @{$mysql->subscribers('connection')}, 0, 'removed connection event handler';
+
+$db->query('drop table if exists strict_mode_test_table');
+
+done_testing;

Does that make sense?

[jhthorsen]

I don't think I can turn it on by default, but I would like to add a warning, if strict_mode() has not been called. Could even make it into a constructor..?

[shadowcat-mst]

On Wed, Mar 22, 2017 at 03:32:41PM -0700, Jan Henning Thorsen wrote: Does that make sense?

Looks about right at a quick glance.

…

-- Matt S Trout - Shadowcat Systems - Perl consulting with a commit bit and a clue http://shadowcat.co.uk/blog/matt-s-trout/ http://twitter.com/shadowcat_mst/ Email me now on mst (at) shadowcat.co.uk and let's chat about how our CPAN commercial support, training and consultancy packages could help your team.

[jhthorsen]

Mojo::mysql 1.01 is on its way to CPAN.

Thanks for helping out @shadowcat-mst 👍

[jhthorsen]

Would love to hear your feedback @shadowcat-mst or if you have changes to which command to run in the future. I decided to update the SYNOPSIS to use strict_mode() instead of new().