Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions dbdimp.c
Original file line number Diff line number Diff line change
Expand Up @@ -3372,14 +3372,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;
Expand All @@ -3390,6 +3382,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)
)
Expand Down Expand Up @@ -3440,10 +3437,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);
Expand All @@ -3468,6 +3475,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"))
Expand All @@ -3483,6 +3495,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 */
Expand All @@ -3495,6 +3512,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));
Expand Down
14 changes: 12 additions & 2 deletions t/13disconnect.t
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand Down
Loading