Skip to content

Commit

Permalink
MDEV-31524 Fixing spider table param / variable overriding
Browse files Browse the repository at this point in the history
The existing (incorrect) overriding mechanism is:

Non-minus-one var value overrides table param overrides default value.

Before MDEV-27169, unspecified var value is -1. So if the user sets
both the var to be a value other than -1 and the table param, the var
value will prevail, which is incorrect.

After MDEV-27169, unspecified var value is default value. So if the
user does not set the var but sets the table param, the default value
will prevail, which is even more incorrect.

This patch fixes it so that table param, if specified, always
overrides var value, and the latter if not specified or set to -1,
falls back to the default value

We achieve this by replacing all such overriding in spd_param.cc with
macros that override in the correct way, and removing all the
"overriding -1" lines involving table params in
spider_set_connect_info_default() except for those table params not
defined as sysvar/thdvar in spd_params.cc

We also introduced macros for non-overriding sysvar and thdvar, so
that the code is cleaner and less error-prone

In server versions where MDEV-27169 has not been applied, we also
backport the patch, that is, replacing -1 default values with real
default values

In server versions where MDEV-28006 has not been applied, we do the
same for udf params
  • Loading branch information
mariadb-YuchenPei committed Jul 13, 2023
1 parent 7dde504 commit e1d31a1
Show file tree
Hide file tree
Showing 7 changed files with 449 additions and 1,107 deletions.
46 changes: 46 additions & 0 deletions storage/spider/mysql-test/spider/bugfix/r/mdev_31524.result
@@ -0,0 +1,46 @@

MDEV-31524 Spider variables that double as table params overriding mechanism is buggy

for master_1
for child2
for child3
SET @old_spider_read_only_mode = @@session.spider_read_only_mode;
CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root');
set session spider_read_only_mode = default;
create table t2 (c int);
create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_mdev_31524",TABLE "t2"';
/* 1 */ insert into t1 values (42);
drop table t1, t2;
set session spider_read_only_mode = 1;
create table t2 (c int);
create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_mdev_31524",TABLE "t2"';
/* 2 */ insert into t1 values (42);
ERROR HY000: Table 'test.t1' is read only
drop table t1, t2;
set session spider_read_only_mode = -1;
create table t2 (c int);
create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "srv_mdev_31524",TABLE "t2"';
/* 3 */ insert into t1 values (42);
drop table t1, t2;
SET session spider_read_only_mode = default;
create table t2 (c int);
create table t1 (c int) ENGINE=Spider COMMENT='read_only_mode "1", WRAPPER "mysql", srv "srv_mdev_31524",TABLE "t2"';
/* 4 */ insert into t1 values (42);
ERROR HY000: Table 'test.t1' is read only
drop table t1, t2;
set session spider_read_only_mode = 1;
create table t2 (c int);
create table t1 (c int) ENGINE=Spider COMMENT='read_only_mode "0", WRAPPER "mysql", srv "srv_mdev_31524",TABLE "t2"';
/* 5 */ insert into t1 values (42);
drop table t1, t2;
SET session spider_read_only_mode = -1;
create table t2 (c int);
create table t1 (c int) ENGINE=Spider COMMENT='read_only_mode "1", WRAPPER "mysql", srv "srv_mdev_31524",TABLE "t2"';
/* 6 */ insert into t1 values (42);
ERROR HY000: Table 'test.t1' is read only
drop table t1, t2;
drop server srv_mdev_31524;
SET session spider_read_only_mode = @old_spider_read_only_mode;
for master_1
for child2
for child3
73 changes: 73 additions & 0 deletions storage/spider/mysql-test/spider/bugfix/t/mdev_31524.test
@@ -0,0 +1,73 @@
--echo
--echo MDEV-31524 Spider variables that double as table params overriding mechanism is buggy
--echo

--disable_query_log
--disable_result_log
--source ../../t/test_init.inc
--enable_result_log
--enable_query_log

--let $srv=srv_mdev_31524
SET @old_spider_read_only_mode = @@session.spider_read_only_mode;
evalp CREATE SERVER $srv FOREIGN DATA WRAPPER MYSQL OPTIONS (SOCKET "$MASTER_1_MYSOCK", DATABASE 'test',user 'root');

# when the user does not set var nor the table option, the default
# value (0 in this case) takes effect.
set session spider_read_only_mode = default;
create table t2 (c int);
eval create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t2"';
/* 1 */ insert into t1 values (42);
drop table t1, t2;

# when the user sets var but not the table option, the var should be
# take effect.
set session spider_read_only_mode = 1;
create table t2 (c int);
eval create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t2"';
--error 12518
/* 2 */ insert into t1 values (42);
drop table t1, t2;

