Skip to content
/ server Public

MDEV-39008 FederatedX: set time_zone on connection change #4791

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.

Sign up for GitHub

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

Open
PeterKoletzki wants to merge 1 commit into
base: 10.11
Choose a base branch
from
Open

MDEV-39008 FederatedX: set time_zone on connection change #4791

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
17 changes: 16 additions & 1 deletion storage/federatedx/federatedx_io_mysql.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ class federatedx_io_mysql :public federatedx_io
DYNAMIC_ARRAY savepoints;
bool requested_autocommit;
bool actual_autocommit;
/** VIO for which time_zone was set; when vio changes (reconnect), set again */
void *time_zone_set_vio;

int actual_query(const char *buffer, size_t length);
bool test_all_restrict() const;
Expand Down Expand Up @@ -134,7 +136,8 @@ federatedx_io *instantiate_io_mysql(MEM_ROOT *server_root,

federatedx_io_mysql::federatedx_io_mysql(FEDERATEDX_SERVER *aserver)
: federatedx_io(aserver),
requested_autocommit(TRUE), actual_autocommit(TRUE)
requested_autocommit(TRUE), actual_autocommit(TRUE),
time_zone_set_vio(NULL)
{
DBUG_ENTER("federatedx_io_mysql::federatedx_io_mysql");

Expand Down Expand Up @@ -162,6 +165,7 @@ void federatedx_io_mysql::reset()
{
reset_dynamic(&savepoints);
set_active(FALSE);
time_zone_set_vio= NULL;

requested_autocommit= TRUE;
mysql.reconnect= 1;
Expand Down Expand Up @@ -455,8 +459,19 @@ int federatedx_io_mysql::actual_query(const char *buffer, size_t length)
if ((error= mysql_real_query(&mysql, STRING_WITH_LEN("set time_zone='+00:00'"))))
DBUG_RETURN(error);

time_zone_set_vio= mysql.net.vio;
mysql.reconnect= 1;
}
else if (mysql.net.vio != time_zone_set_vio)
Copy link
Member

@gkodinov gkodinov Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer that you develop a single boolean flag: e.g. needs_to_set_timezone and set it where it needs to be set. And then add a single check for it right before mysql_real_query() and set the timezone.

Note that this still has a potential for error: if you haven't detected the disconnect yet the first query you execute will be executed sans timezone set. The ones that follow will execute timezone set. But the first one will be off.

This is because automatic reconnect is done inside the client library. And it will automatically retry the same query right after re-connecting on its own.

So, MYSQL.reconnect should not be used for anything that can depend on session state. This is why the mariadb CLI is implementing its own, client app level reconnect.

If you really want to fix this in all cases, please consider removing the use of MYSQL.reconnect and do as mysql.cc does.

I will take the minor refactoring suggested in the first paragraph too. But I would like you to consider doing the full fix first.

{
/*
Connection changed (e.g. driver reconnect or new connection from pool).
Set time_zone so TIMESTAMP is consistent (avoids TZ mismatch with LB).
*/
if ((error= mysql_real_query(&mysql, STRING_WITH_LEN("set time_zone='+00:00'"))))
DBUG_RETURN(error);
time_zone_set_vio= mysql.net.vio;
}

error= mysql_real_query(&mysql, buffer, (ulong)length);

Expand Down