From: Peter Rabbitson <ribasushi@cpan.org>
Date: Thu, 10 Apr 2014 22:43:35 +0000 (+0200)
Subject: Properly detect and test mysql v3 default JOIN behavior
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=dbsrgits%2FDBIx-Class.git;a=commitdiff_plain;h=2231d31c29347c34a6b58b88782da220775bddaa

Properly detect and test mysql v3 default JOIN behavior

While investigating how to rewrite the rather useless 39712b481, it became
clear that not only can the test be performed offline, but that there is
already a pseudo-test due to sqlite's version of 3.x.y being mistaken for
MySQL 3

Properly fix the test to check behavior on both versions 3 and 4, and in the
process fix a potential bug of a stale sqlmaker when a reconnect cycle takes
place against a *different* physical rdbms (which btw makes me realize another
problem with trying to do mixed-environment replication... sigh)
---

diff --git a/lib/DBIx/Class/Storage/DBI.pm b/lib/DBIx/Class/Storage/DBI.pm
index 5245bea..1a302ce 100644
--- a/lib/DBIx/Class/Storage/DBI.pm
+++ b/lib/DBIx/Class/Storage/DBI.pm
@@ -1061,6 +1061,7 @@ sub _populate_dbh {
 
   $self->_dbh(undef); # in case ->connected failed we might get sent here
   $self->_dbh_details({}); # reset everything we know
+  $self->_sql_maker(undef); # this may also end up being different
 
   $self->_dbh($self->_connect);
 
diff --git a/lib/DBIx/Class/Storage/DBI/mysql.pm b/lib/DBIx/Class/Storage/DBI/mysql.pm
index 83ee8b2..0605983 100644
--- a/lib/DBIx/Class/Storage/DBI/mysql.pm
+++ b/lib/DBIx/Class/Storage/DBI/mysql.pm
@@ -106,15 +106,17 @@ sub _run_connection_actions {
 sub sql_maker {
   my $self = shift;
 
-  unless ($self->_sql_maker) {
-    my $maker = $self->next::method (@_);
+  # it is critical to get the version *before* calling next::method
+  # otherwise the potential connect will obliterate the sql_maker
+  # next::method will populate in the _sql_maker accessor
+  my $mysql_ver = $self->_server_info->{normalized_dbms_version};
 
-    # mysql 3 does not understand a bare JOIN
-    my $mysql_ver = $self->_dbh_get_info('SQL_DBMS_VER');
-    $maker->{_default_jointype} = 'INNER' if $mysql_ver =~ /^3/;
-  }
+  my $sm = $self->next::method(@_);
+
+  # mysql 3 does not understand a bare JOIN
+  $sm->{_default_jointype} = 'INNER' if $mysql_ver < 4;
 
-  return $self->_sql_maker;
+  $sm;
 }
 
 sub sqlt_type {
diff --git a/t/71mysql.t b/t/71mysql.t
index 242989e..e1e68ee 100644
--- a/t/71mysql.t
+++ b/t/71mysql.t
@@ -199,20 +199,6 @@ lives_ok { $cd->set_producers ([ $producer ]) } 'set_relationship doesnt die';
     my $cd = $rs->next;
     is ($cd->artist->name, $artist->name, 'Prefetched artist');
   }, 'join does not throw (mysql 3 test)';
-
-  # induce a jointype override, make sure it works even if we don't have mysql3
-  local $schema->storage->sql_maker->{_default_jointype} = 'inner';
-  is_same_sql_bind (
-    $rs->as_query,
-    '(
-      SELECT `me`.`cdid`, `me`.`artist`, `me`.`title`, `me`.`year`, `me`.`genreid`, `me`.`single_track`,
-             `artist`.`artistid`, `artist`.`name`, `artist`.`rank`, `artist`.`charfield`
-        FROM cd `me`
-        INNER JOIN `artist` `artist` ON `artist`.`artistid` = `me`.`artist`
-    )',
-    [],
-    'overridden default join type works',
-  );
 }
 
 ## Can we properly deal with the null search problem?
diff --git a/t/sqlmaker/mysql.t b/t/sqlmaker/mysql.t
index 2755a3d..b5ce8a5 100644
--- a/t/sqlmaker/mysql.t
+++ b/t/sqlmaker/mysql.t
@@ -12,6 +12,7 @@ use DBIC::DebugObj;
 my $schema = DBICTest::Schema->connect (DBICTest->_database, { quote_char => '`' });
 # cheat
 require DBIx::Class::Storage::DBI::mysql;
+*DBIx::Class::Storage::DBI::mysql::_get_server_version = sub { 5 };
 bless ( $schema->storage, 'DBIx::Class::Storage::DBI::mysql' );
 
 # check that double-subqueries are properly wrapped
@@ -102,7 +103,7 @@ bless ( $schema->storage, 'DBIx::Class::Storage::DBI::mysql' );
           FROM (
             SELECT `artist`.`artistid`
               FROM cd `me`
-              INNER JOIN `artist` `artist`
+              JOIN `artist` `artist`
                 ON `artist`.`artistid` = `me`.`artist`
             WHERE `artist`.`name` LIKE ?
           ) `_forced_double_subquery`
@@ -138,4 +139,32 @@ bless ( $schema->storage, 'DBIx::Class::Storage::DBI::mysql' );
   );
 }
 
+# Test support for inner joins on mysql v3
+for (
+  [ 3 => 'INNER JOIN' ],
+  [ 4 => 'JOIN' ],
+) {
+  my ($ver, $join_op) = @$_;
+
+  no warnings 'redefine';
+  local *DBIx::Class::Storage::DBI::mysql::_get_server_version = sub { $ver };
+
+  # we do not care at this point if data is available, just do a reconnect cycle
+  # to clear all caches
+  $schema->storage->disconnect;
+  $schema->storage->ensure_connected;
+
+  is_same_sql_bind (
+    $schema->resultset('CD')->search ({}, { prefetch => 'artist' })->as_query,
+    "(
+      SELECT `me`.`cdid`, `me`.`artist`, `me`.`title`, `me`.`year`, `me`.`genreid`, `me`.`single_track`,
+             `artist`.`artistid`, `artist`.`name`, `artist`.`rank`, `artist`.`charfield`
+        FROM cd `me`
+        $join_op `artist` `artist` ON `artist`.`artistid` = `me`.`artist`
+    )",
+    [],
+    "default join type works for version $ver",
+  );
+}
+
 done_testing;