maria-developers team mailing list archive
-
maria-developers team
-
Mailing list archive
-
Message #07384
Re: Rev 3986: MDEV-5231: Per query variables from Percona Server
Hi, Sanja!
On May 22, sanja@xxxxxxxxxxxx wrote:
> === added file 'mysql-test/suite/percona/percona_rpl_stm_per_query_variables_settings.test'
> --- a/mysql-test/suite/percona/percona_rpl_stm_per_query_variables_settings.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/percona/percona_rpl_stm_per_query_variables_settings.test 2014-05-22 12:50:01 +0000
could you rename tests to have shorter names?
rpl_set_statement looks good enough to me
> @@ -0,0 +1,57 @@
> +--source include/master-slave.inc
> +--source include/have_binlog_format_statement.inc
> +
> +--disable_warnings
> +DROP TABLE IF EXISTS t1;
> +DROP TABLE IF EXISTS t2;
> +--enable_warnings
> +
> +call mtr.add_suppression("Unsafe statement written to the binary log*");
> +CREATE TABLE t1 (a bigint unsigned not null);
> +CREATE TABLE t2 (a char(255) not null);
okay, although I wouldn't bother creating *two* tables,
I'd create one and hard-coded it into percona_rpl_set_statement_variable_test.inc
(removing rpl_ssvt_table parameter)
> +
> +--echo
> +--echo There are the following types of variables:
> +--echo 1) variables that are NOT replicated correctly when using STATEMENT mode;
> +--echo
> +
> +--let $rpl_ssvt_var_name=max_join_size
> +--let $rpl_ssvt_var_value=2
> +--let $rpl_ssvt_table=t1
> +--source suite/percona/percona_rpl_set_statement_variable_test.inc
> +
> +--echo
> +--echo 2) variables thar ARE replicated correctly
> +--echo They must be replicated correctly with "SET STATEMENT" too.
> +--echo
> +--let $rpl_ssvt_var_name=auto_increment_increment
> +--let $rpl_ssvt_var_value=10
> +--let $rpl_ssvt_table=t1
> +--source suite/percona/percona_rpl_set_statement_variable_test.inc
> +
> +--echo
> +--echo 3) sql_mode which is replicated correctly exept NO_DIR_IN_CREATE value;
> +--echo
> +--let $rpl_ssvt_var_name=sql_mode
> +--let $rpl_ssvt_var_value='ERROR_FOR_DIVISION_BY_ZERO'
> +--let $rpl_ssvt_table=t2
> +--source suite/percona/percona_rpl_set_statement_variable_test.inc
> +--let $rpl_ssvt_var_name=sql_mode
> +--let $rpl_ssvt_var_value='NO_DIR_IN_CREATE'
> +--let $rpl_ssvt_table=t2
> +--source suite/percona/percona_rpl_set_statement_variable_test.inc
> +
> +--echo
> +--echo 4) variables that are not replicated at all:
> +--echo default_storage_engine, storage_engine, max_heap_table_size
> +--echo
> +--let $rpl_ssvt_var_name=max_heap_table_size
> +--let $rpl_ssvt_var_value=16384
> +--let $rpl_ssvt_table=t1
> +--source suite/percona/percona_rpl_set_statement_variable_test.inc
> +
> +connection master;
> +DROP TABLE t1;
> +DROP TABLE t2;
> +sync_slave_with_master;
> +source include/stop_slave.inc;
>
> === added file 'mysql-test/suite/percona/percona_statement_set.result'
> --- a/mysql-test/suite/percona/percona_statement_set.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/percona/percona_statement_set.result 2014-05-22 12:50:01 +0000
reverse polish notation in percona someone loves.
starwars too much seen he apparently.
Please, rename to "set_statement.result"
> @@ -0,0 +1,875 @@
> +'# SET STATEMENT ..... FOR .... TEST'
> +DROP TABLE IF EXISTS t1;
> +DROP FUNCTION IF EXISTS myProc;
> +DROP PROCEDURE IF EXISTS p1;
> +DROP PROCEDURE IF EXISTS p2;
> +DROP PROCEDURE IF EXISTS p3;
> +DROP PROCEDURE IF EXISTS p4;
> +DROP PROCEDURE IF EXISTS p5;
> +DROP TABLE IF EXISTS STATEMENT;
> +'# Setup database'
> +CREATE TABLE t1 (v1 INT, v2 INT);
> +INSERT INTO t1 VALUES (1,2);
> +INSERT INTO t1 VALUES (3,4);
> +''
> +'#------------------ STATEMENT Test 1 -----------------------#'
> +'# Initialize variables to known setting'
> +SET SESSION sort_buffer_size=100000;
> +''
> +'# Pre-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +Variable_name Value
> +sort_buffer_size 100000
> +SET STATEMENT sort_buffer_size=150000 FOR SELECT * FROM t1;
> +v1 v2
> +1 2
> +3 4
> +''
> +'# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +Variable_name Value
> +sort_buffer_size 100000
> +''
> +'#------------------ STATEMENT Test 2 -----------------------#'
> +'# Initialize variables to known setting'
> +SET SESSION binlog_format=mixed;
> +SET SESSION sort_buffer_size=100000;
> +'# Pre-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +Variable_name Value
> +sort_buffer_size 100000
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +Variable_name Value
> +binlog_format MIXED
> +SET STATEMENT sort_buffer_size=150000, binlog_format=row
> +FOR SELECT * FROM t1;
> +v1 v2
> +1 2
> +3 4
> +'# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +Variable_name Value
> +sort_buffer_size 100000
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +Variable_name Value
> +binlog_format MIXED
> +''
> +'#------------------ STATEMENT Test 3 -----------------------#'
> +'# set initial variable value, make prepared statement
> +SET SESSION binlog_format=row;
> +PREPARE stmt1 FROM 'SET STATEMENT binlog_format=row FOR SELECT * FROM t1';
> +''
> +'# Change variable setting'
> +SET SESSION binlog_format=mixed;
> +''
> +'# Pre-STATEMENT variable value'
> +''
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +Variable_name Value
> +binlog_format MIXED
> +''
> +EXECUTE stmt1;
> +v1 v2
> +1 2
> +3 4
> +''
> +'# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +Variable_name Value
> +binlog_format MIXED
> +''
> +DEALLOCATE PREPARE stmt1;
> +'#------------------ STATEMENT Test 4 -----------------------#'
> +'# set initial variable value, make prepared statement
> +SET SESSION myisam_sort_buffer_size=500000, myisam_repair_threads=1;
> +''
> +'# Pre-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'myisam_sort_buffer_size';
> +Variable_name Value
> +myisam_sort_buffer_size 500000
> +SHOW SESSION VARIABLES LIKE 'myisam_repair_threads';
> +Variable_name Value
> +myisam_repair_threads 1
> +''
> +SET STATEMENT myisam_sort_buffer_size=800000,
> +myisam_repair_threads=2 FOR OPTIMIZE TABLE t1;
how comes that is "make prepared statement" (see comment above) ?
> +Table Op Msg_type Msg_text
> +test.t1 optimize status OK
> +''
> +'# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'myisam_sort_buffer_size';
> +Variable_name Value
> +myisam_sort_buffer_size 500000
> +SHOW SESSION VARIABLES LIKE 'myisam_repair_threads';
> +Variable_name Value
> +myisam_repair_threads 1
> +''
...
> === added file 'mysql-test/suite/percona/percona_statement_set.test'
> --- a/mysql-test/suite/percona/percona_statement_set.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/percona/percona_statement_set.test 2014-05-22 12:50:01 +0000
> @@ -0,0 +1,847 @@
> +--echo '# SET STATEMENT ..... FOR .... TEST'
many of the tests below are pretty useless. they don't test that the
value is actually set. See below:
> +############################ STATEMENT_SET #############################
> +# #
> +# Testing working functionality of SET STATEMENT #
> +# #
> +# #
> +# There is important documentation within #
> +# #
> +# #
> +# Author: Joe Lukas #
> +# Creation: #
> +# 2009-08-02 Implement this test as part of #
> +# WL#681 Per query variable settings #
> +# #
> +########################################################################
> +
> +--disable_warnings
> +DROP TABLE IF EXISTS t1;
> +DROP FUNCTION IF EXISTS myProc;
> +DROP PROCEDURE IF EXISTS p1;
> +DROP PROCEDURE IF EXISTS p2;
> +DROP PROCEDURE IF EXISTS p3;
> +DROP PROCEDURE IF EXISTS p4;
> +DROP PROCEDURE IF EXISTS p5;
> +DROP TABLE IF EXISTS STATEMENT;
> +--enable_warnings
> +####################################################################
> +#Set up current database
> +####################################################################
> +--echo '# Setup database'
> +CREATE TABLE t1 (v1 INT, v2 INT);
> +INSERT INTO t1 VALUES (1,2);
> +INSERT INTO t1 VALUES (3,4);
> +--echo ''
> +--echo '#------------------ STATEMENT Test 1 -----------------------#'
> +####################################################################
> +# Checks with variable value type ulong #
> +####################################################################
> +--echo '# Initialize variables to known setting'
> +SET SESSION sort_buffer_size=100000;
> +--echo ''
> +--echo '# Pre-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +SET STATEMENT sort_buffer_size=150000 FOR SELECT * FROM t1;
How do you know that SET STATEMENT did anything at all?
> +--echo ''
> +--echo '# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +--echo ''
> +--echo '#------------------ STATEMENT Test 2 -----------------------#'
> +####################################################################
> +# Checks for multiple set values inside STATEMENT ... FOR #
> +####################################################################
> +--echo '# Initialize variables to known setting'
> +SET SESSION binlog_format=mixed;
> +SET SESSION sort_buffer_size=100000;
> +--echo '# Pre-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +SET STATEMENT sort_buffer_size=150000, binlog_format=row
> + FOR SELECT * FROM t1;
> +--echo '# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'sort_buffer_size';
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +
> +--echo ''
> +--echo '#------------------ STATEMENT Test 3 -----------------------#'
> +####################################################################
> +# Check current variable value is stored in using stored #
> +# statements. #
> +####################################################################
> +--echo '# set initial variable value, make prepared statement
> +SET SESSION binlog_format=row;
> +PREPARE stmt1 FROM 'SET STATEMENT binlog_format=row FOR SELECT * FROM t1';
How will you know that SET STATEMENT did anything at all?
(the same applies to most tests below, I'll stop repeating it)
> +--echo ''
> +--echo '# Change variable setting'
> +SET SESSION binlog_format=mixed;
> +--echo ''
> +--echo '# Pre-STATEMENT variable value'
> +--echo ''
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +--echo ''
> +EXECUTE stmt1;
> +--echo ''
> +--echo '# Post-STATEMENT variable value'
> +SHOW SESSION VARIABLES LIKE 'binlog_format';
> +
> +--echo ''
> +DEALLOCATE PREPARE stmt1;
...
> === modified file 'sql/share/errmsg-utf8.txt'
> --- a/sql/share/errmsg-utf8.txt 2014-02-11 13:21:48 +0000
> +++ b/sql/share/errmsg-utf8.txt 2014-05-22 12:50:01 +0000
> @@ -7067,3 +7067,5 @@ ER_IT_IS_A_VIEW 42S02
> eng "'%-.192s' is a view"
> ER_SLAVE_SKIP_NOT_IN_GTID
> eng "When using GTID, @@sql_slave_skip_counter can not be used. Instead, setting @@gtid_slave_pos explicitly can be used to skip to after a given GTID position."
> +ER_SET_STATEMENT_TABLES_IS_NOT_SUPPORTED
> + eng "SET STATEMENT does not support using tables in the assignment variables expressions"
why not?
>
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc 2014-03-29 10:33:25 +0000
> +++ b/sql/sql_parse.cc 2014-05-22 12:50:01 +0000
> @@ -2430,6 +2432,33 @@ mysql_execute_command(THD *thd)
> thd->get_binlog_format(&orig_binlog_format,
> &orig_current_stmt_binlog_format);
>
> + if (lex->set_statement && !lex->var_list.is_empty())
Hmm? No separate list of variables, but a common lex->var_list?
Then the following won't work:
SET STATEMENT A=1 SET B=2
I remember in the WL and in GSoC we agreed that for
SET STATEMENT A=2 SET A=2
(same variable in SET STATEMENT and SET), it's acceptable to restore the
original value of A, basically ignoring SET.
But I doubt that the same logic applies when SET STATEMENT and SET
modify *different* variables.
> + {
> + MEM_ROOT *save_mem_root;
> + /*
> + We should be sure that variables backup and assignment made on
> + runtime memory
> + */
> + save_mem_root= thd->mem_root;
> + thd->mem_root= thd->get_execution_mem_root();
Why?
> + per_query_variables_backup= copy_system_variables(&thd->variables);
> + if (!per_query_variables_backup ||
> + (res= sql_set_variables(thd, &lex->var_list)))
> + {
> + thd->mem_root= save_mem_root;
> + /*
> + We encountered some sort of error, but no message was sent.
> + Send something semi-generic here since we don't know which
> + assignment in the list caused the error.
> + */
> + if (!thd->is_error())
> + my_error(ER_WRONG_ARGUMENTS, MYF(0), "SET");
> + goto error;
> + }
> + thd->mem_root= save_mem_root;
> + }
> +
> /*
> Force statement logging for DDL commands to allow us to update
> privilege, system or statistic tables directly without the updates
> @@ -5144,6 +5179,30 @@ finish:
>
> lex->unit.cleanup();
>
> + if (lex->set_statement && !lex->var_list.is_empty()) {
> + List_iterator_fast<set_var_base> it(thd->lex->var_list);
> + set_var *var;
> +
> + free_system_variables(&thd->variables);
> + thd->variables= *per_query_variables_backup;
> + /* per_query_variables_backup was in a mem_root */
> + per_query_variables_backup= NULL;
> + /*
> + When variables are restored after "SET STATEMENT ... FOR ..." statement
> + execution an update callback must be invoked for the system variables
> + to save special logic if it is. set_var_base class does not contain
> + refference to variable as it is just an interface class. But only
> + system variables are allowed to be used in "SET STATEMENT ... FOR ..."
> + statement, so cast from set_var_base* to set_var* can be used here.
> + */
> + while ((var=(set_var *)it++))
> + {
> + var->var->stmt_update(thd);
> + }
I don't think it'll work. Compare set_var::update() and
set_var::stmt_update(). The former calls ::session_update(),
while the latter does not. Normally, session_update() simply updates the
value, performs the assignment. And it is, indeed, not needed here.
but in some custom classes session_update() is more complex, and it's not
safe to omit it. Please
1) see all non-trivial implementations of sys_var::session_update()
2) write SET SESSION tests for these variables
3) if they'll all work - please explain why
4) if they won't - I'm afraid, this whole approach needs to be redone, then
> +
> + thd->lex->set_statement= false;
> + }
> +
> if (! thd->in_sub_stmt)
> {
> if (thd->killed != NOT_KILLED)
>
> === modified file 'sql/sql_plugin.cc'
> --- a/sql/sql_plugin.cc 2014-03-26 21:25:38 +0000
> +++ b/sql/sql_plugin.cc 2014-05-22 12:50:01 +0000
> @@ -4088,3 +4088,48 @@ static void restore_ptr_backup(uint n, s
> (backup++)->restore();
> }
>
> +
> +/**
> + Create deep copy of system_variables instance.
> +*/
> +struct system_variables *
> +copy_system_variables(const struct system_variables *src)
What is it doing in sql_plugin.cc?
> +{
> + struct system_variables *dst;
> +
> + DBUG_ASSERT(src);
> +
> + dst= (struct system_variables *)
> + sql_alloc(sizeof(struct system_variables));
> + if (!dst)
> + return NULL;
> + *dst= *src;
> +
> + if (dst->dynamic_variables_ptr)
> + {
> + if (!(dst->dynamic_variables_ptr=
> + (char *)my_malloc(dst->dynamic_variables_size, MYF(MY_WME | MY_FAE))))
> + return NULL;
> + memcpy(dst->dynamic_variables_ptr,
> + src->dynamic_variables_ptr,
> + src->dynamic_variables_size);
> + }
> +
> + mysql_mutex_lock(&LOCK_plugin);
> + dst->table_plugin=
> + intern_plugin_lock(NULL, src->table_plugin);
I'm surprised that plugin locking actually works in here,
(I believe it does, otherwise you'd noticed it).
What about SET STATEMENT default_engine=MEMORY ?
What about SET STATEMENT default_engine=MyISAM (where MyISAM is already default)?
please, try both in debug build.
> + mysql_mutex_unlock(&LOCK_plugin);
> +
> + return dst;
> +}
> +
> +void free_system_variables(struct system_variables *v)
> +{
> + DBUG_ASSERT(v);
> +
> + mysql_mutex_lock(&LOCK_plugin);
> + intern_plugin_unlock(NULL, v->table_plugin);
> + mysql_mutex_unlock(&LOCK_plugin);
> +
> + my_free(v->dynamic_variables_ptr);
> +}
>
> === modified file 'sql/sql_yacc.yy'
> --- a/sql/sql_yacc.yy 2014-03-26 21:25:38 +0000
> +++ b/sql/sql_yacc.yy 2014-05-22 12:50:01 +0000
> @@ -813,8 +813,10 @@ static void sp_create_assignment_lex(THD
> lex->one_shot_set= 0;
> lex->autocommit= 0;
> /* get_ptr() is only correct with no lookahead. */
> - DBUG_ASSERT(no_lookahead);
> + if (no_lookahead)
> lex->sphead->m_tmp_query= lip->get_ptr();
> + else
> + lex->sphead->m_tmp_query= lip->get_tok_end();
Why?
> /* Inherit from outer lex. */
> lex->option_type= old_lex->option_type;
> }
> @@ -955,10 +957,10 @@ bool my_yyoverflow(short **a, YYSTYPE **
> %parse-param { THD *thd }
> %lex-param { THD *thd }
> /*
> - Currently there are 163 shift/reduce conflicts.
> + Currently there are 164 shift/reduce conflicts.
> We should not introduce new conflicts any more.
> */
> -%expect 163
> +%expect 164
Why?
>
> /*
> Comments for TOKENS.
> @@ -14427,8 +14431,44 @@ set:
> }
> start_option_value_list
> {}
> + | SET STATEMENT_SYM
> + {
> + LEX *lex= Lex;
> + mysql_init_select(lex);
> + lex->sql_command= SQLCOM_SET_OPTION;
> + /* Don't clear var_list in the case of recursive statement */
> + if (!lex->set_statement)
> + lex->var_list.empty();
When var_list is not empty here (and set_statement not set)?
In other words, when this statement will actually have any effect?
> + lex->one_shot_set= 0;
> + lex->autocommit= 0;
> + lex->set_statement= true;
> + sp_head *sp= lex->sphead;
> + if (sp && !sp->is_invoked())
> + {
> + sp->m_param_begin= ((yychar == YYEMPTY) ? YYLIP->get_ptr() : YYLIP->get_tok_start());
> + sp->m_param_end= ((yychar == YYEMPTY) ? YYLIP->get_ptr() : YYLIP->get_tok_end());
Why?
> + }
> + }
> + set_stmt_option_value_following_option_type_list
Eh? What does "set stmt option value following option type list" mean?
Please, rename to set_stmt_option_value_list or even set_stmt_list
> + {
> + if (Lex->query_tables)
> + {
> + my_error(ER_SET_STATEMENT_TABLES_IS_NOT_SUPPORTED, MYF(0));
> + MYSQL_YYABORT;
> + }
> + }
> + FOR_SYM statement
> + {}
> ;
>
> +set_stmt_option_value_following_option_type_list:
> + /*
> + Only system variables can be used here. If this condition is changed
> + please check careful code under lex->option_type == OPT_STATEMENT
> + condition on wrong type casts.
> + */
> + option_value_following_option_type
> + | set_stmt_option_value_following_option_type_list ',' option_value_following_option_type
>
> // Start of option value list
> start_option_value_list:
Regards,
Sergei
