Merge pull request #212 from choroba/reconnect

Do not call mariadb_db_reconnect() during changing $dbh attributes unless needed

Author: choroba

dbdimp.c

@@ -3402,14 +3402,6 @@ mariadb_db_STORE_attrib(
   char *key = SvPV(keysv, kl); /* needs to process get magic */
   const bool bool_value = SvTRUE_nomg(valuesv);
 
-  if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
-  {
-    mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
-    if (memEQs(key, kl, "AutoCommit"))
-      croak_autocommit_failure(); /* does not return */
-    return 0;
-  }
-
   if (memEQs(key, kl, "AutoCommit"))
   {
       bool oldval = DBIc_has(imp_dbh, DBIcf_AutoCommit) ? TRUE : FALSE;
@@ -3420,6 +3412,11 @@ mariadb_db_STORE_attrib(
       /* if setting AutoCommit on ... */
       if (!imp_dbh->no_autocommit_cmd)
       {
+        if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
+        {
+          mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
+          croak_autocommit_failure(); /* does not return */
+        }
         if (
             mysql_autocommit(imp_dbh->pmysql, bool_value)
            )
@@ -3470,10 +3467,20 @@ mariadb_db_STORE_attrib(
         error_nul_character(dbh, "mariadb_fabric_opt_group");
         return 0;
       }
+      if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
+      {
+        mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
+        return 0;
+      }
       mysql_options(imp_dbh->pmysql, FABRIC_OPT_GROUP, str);
     }
     else if (memEQs(key, kl, "mariadb_fabric_opt_default_mode"))
     {
+      if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
+      {
+        mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
+        return 0;
+      }
       if (SvOK(valuesv)) {
         STRLEN len;
         const char *str = SvPV_nomg(valuesv, len);
@@ -3498,6 +3505,11 @@ mariadb_db_STORE_attrib(
         mariadb_dr_do_error(dbh, CR_UNKNOWN_ERROR, "Valid settings for FABRIC_OPT_MODE are 'ro' or 'rw'", "HY000");
         return 0;
       }
+      if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
+      {
+        mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
+        return 0;
+      }
       mysql_options(imp_dbh->pmysql, FABRIC_OPT_MODE, str);
     }
     else if (memEQs(key, kl, "mariadb_fabric_opt_group_credentials"))
@@ -3518,6 +3530,11 @@ mariadb_db_STORE_attrib(
       /* MYSQL_OPT_MAX_ALLOWED_PACKET was added in MariaDB 10.2.6 and MariaDB 10.3.1 */
       UV uv = SvUV_nomg(valuesv);
       unsigned long packet_size = (uv <= ULONG_MAX ? uv : ULONG_MAX);
+      if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
+      {
+        mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
+        return 0;
+      }
       mysql_options(imp_dbh->pmysql, MYSQL_OPT_MAX_ALLOWED_PACKET, &packet_size);
 #else
       /* before MySQL 5.7.9 and MariaDB 10.2.6 and MariaDB 10.3.0 it is not possible to change max_allowed_packet after connection was established */
@@ -3530,6 +3547,11 @@ mariadb_db_STORE_attrib(
     {
       if (!imp_dbh->connected) /* When not connected, it is handled in mariadb_dr_connect() */
         return 0;
+      if (!imp_dbh->pmysql && !mariadb_db_reconnect(dbh, NULL))
+      {
+        mariadb_dr_do_error(dbh, CR_SERVER_GONE_ERROR, "MySQL server has gone away", "HY000");
+        return 0;
+      }
       if (mysql_set_server_option(imp_dbh->pmysql, bool_value ? MYSQL_OPTION_MULTI_STATEMENTS_ON : MYSQL_OPTION_MULTI_STATEMENTS_OFF) != 0)
       {
         mariadb_dr_do_error(dbh, mysql_errno(imp_dbh->pmysql), mysql_error(imp_dbh->pmysql), mysql_sqlstate(imp_dbh->pmysql));

t/13disconnect.t

@@ -10,11 +10,21 @@ require 'lib.pl';
 
 my $dbh1 = DbiTestConnect($test_dsn, $test_user, $test_password, { RaiseError => 1, PrintError => 0 });
 
-plan tests => 17;
-
+plan tests => 19;
+
+my $err1;
+my $err2;
+# set HandleError with catching of all errors
+$dbh1->{HandleError} = sub { $err1 = $_[0]; };
+{
+# localize HandleError with different closure and ensure that the original one is restored after the disconnect()
+local $dbh1->{HandleError} = sub { $err2 = $_[0]; };
 ok $dbh1->{Active};
 ok $dbh1->disconnect();
 ok !$dbh1->{Active};
+}
+ok !defined $err1 or diag $err1;
+ok !defined $err2 or diag $err2;
 
 ok my $dbh2 = DBI->connect($test_dsn, $test_user, $test_password, { RaiseError => 1, PrintError => 0 });
 ok my $dbh3 = DBI->connect($test_dsn, $test_user, $test_password, { RaiseError => 1, PrintError => 0 });