diff options
author | Dmitry Shulga <dmitry.shulga@mariadb.com> | 2021-02-25 14:20:11 +0700 |
---|---|---|
committer | Dmitry Shulga <dmitry.shulga@mariadb.com> | 2021-02-25 14:36:09 +0700 |
commit | 259e5243faa88370bbb890342326a324fb648f7d (patch) | |
tree | 0888261eb724acd28a923bf92331707ccf880c1d | |
parent | 0a95c922c8a2c1625ec6756b1e5c523e1cc4f73d (diff) | |
download | mariadb-git-259e5243faa88370bbb890342326a324fb648f7d.tar.gz |
MDEV-24860: Incorrect behaviour of SET STATEMENT in case it is executed as a prepared statement
Running statements with SET STATEMENT FOR clause is handled incorrectly in
case the whole statement is executed in prepared statement mode.
For example, running of the following statement
SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR CREATE TABLE t1 AS SELECT CONCAT('abc') AS c1;
results in different definition of the table t1 depending on whether
the statement is executed as a prepared or as a regular statement.
In first case the column c1 is defined as
`c1` varchar(3) DEFAULT NULL
in the last case the column c1 is defined as
`c1` varchar(3) NOT NULL
Different definition for the column c1 arise due to the fact that
a value of the data memeber Item_func_concat::maybe_null depends on
whether strict mode is on or off. Below is definition of the method
fix_fields() of the class Item_str_func that is base class for the
class Item_func_concat that is created on parsing the
SET STATEMENT FOR clause.
bool Item_str_func::fix_fields(THD *thd, Item **ref)
{
bool res= Item_func::fix_fields(thd, ref);
/*
In Item_str_func::check_well_formed_result() we may set null_value
flag on the same condition as in test() below.
*/
maybe_null= maybe_null || thd->is_strict_mode();
return res;
}
Although the clause SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR
is parsed on PREPARE phase during processing of the prepared statement,
real setting of the sql_mode system variable is done on EXECUTION phase.
On the other hand, the method Item_str_func::fix_fields is called on PREPARE
phase. In result, thd->is_strict_mode() returns true during calling the method
Item_str_func::fix_fields(), the data member maybe_null is assigned the value
true and column c1 is defined as DEFAULT NULL.
To fix the issue the system variables listed in the SET STATEMENT FOR clause
are set at the beginning of handling the PREPARE phase just right before
calling the function check_prepared_statement() and their original values
restored immediate after return from this function.
Additionally, to avoid code duplication the source code used in the function
mysql_execute_command for setting variables, specified by SET STATEMENT
clause, were extracted to the standalone functions
run_set_statement_if_requested(). This new function is called from
the function mysql_execute_command() and the method
Prepared_statement::prepare().
-rw-r--r-- | mysql-test/r/set_statement.result | 25 | ||||
-rw-r--r-- | mysql-test/t/set_statement.test | 23 | ||||
-rw-r--r-- | sql/sql_parse.cc | 267 | ||||
-rw-r--r-- | sql/sql_parse.h | 1 | ||||
-rw-r--r-- | sql/sql_prepare.cc | 16 |
5 files changed, 212 insertions, 120 deletions
diff --git a/mysql-test/r/set_statement.result b/mysql-test/r/set_statement.result index 845c0f21e3e..cd4859c32e8 100644 --- a/mysql-test/r/set_statement.result +++ b/mysql-test/r/set_statement.result @@ -1217,3 +1217,28 @@ set @rnd=1; select @rnd; @rnd 0 +# +# MDEV-24860: Incorrect behaviour of SET STATEMENT in case +# it is executed as a prepared statement +# +PREPARE stmt FROM "SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR CREATE TABLE t1 AS SELECT CONCAT('abc') AS c1"; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; +# Show definition of the table t1 created using Prepared Statement +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `c1` varchar(3) NOT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +DROP TABLE t1; +# Create the table t1 with the same definition as it used before +# using regular statement execution mode. +SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR CREATE TABLE t1 AS SELECT CONCAT('abc') AS c1; +# Show that the table has the same definition as it is in case the table +# created in prepared statement mode. +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `c1` varchar(3) NOT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +DROP TABLE t1; diff --git a/mysql-test/t/set_statement.test b/mysql-test/t/set_statement.test index 1c70ed6281a..a5f5c03098d 100644 --- a/mysql-test/t/set_statement.test +++ b/mysql-test/t/set_statement.test @@ -1136,3 +1136,26 @@ while ($1) --enable_query_log --echo # @rnd should be 0 select @rnd; + +--echo # +--echo # MDEV-24860: Incorrect behaviour of SET STATEMENT in case +--echo # it is executed as a prepared statement +--echo # +PREPARE stmt FROM "SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR CREATE TABLE t1 AS SELECT CONCAT('abc') AS c1"; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; + +--echo # Show definition of the table t1 created using Prepared Statement +SHOW CREATE TABLE t1; + +DROP TABLE t1; + +--echo # Create the table t1 with the same definition as it used before +--echo # using regular statement execution mode. +SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR CREATE TABLE t1 AS SELECT CONCAT('abc') AS c1; + +--echo # Show that the table has the same definition as it is in case the table +--echo # created in prepared statement mode. +SHOW CREATE TABLE t1; + +DROP TABLE t1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 131ba4a86c5..7f756245c83 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2995,6 +2995,146 @@ static bool do_execute_sp(THD *thd, sp_head *sp) /** + Check whether the SQL statement being processed is prepended by + SET STATEMENT clause and handle variables assignment if it is. + + @param thd thread handle + @param lex current lex + + @return false in case of success, true in case of error. +*/ + +bool run_set_statement_if_requested(THD *thd, LEX *lex) +{ + if (!lex->stmt_var_list.is_empty() && !thd->slave_thread) + { + Query_arena backup; + DBUG_PRINT("info", ("SET STATEMENT %d vars", lex->stmt_var_list.elements)); + + lex->old_var_list.empty(); + List_iterator_fast<set_var_base> it(lex->stmt_var_list); + set_var_base *var; + + if (lex->set_arena_for_set_stmt(&backup)) + return true; + + MEM_ROOT *mem_root= thd->mem_root; + while ((var= it++)) + { + DBUG_ASSERT(var->is_system()); + set_var *o= NULL, *v= (set_var*)var; + if (!v->var->is_set_stmt_ok()) + { + my_error(ER_SET_STATEMENT_NOT_SUPPORTED, MYF(0), v->var->name.str); + lex->reset_arena_for_set_stmt(&backup); + lex->old_var_list.empty(); + lex->free_arena_for_set_stmt(); + return true; + } + if (v->var->session_is_default(thd)) + o= new set_var(thd,v->type, v->var, &v->base, NULL); + else + { + switch (v->var->option.var_type & GET_TYPE_MASK) + { + case GET_BOOL: + case GET_INT: + case GET_LONG: + case GET_LL: + { + bool null_value; + longlong val= v->var->val_int(&null_value, thd, v->type, &v->base); + o= new set_var(thd, v->type, v->var, &v->base, + (null_value ? + (Item *) new (mem_root) Item_null(thd) : + (Item *) new (mem_root) Item_int(thd, val))); + } + break; + case GET_UINT: + case GET_ULONG: + case GET_ULL: + { + bool null_value; + ulonglong val= v->var->val_int(&null_value, thd, v->type, &v->base); + o= new set_var(thd, v->type, v->var, &v->base, + (null_value ? + (Item *) new (mem_root) Item_null(thd) : + (Item *) new (mem_root) Item_uint(thd, val))); + } + break; + case GET_DOUBLE: + { + bool null_value; + double val= v->var->val_real(&null_value, thd, v->type, &v->base); + o= new set_var(thd, v->type, v->var, &v->base, + (null_value ? + (Item *) new (mem_root) Item_null(thd) : + (Item *) new (mem_root) Item_float(thd, val, 1))); + } + break; + default: + case GET_NO_ARG: + case GET_DISABLED: + DBUG_ASSERT(0); + /* fall through */ + case 0: + case GET_FLAGSET: + case GET_ENUM: + case GET_SET: + case GET_STR: + case GET_STR_ALLOC: + { + char buff[STRING_BUFFER_USUAL_SIZE]; + String tmp(buff, sizeof(buff), v->var->charset(thd)),*val; + val= v->var->val_str(&tmp, thd, v->type, &v->base); + if (val) + { + Item_string *str= + new (mem_root) Item_string(thd, v->var->charset(thd), + val->ptr(), val->length()); + o= new set_var(thd, v->type, v->var, &v->base, str); + } + else + o= new set_var(thd, v->type, v->var, &v->base, + new (mem_root) Item_null(thd)); + } + break; + } + } + DBUG_ASSERT(o); + lex->old_var_list.push_back(o, thd->mem_root); + } + lex->reset_arena_for_set_stmt(&backup); + + if (lex->old_var_list.is_empty()) + lex->free_arena_for_set_stmt(); + + if (thd->is_error() || + sql_set_variables(thd, &lex->stmt_var_list, false)) + { + if (!thd->is_error()) + my_error(ER_WRONG_ARGUMENTS, MYF(0), "SET"); + lex->restore_set_statement_var(); + return true; + } + /* + The value of last_insert_id is remembered in THD to be written to binlog + when it's used *the first time* in the statement. But SET STATEMENT + must read the old value of last_insert_id to be able to restore it at + the end. This should not count at "reading of last_insert_id" and + should not remember last_insert_id for binlog. That is, it should clear + stmt_depends_on_first_successful_insert_id_in_prev_stmt flag. + */ + if (!thd->in_sub_stmt) + { + thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; + } + } + return false; +} + + +/** Execute command saved in thd and lex->sql_command. @param thd Thread handle @@ -3253,126 +3393,13 @@ mysql_execute_command(THD *thd) thd->get_binlog_format(&orig_binlog_format, &orig_current_stmt_binlog_format); - if (!lex->stmt_var_list.is_empty() && !thd->slave_thread) - { - Query_arena backup; - DBUG_PRINT("info", ("SET STATEMENT %d vars", lex->stmt_var_list.elements)); - - lex->old_var_list.empty(); - List_iterator_fast<set_var_base> it(lex->stmt_var_list); - set_var_base *var; - - if (lex->set_arena_for_set_stmt(&backup)) - goto error; - - MEM_ROOT *mem_root= thd->mem_root; - while ((var= it++)) - { - DBUG_ASSERT(var->is_system()); - set_var *o= NULL, *v= (set_var*)var; - if (!v->var->is_set_stmt_ok()) - { - my_error(ER_SET_STATEMENT_NOT_SUPPORTED, MYF(0), v->var->name.str); - lex->reset_arena_for_set_stmt(&backup); - lex->old_var_list.empty(); - lex->free_arena_for_set_stmt(); - goto error; - } - if (v->var->session_is_default(thd)) - o= new set_var(thd,v->type, v->var, &v->base, NULL); - else - { - switch (v->var->option.var_type & GET_TYPE_MASK) - { - case GET_BOOL: - case GET_INT: - case GET_LONG: - case GET_LL: - { - bool null_value; - longlong val= v->var->val_int(&null_value, thd, v->type, &v->base); - o= new set_var(thd, v->type, v->var, &v->base, - (null_value ? - (Item *) new (mem_root) Item_null(thd) : - (Item *) new (mem_root) Item_int(thd, val))); - } - break; - case GET_UINT: - case GET_ULONG: - case GET_ULL: - { - bool null_value; - ulonglong val= v->var->val_int(&null_value, thd, v->type, &v->base); - o= new set_var(thd, v->type, v->var, &v->base, - (null_value ? - (Item *) new (mem_root) Item_null(thd) : - (Item *) new (mem_root) Item_uint(thd, val))); - } - break; - case GET_DOUBLE: - { - bool null_value; - double val= v->var->val_real(&null_value, thd, v->type, &v->base); - o= new set_var(thd, v->type, v->var, &v->base, - (null_value ? - (Item *) new (mem_root) Item_null(thd) : - (Item *) new (mem_root) Item_float(thd, val, 1))); - } - break; - default: - case GET_NO_ARG: - case GET_DISABLED: - DBUG_ASSERT(0); - case 0: - case GET_FLAGSET: - case GET_ENUM: - case GET_SET: - case GET_STR: - case GET_STR_ALLOC: - { - char buff[STRING_BUFFER_USUAL_SIZE]; - String tmp(buff, sizeof(buff), v->var->charset(thd)),*val; - val= v->var->val_str(&tmp, thd, v->type, &v->base); - if (val) - { - Item_string *str= new (mem_root) Item_string(thd, v->var->charset(thd), - val->ptr(), val->length()); - o= new set_var(thd, v->type, v->var, &v->base, str); - } - else - o= new set_var(thd, v->type, v->var, &v->base, - new (mem_root) Item_null(thd)); - } - break; - } - } - DBUG_ASSERT(o); - lex->old_var_list.push_back(o, thd->mem_root); - } - lex->reset_arena_for_set_stmt(&backup); - if (lex->old_var_list.is_empty()) - lex->free_arena_for_set_stmt(); - if (thd->is_error() || - (res= sql_set_variables(thd, &lex->stmt_var_list, false))) - { - if (!thd->is_error()) - my_error(ER_WRONG_ARGUMENTS, MYF(0), "SET"); - lex->restore_set_statement_var(); - goto error; - } - /* - The value of last_insert_id is remembered in THD to be written to binlog - when it's used *the first time* in the statement. But SET STATEMENT - must read the old value of last_insert_id to be able to restore it at - the end. This should not count at "reading of last_insert_id" and - should not remember last_insert_id for binlog. That is, it should clear - stmt_depends_on_first_successful_insert_id_in_prev_stmt flag. - */ - if (!thd->in_sub_stmt) - { - thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; - } - } + /* + Assign system variables with values specified by the clause + SET STATEMENT var1=value1 [, var2=value2, ...] FOR <statement> + if they are any. + */ + if (run_set_statement_if_requested(thd, lex)) + goto error; if (thd->lex->mi.connection_name.str == NULL) thd->lex->mi.connection_name= thd->variables.default_master_connection; diff --git a/sql/sql_parse.h b/sql/sql_parse.h index ce5953696a2..968fa223e1e 100644 --- a/sql/sql_parse.h +++ b/sql/sql_parse.h @@ -101,6 +101,7 @@ void mysql_init_multi_delete(LEX *lex); bool multi_delete_set_locks_and_link_aux_tables(LEX *lex); void create_table_set_open_action_and_adjust_tables(LEX *lex); pthread_handler_t handle_bootstrap(void *arg); +bool run_set_statement_if_requested(THD *thd, LEX *lex); int mysql_execute_command(THD *thd); bool do_command(THD *thd); void do_handle_bootstrap(THD *thd); diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 3dff2cb8106..fa335465f02 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -4249,6 +4249,16 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) */ MDL_savepoint mdl_savepoint= thd->mdl_context.mdl_savepoint(); + /* + Set variables specified by + SET STATEMENT var1=value1 [, var2=value2, ...] FOR <statement> + clause for duration of prepare phase. Original values of variable + listed in the SET STATEMENT clause is restored right after return + from the function check_prepared_statement() + */ + if (likely(error == 0)) + error= run_set_statement_if_requested(thd, lex); + /* The only case where we should have items in the thd->free_list is after stmt->set_params_from_vars(), which may in some cases create @@ -4267,6 +4277,12 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) lex->context_analysis_only&= ~CONTEXT_ANALYSIS_ONLY_PREPARE; } + /* + Restore original values of variables modified on handling + SET STATEMENT clause. + */ + thd->lex->restore_set_statement_var(); + /* The order is important */ lex->unit.cleanup(); |