# when the user sets var to -1 and does not set the table option, the
# default value takes effect.
set session spider_read_only_mode = -1;
create table t2 (c int);
eval create table t1 (c int) ENGINE=Spider COMMENT='WRAPPER "mysql", srv "$srv",TABLE "t2"';
/* 3 */ insert into t1 values (42);
drop table t1, t2;

# when the user does not set the var, but sets the table option, the
# table option should take effect
SET session spider_read_only_mode = default;
create table t2 (c int);
eval create table t1 (c int) ENGINE=Spider COMMENT='read_only_mode "1", WRAPPER "mysql", srv "$srv",TABLE "t2"';
--error 12518
/* 4 */ insert into t1 values (42);
drop table t1, t2;

# when the user sets both var and table option, the table option
# should take precedence
set session spider_read_only_mode = 1;
create table t2 (c int);
eval create table t1 (c int) ENGINE=Spider COMMENT='read_only_mode "0", WRAPPER "mysql", srv "$srv",TABLE "t2"';
/* 5 */ insert into t1 values (42);
drop table t1, t2;

# when the user sets the var to -1 and sets the table option, the
# table option should take effect
SET session spider_read_only_mode = -1;
create table t2 (c int);
eval create table t1 (c int) ENGINE=Spider COMMENT='read_only_mode "1", WRAPPER "mysql", srv "$srv",TABLE "t2"';
--error 12518
/* 6 */ insert into t1 values (42);
drop table t1, t2;

eval drop server $srv;

SET session spider_read_only_mode = @old_spider_read_only_mode;
--disable_query_log
--disable_result_log
--source ../../t/test_deinit.inc
--enable_result_log
--enable_query_log
6 changes: 0 additions & 6 deletions storage/spider/spd_copy_tables.cc
Expand Up @@ -64,12 +64,6 @@ int spider_udf_set_copy_tables_param_default(
}
}

if (copy_tables->bulk_insert_interval == -1)
copy_tables->bulk_insert_interval = 10;
if (copy_tables->bulk_insert_rows == -1)
copy_tables->bulk_insert_rows = 100;
if (copy_tables->use_table_charset == -1)
copy_tables->use_table_charset = 1;
if (copy_tables->use_transaction == -1)
copy_tables->use_transaction = 1;
#ifndef WITHOUT_SPIDER_BG_SEARCH
Expand Down
4 changes: 1 addition & 3 deletions storage/spider/spd_db_conn.cc
Expand Up @@ -11559,7 +11559,7 @@ int spider_db_udf_copy_tables(
error_num = result->get_errno();
if (error_num == HA_ERR_END_OF_FILE)
{
if (roop_count < copy_tables->bulk_insert_rows)
if (roop_count < bulk_insert_rows)
{
end_of_file = TRUE;
if (roop_count)
Expand All @@ -11583,8 +11583,6 @@ int spider_db_udf_copy_tables(
pthread_mutex_unlock(&tmp_conn->mta_conn_mutex);
goto error_db_query;
}
bulk_insert_rows = spider_param_udf_ct_bulk_insert_rows(
copy_tables->bulk_insert_rows);
if (
select_ct->append_key_order_str(key_info, 0, FALSE) ||
select_ct->append_limit(0, bulk_insert_rows) ||
Expand Down
15 changes: 0 additions & 15 deletions storage/spider/spd_direct_sql.cc
Expand Up @@ -1451,25 +1451,10 @@ int spider_udf_set_direct_sql_param_default(
}
}

if (direct_sql->table_loop_mode == -1)
direct_sql->table_loop_mode = 0;
if (direct_sql->priority == -1)
direct_sql->priority = 1000000;
if (direct_sql->connect_timeout == -1)
direct_sql->connect_timeout = 6;
if (direct_sql->net_read_timeout == -1)
direct_sql->net_read_timeout = 600;
if (direct_sql->net_write_timeout == -1)
direct_sql->net_write_timeout = 600;
if (direct_sql->bulk_insert_rows == -1)
direct_sql->bulk_insert_rows = 3000;
if (direct_sql->connection_channel == -1)
direct_sql->connection_channel = 0;
#if MYSQL_VERSION_ID < 50500
#else
if (direct_sql->use_real_table == -1)
direct_sql->use_real_table = 0;
#endif
if (direct_sql->error_rw_mode == -1)
direct_sql->error_rw_mode = 0;
for (roop_count = 0; roop_count < direct_sql->table_count; roop_count++)
Expand Down

0 comments on commit e1d31a1

Please sign in to comment.