From 259e5243faa88370bbb890342326a324fb648f7d Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Thu, 25 Feb 2021 14:20:11 +0700 Subject: [PATCH 01/16] 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(). --- mysql-test/r/set_statement.result | 25 +++ mysql-test/t/set_statement.test | 23 +++ sql/sql_parse.cc | 267 ++++++++++++++++-------------- sql/sql_parse.h | 1 + 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 @@ -2994,6 +2994,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 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. @@ -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 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 + 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 + 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(); From 1d80e8e4f31b2e361241973467070975f6d80136 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 25 Feb 2021 11:55:08 +0200 Subject: [PATCH 02/16] MENT-1098 Crash during update on 10.4.17 after upgrade from 10.4.10 The reason for the crash was that there was not a write lock to protect against file rotations in the server_audit plugin after an audit plugin patch to changed audit mutexes to read & write locks. The fixes are: * Moving server_audit.c to use read & write locks (which improves performance). * Added functionality in file_logger.c to not do file rotations until it is allowed by the caller (done without any interface changes for the logging service). * Move checking of file size limit to server_audit.c and if it is time to do a rotation change the read lock to a write lock and tell file_logger that it is now allowed to rotate the log files. --- mysys/file_logger.c | 55 +++++++++------ plugin/server_audit/server_audit.c | 104 +++++++++++++++++------------ 2 files changed, 97 insertions(+), 62 deletions(-) diff --git a/mysys/file_logger.c b/mysys/file_logger.c index 71394be7afc..476ed44089e 100644 --- a/mysys/file_logger.c +++ b/mysys/file_logger.c @@ -150,23 +150,34 @@ exit: } +/* + Return 1 if we should rotate the log +*/ + +my_bool logger_time_to_rotate(LOGGER_HANDLE *log) +{ + my_off_t filesize; + if (log->rotations > 0 && + (filesize= my_tell(log->file, MYF(0))) != (my_off_t) -1 && + ((ulonglong) filesize >= log->size_limit)) + return 1; + return 0; +} + + int logger_vprintf(LOGGER_HANDLE *log, const char* fmt, va_list ap) { int result; - my_off_t filesize; char cvtbuf[1024]; size_t n_bytes; flogger_mutex_lock(&log->lock); - if (log->rotations > 0) - if ((filesize= my_tell(log->file, MYF(0))) == (my_off_t) -1 || - ((unsigned long long)filesize >= log->size_limit && - do_rotate(log))) - { - result= -1; - errno= my_errno; - goto exit; /* Log rotation needed but failed */ - } + if (logger_time_to_rotate(log) && do_rotate(log)) + { + result= -1; + errno= my_errno; + goto exit; /* Log rotation needed but failed */ + } n_bytes= my_vsnprintf(cvtbuf, sizeof(cvtbuf), fmt, ap); if (n_bytes >= sizeof(cvtbuf)) @@ -180,21 +191,18 @@ exit: } -int logger_write(LOGGER_HANDLE *log, const char *buffer, size_t size) +static int logger_write_r(LOGGER_HANDLE *log, my_bool allow_rotations, + const char *buffer, size_t size) { int result; - my_off_t filesize; flogger_mutex_lock(&log->lock); - if (log->rotations > 0) - if ((filesize= my_tell(log->file, MYF(0))) == (my_off_t) -1 || - ((unsigned long long)filesize >= log->size_limit && - do_rotate(log))) - { - result= -1; - errno= my_errno; - goto exit; /* Log rotation needed but failed */ - } + if (allow_rotations && logger_time_to_rotate(log) && do_rotate(log)) + { + result= -1; + errno= my_errno; + goto exit; /* Log rotation needed but failed */ + } result= (int)my_write(log->file, (uchar *) buffer, size, MYF(0)); @@ -204,6 +212,11 @@ exit: } +int logger_write(LOGGER_HANDLE *log, const char *buffer, size_t size) +{ + return logger_write_r(log, TRUE, buffer, size); +} + int logger_rotate(LOGGER_HANDLE *log) { int result; diff --git a/plugin/server_audit/server_audit.c b/plugin/server_audit/server_audit.c index cb6b0586524..f6661772df1 100644 --- a/plugin/server_audit/server_audit.c +++ b/plugin/server_audit/server_audit.c @@ -15,7 +15,7 @@ #define PLUGIN_VERSION 0x104 -#define PLUGIN_STR_VERSION "1.4.10" +#define PLUGIN_STR_VERSION "1.4.11" #define _my_thread_var loc_thread_var @@ -139,6 +139,7 @@ static int loc_file_errno; #define logger_write loc_logger_write #define logger_rotate loc_logger_rotate #define logger_init_mutexts loc_logger_init_mutexts +#define logger_time_to_rotate loc_logger_time_to_rotate static size_t loc_write(File Filedes, const uchar *Buffer, size_t Count) @@ -553,22 +554,22 @@ static struct st_mysql_show_var audit_status[]= {0,0,0} }; -#if defined(HAVE_PSI_INTERFACE) && !defined(FLOGGER_NO_PSI) -/* These belong to the service initialization */ +#ifdef HAVE_PSI_INTERFACE static PSI_mutex_key key_LOCK_operations; -static PSI_mutex_key key_LOCK_atomic; -static PSI_mutex_key key_LOCK_bigbuffer; static PSI_mutex_info mutex_key_list[]= { { &key_LOCK_operations, "SERVER_AUDIT_plugin::lock_operations", - PSI_FLAG_GLOBAL}, + PSI_FLAG_GLOBAL} +#ifndef FLOGGER_NO_PSI + , { &key_LOCK_atomic, "SERVER_AUDIT_plugin::lock_atomic", PSI_FLAG_GLOBAL}, { &key_LOCK_bigbuffer, "SERVER_AUDIT_plugin::lock_bigbuffer", PSI_FLAG_GLOBAL} +#endif /*FLOGGER_NO_PSI*/ }; -#endif -static mysql_mutex_t lock_operations; +#endif /*HAVE_PSI_INTERFACE*/ +static mysql_prlock_t lock_operations; static mysql_mutex_t lock_atomic; static mysql_mutex_t lock_bigbuffer; @@ -1319,19 +1320,41 @@ static void change_connection(struct connection_info *cn, event->ip, event->ip_length); } +/* + Write to the log + + @param take_lock If set, take a read lock (or write lock on rotate). + If not set, the caller has a already taken a write lock +*/ + static int write_log(const char *message, size_t len, int take_lock) { int result= 0; if (take_lock) - flogger_mutex_lock(&lock_operations); + { + /* Start by taking a read lock */ + mysql_prlock_rdlock(&lock_operations); + } if (output_type == OUTPUT_FILE) { - if (logfile && - (is_active= (logger_write(logfile, message, len) == (int) len))) - goto exit; - ++log_write_failures; - result= 1; + if (logfile) + { + my_bool allow_rotate= !take_lock; /* Allow rotate if caller write lock */ + if (take_lock && logger_time_to_rotate(logfile)) + { + /* We have to rotate the log, change above read lock to write lock */ + mysql_prlock_unlock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); + allow_rotate= 1; + } + if (!(is_active= (logger_write_r(logfile, allow_rotate, message, len) == + (int) len))) + { + ++log_write_failures; + result= 1; + } + } } else if (output_type == OUTPUT_SYSLOG) { @@ -1339,9 +1362,8 @@ static int write_log(const char *message, size_t len, int take_lock) syslog_priority_codes[syslog_priority], "%s %.*s", syslog_info, (int) len, message); } -exit: if (take_lock) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); return result; } @@ -1589,7 +1611,7 @@ static int do_log_user(const char *name, int len, return 0; if (take_lock) - flogger_mutex_lock(&lock_operations); + mysql_prlock_rdlock(&lock_operations); if (incl_user_coll.n_users) { @@ -1605,7 +1627,7 @@ static int do_log_user(const char *name, int len, result= 1; if (take_lock) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); return result; } @@ -2496,11 +2518,11 @@ static int server_audit_init(void *p __attribute__((unused))) servhost_len= (uint)strlen(servhost); logger_init_mutexes(); -#if defined(HAVE_PSI_INTERFACE) && !defined(FLOGGER_NO_PSI) +#ifdef HAVE_PSI_INTERFACE if (PSI_server) PSI_server->register_mutex("server_audit", mutex_key_list, 1); #endif - flogger_mutex_init(key_LOCK_operations, &lock_operations, MY_MUTEX_INIT_FAST); + mysql_prlock_init(key_LOCK_operations, &lock_operations); flogger_mutex_init(key_LOCK_operations, &lock_atomic, MY_MUTEX_INIT_FAST); flogger_mutex_init(key_LOCK_operations, &lock_bigbuffer, MY_MUTEX_INIT_FAST); @@ -2588,7 +2610,7 @@ static int server_audit_deinit(void *p __attribute__((unused))) closelog(); (void) free(big_buffer); - flogger_mutex_destroy(&lock_operations); + mysql_prlock_destroy(&lock_operations); flogger_mutex_destroy(&lock_atomic); flogger_mutex_destroy(&lock_bigbuffer); @@ -2699,7 +2721,7 @@ static void update_file_path(MYSQL_THD thd, fprintf(stderr, "Log file name was changed to '%s'.\n", new_name); if (!maria_55_started || !debug_server_started) - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); if (logging) log_current_query(thd); @@ -2731,7 +2753,7 @@ static void update_file_path(MYSQL_THD thd, file_path= path_buffer; exit_func: if (!maria_55_started || !debug_server_started) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); ADD_ATOMIC(internal_stop_logging, -1); } @@ -2747,9 +2769,9 @@ static void update_file_rotations(MYSQL_THD thd __attribute__((unused)), if (!logging || output_type != OUTPUT_FILE) return; - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); logfile->rotations= rotations; - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); } @@ -2765,9 +2787,9 @@ static void update_file_rotate_size(MYSQL_THD thd __attribute__((unused)), if (!logging || output_type != OUTPUT_FILE) return; - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); logfile->size_limit= file_rotate_size; - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); } @@ -2812,7 +2834,7 @@ static void update_incl_users(MYSQL_THD thd, char *new_users= (*(char **) save) ? *(char **) save : empty_str; size_t new_len= strlen(new_users) + 1; if (!maria_55_started || !debug_server_started) - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); mark_always_logged(thd); if (new_len > sizeof(incl_user_buffer)) @@ -2826,7 +2848,7 @@ static void update_incl_users(MYSQL_THD thd, error_header(); fprintf(stderr, "server_audit_incl_users set to '%s'.\n", incl_users); if (!maria_55_started || !debug_server_started) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); } @@ -2837,7 +2859,7 @@ static void update_excl_users(MYSQL_THD thd __attribute__((unused)), char *new_users= (*(char **) save) ? *(char **) save : empty_str; size_t new_len= strlen(new_users) + 1; if (!maria_55_started || !debug_server_started) - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); mark_always_logged(thd); if (new_len > sizeof(excl_user_buffer)) @@ -2851,7 +2873,7 @@ static void update_excl_users(MYSQL_THD thd __attribute__((unused)), error_header(); fprintf(stderr, "server_audit_excl_users set to '%s'.\n", excl_users); if (!maria_55_started || !debug_server_started) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); } @@ -2864,7 +2886,7 @@ static void update_output_type(MYSQL_THD thd, return; ADD_ATOMIC(internal_stop_logging, 1); - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); if (logging) { log_current_query(thd); @@ -2878,7 +2900,7 @@ static void update_output_type(MYSQL_THD thd, if (logging) start_logging(); - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); ADD_ATOMIC(internal_stop_logging, -1); } @@ -2908,9 +2930,9 @@ static void update_syslog_priority(MYSQL_THD thd __attribute__((unused)), if (syslog_priority == new_priority) return; - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); mark_always_logged(thd); - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); error_header(); fprintf(stderr, "SysLog priority was changed from '%s' to '%s'.\n", syslog_priority_names[syslog_priority], @@ -2929,7 +2951,7 @@ static void update_logging(MYSQL_THD thd, ADD_ATOMIC(internal_stop_logging, 1); if (!maria_55_started || !debug_server_started) - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); if ((logging= new_logging)) { start_logging(); @@ -2946,7 +2968,7 @@ static void update_logging(MYSQL_THD thd, } if (!maria_55_started || !debug_server_started) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); ADD_ATOMIC(internal_stop_logging, -1); } @@ -2961,13 +2983,13 @@ static void update_mode(MYSQL_THD thd __attribute__((unused)), ADD_ATOMIC(internal_stop_logging, 1); if (!maria_55_started || !debug_server_started) - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); mark_always_logged(thd); error_header(); fprintf(stderr, "Logging mode was changed from %d to %d.\n", mode, new_mode); mode= new_mode; if (!maria_55_started || !debug_server_started) - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); ADD_ATOMIC(internal_stop_logging, -1); } @@ -2982,14 +3004,14 @@ static void update_syslog_ident(MYSQL_THD thd __attribute__((unused)), syslog_ident= syslog_ident_buffer; error_header(); fprintf(stderr, "SYSYLOG ident was changed to '%s'\n", syslog_ident); - flogger_mutex_lock(&lock_operations); + mysql_prlock_wrlock(&lock_operations); mark_always_logged(thd); if (logging && output_type == OUTPUT_SYSLOG) { stop_logging(); start_logging(); } - flogger_mutex_unlock(&lock_operations); + mysql_prlock_unlock(&lock_operations); } From 25ecf8ed4b4cbca69a9fa09c27bbd4e5c83fafe3 Mon Sep 17 00:00:00 2001 From: Alexey Botchkov Date: Fri, 26 Feb 2021 13:26:00 +0400 Subject: [PATCH 03/16] MDEV-24965 With ALTER USER ...IDENTIFIED BY command, password doesn't replaced by asterisks in audit log. Check for the ALTER USER command added. --- mysql-test/suite/plugins/r/server_audit.result | 3 +++ mysql-test/suite/plugins/t/server_audit.test | 1 + plugin/server_audit/server_audit.c | 3 +++ 3 files changed, 7 insertions(+) diff --git a/mysql-test/suite/plugins/r/server_audit.result b/mysql-test/suite/plugins/r/server_audit.result index 3fce3346f29..ff22cdff8d6 100644 --- a/mysql-test/suite/plugins/r/server_audit.result +++ b/mysql-test/suite/plugins/r/server_audit.result @@ -118,6 +118,7 @@ CREATE USER u1 IDENTIFIED BY 'pwd-123'; GRANT ALL ON sa_db TO u2 IDENTIFIED BY "pwd-321"; SET PASSWORD FOR u1 = PASSWORD('pwd 098'); CREATE USER u3 IDENTIFIED BY ''; +ALTER USER u3 IDENTIFIED BY 'pwd-456'; drop user u1, u2, u3; set global server_audit_events='query_ddl'; create table t1(id int); @@ -382,6 +383,8 @@ TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,proxies_priv, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,roles_mapping, TIME,HOSTNAME,root,localhost,ID,ID,QUERY,sa_db,'CREATE USER u3 IDENTIFIED BY *****',0 TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,user, +TIME,HOSTNAME,root,localhost,ID,ID,QUERY,sa_db,'ALTER USER u3 IDENTIFIED BY \'pwd-456\'',0 +TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,user, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,db, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,tables_priv, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,columns_priv, diff --git a/mysql-test/suite/plugins/t/server_audit.test b/mysql-test/suite/plugins/t/server_audit.test index fa5bd7e1349..6c75c3bf732 100644 --- a/mysql-test/suite/plugins/t/server_audit.test +++ b/mysql-test/suite/plugins/t/server_audit.test @@ -95,6 +95,7 @@ CREATE USER u1 IDENTIFIED BY 'pwd-123'; GRANT ALL ON sa_db TO u2 IDENTIFIED BY "pwd-321"; SET PASSWORD FOR u1 = PASSWORD('pwd 098'); CREATE USER u3 IDENTIFIED BY ''; +ALTER USER u3 IDENTIFIED BY 'pwd-456'; drop user u1, u2, u3; set global server_audit_events='query_ddl'; diff --git a/plugin/server_audit/server_audit.c b/plugin/server_audit/server_audit.c index f6661772df1..9a954365d83 100644 --- a/plugin/server_audit/server_audit.c +++ b/plugin/server_audit/server_audit.c @@ -819,6 +819,7 @@ enum sa_keywords SQLCOM_DML, SQLCOM_GRANT, SQLCOM_CREATE_USER, + SQLCOM_ALTER_USER, SQLCOM_CHANGE_MASTER, SQLCOM_CREATE_SERVER, SQLCOM_SET_OPTION, @@ -926,6 +927,7 @@ struct sa_keyword passwd_keywords[]= { {3, "SET", &password_word, SQLCOM_SET_OPTION}, {5, "ALTER", &server_word, SQLCOM_ALTER_SERVER}, + {5, "ALTER", &user_word, SQLCOM_ALTER_USER}, {5, "GRANT", 0, SQLCOM_GRANT}, {6, "CREATE", &user_word, SQLCOM_CREATE_USER}, {6, "CREATE", &server_word, SQLCOM_CREATE_SERVER}, @@ -1845,6 +1847,7 @@ do_log_query: { case SQLCOM_GRANT: case SQLCOM_CREATE_USER: + case SQLCOM_ALTER_USER: csize+= escape_string_hide_passwords(query, query_len, uh_buffer, uh_buffer_size, "IDENTIFIED", 10, "BY", 2, 0); From dd9e5827a68c7864d61af44a4dea6932f963c55c Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 25 Feb 2021 14:10:25 +0100 Subject: [PATCH 04/16] mtr --gdb: fix for --rr and for a warning use _RR_TRACE_DIR=dir instead of -o dir, as the former can store multiple traces in dir (if, e.g., the test restarts mysqld) suppress uninitialized warning when $exe is undefined (--manual-XXX) --- mysql-test/lib/My/Debugger.pm | 3 ++- mysql-test/mysql-test-run.pl | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/mysql-test/lib/My/Debugger.pm b/mysql-test/lib/My/Debugger.pm index d2add55d680..a2d5f2a5435 100644 --- a/mysql-test/lib/My/Debugger.pm +++ b/mysql-test/lib/My/Debugger.pm @@ -74,7 +74,8 @@ my %debuggers = ( options => '-f -o {log} {exe} {args}', }, rr => { - options => 'record -o {log} {exe} {args}', + options => '_RR_TRACE_DIR={log} rr record {exe} {args}', + run => 'env', pre => sub { ::mtr_error('rr requires kernel.perf_event_paranoid <= 1') if ::mtr_grab_file('/proc/sys/kernel/perf_event_paranoid') > 1; diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index 07f103ba91e..fd25c28dc06 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -5071,7 +5071,7 @@ sub mysqld_start ($$) { $ENV{'MYSQLD_LAST_CMD'}= "$exe @$args"; My::Debugger::setup_args(\$args, \$exe, $mysqld->name()); - $ENV{'VALGRIND_TEST'}= $opt_valgrind = int($exe && $exe eq 'valgrind'); + $ENV{'VALGRIND_TEST'}= $opt_valgrind = int(($exe || '') eq 'valgrind'); # Remove the old pidfile if any unlink($mysqld->value('pid-file')); From a18b39e3f49f4916792b72abd7f0fc153a7454fa Mon Sep 17 00:00:00 2001 From: Alexey Botchkov Date: Mon, 1 Mar 2021 20:08:14 +0400 Subject: [PATCH 05/16] MDEV-24965 With ALTER USER ...IDENTIFIED BY command, password doesn't replaced by asterisks in audit log. Test result fixed. --- mysql-test/suite/plugins/r/server_audit.result | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql-test/suite/plugins/r/server_audit.result b/mysql-test/suite/plugins/r/server_audit.result index ff22cdff8d6..bbb3a62f9b4 100644 --- a/mysql-test/suite/plugins/r/server_audit.result +++ b/mysql-test/suite/plugins/r/server_audit.result @@ -383,7 +383,7 @@ TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,proxies_priv, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,roles_mapping, TIME,HOSTNAME,root,localhost,ID,ID,QUERY,sa_db,'CREATE USER u3 IDENTIFIED BY *****',0 TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,user, -TIME,HOSTNAME,root,localhost,ID,ID,QUERY,sa_db,'ALTER USER u3 IDENTIFIED BY \'pwd-456\'',0 +TIME,HOSTNAME,root,localhost,ID,ID,QUERY,sa_db,'ALTER USER u3 IDENTIFIED BY *****',0 TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,user, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,db, TIME,HOSTNAME,root,localhost,ID,ID,WRITE,mysql,tables_priv, From 43a0a8139727314f89fc0f0f0d2ba80ffa33b221 Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 27 Feb 2021 19:42:43 +0200 Subject: [PATCH 06/16] Fixed printing of wring filname "maria_open" in maria.maria-recovery2.test eprintf() was missing a va_start(), which caused wrong filename to be printed when printing recovery trace. Added also missing new line when printing "Table is crashed" to trace file --- storage/maria/ma_recovery.c | 1 + storage/maria/ma_recovery_util.c | 1 + 2 files changed, 2 insertions(+) diff --git a/storage/maria/ma_recovery.c b/storage/maria/ma_recovery.c index 5c3dae2708d..6cc5b423720 100644 --- a/storage/maria/ma_recovery.c +++ b/storage/maria/ma_recovery.c @@ -1447,6 +1447,7 @@ static int new_table(uint16 sid, const char *name, LSN lsn_of_file_id) } if (maria_is_crashed(info)) { + tprint(tracef, "\n"); eprint(tracef, "Table '%s' is crashed, skipping it. Please repair it with" " aria_chk -r", share->open_file_name.str); recovery_found_crashed_tables++; diff --git a/storage/maria/ma_recovery_util.c b/storage/maria/ma_recovery_util.c index 3bbda614991..fe43d812600 100644 --- a/storage/maria/ma_recovery_util.c +++ b/storage/maria/ma_recovery_util.c @@ -98,6 +98,7 @@ void eprint(FILE *trace_file __attribute__ ((unused)), fputc('\n', trace_file); if (trace_file != stderr) { + va_start(args, format); my_printv_error(HA_ERR_INITIALIZATION, format, MYF(0), args); } va_end(args); From 6983ce704baff7a6e5dd411a289e3580bc7bea1a Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 27 Feb 2021 19:56:46 +0200 Subject: [PATCH 07/16] MDEV-24710 Uninitialized value upon CREATE .. SELECT ... VALUE... The failure happened for group by queries when all tables where marked as 'const tables' (tables with 0-1 matching rows) and no row matched the where clause and there was in addition a direct reference to a field. In this case the field would not be properly reset and the query would return 'random data' that happended to be in table->record[0]. Fixed by marking all const tables as null tables in this particular case. Sergei also provided an extra test case for the code. @reviewer Sergei Petrunia --- mysql-test/main/group_by.result | 29 +++++++++++ mysql-test/main/group_by.test | 23 +++++++++ sql/sql_select.cc | 87 +++++++++++++++++++++++++++++---- sql/table.h | 6 +++ 4 files changed, 135 insertions(+), 10 deletions(-) diff --git a/mysql-test/main/group_by.result b/mysql-test/main/group_by.result index 246cceb96c3..8be5b4d29b9 100644 --- a/mysql-test/main/group_by.result +++ b/mysql-test/main/group_by.result @@ -2934,5 +2934,34 @@ f COUNT(*) NULL 1 DROP TABLE t1; # +# MDEV-24710 Uninitialized value upon CREATE .. SELECT ... VALUE +# +CREATE TABLE t1 (a VARCHAR(8) NOT NULL DEFAULT ''); +INSERT INTO t1 (a) VALUES ('foo'); +CREATE TABLE t2 AS SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE VALUE(a) IS NOT NULL; +SELECT * from t2; +f1 f2 +NULL NULL +SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE VALUE(a) IS NOT NULL; +f1 f2 +NULL NULL +SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE 1=0; +f1 f2 +NULL NULL +drop table t1,t2; +# Extra test by to check the fix for MDEV-24710 +create table t20 (pk int primary key, a int); +insert into t20 values (1,1); +create table t21 (pk int primary key, b int not null); +insert into t21 values (1,1); +create table t22 (a int); +insert into t22 values (1),(2); +select a, (select max(t21.b) from t20 left join t21 on t21.pk=t20.a+10 +where t20.pk=1 and rand(123) < 0.5) as SUBQ from t22; +a SUBQ +1 NULL +2 NULL +drop table t20, t21, t22; +# # End of 10.3 tests # diff --git a/mysql-test/main/group_by.test b/mysql-test/main/group_by.test index 2866fab3822..8d690222bcc 100644 --- a/mysql-test/main/group_by.test +++ b/mysql-test/main/group_by.test @@ -2040,6 +2040,29 @@ INSERT INTO t1 VALUES ('2032-10-08'); SELECT d != '2023-03-04' AS f, COUNT(*) FROM t1 GROUP BY d WITH ROLLUP; DROP TABLE t1; +--echo # +--echo # MDEV-24710 Uninitialized value upon CREATE .. SELECT ... VALUE +--echo # + +CREATE TABLE t1 (a VARCHAR(8) NOT NULL DEFAULT ''); +INSERT INTO t1 (a) VALUES ('foo'); +CREATE TABLE t2 AS SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE VALUE(a) IS NOT NULL; +SELECT * from t2; +SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE VALUE(a) IS NOT NULL; +SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE 1=0; +drop table t1,t2; + +--echo # Extra test by to check the fix for MDEV-24710 + +create table t20 (pk int primary key, a int); +insert into t20 values (1,1);create table t21 (pk int primary key, b int not null); +insert into t21 values (1,1); +create table t22 (a int); +insert into t22 values (1),(2); +select a, (select max(t21.b) from t20 left join t21 on t21.pk=t20.a+10 + where t20.pk=1 and rand(123) < 0.5) as SUBQ from t22; +drop table t20, t21, t22; + --echo # --echo # End of 10.3 tests --echo # diff --git a/sql/sql_select.cc b/sql/sql_select.cc index c802aa9c18e..7658d843d8b 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -13668,22 +13668,71 @@ return_zero_rows(JOIN *join, select_result *result, List &tables, DBUG_RETURN(0); } -/* - used only in JOIN::clear +/** + used only in JOIN::clear (always) and in do_select() + (if there where no matching rows) + + @param join JOIN + @param cleared_tables If not null, clear also const tables and mark all + cleared tables in the map. cleared_tables is only + set when called from do_select() when there is a + group function and there where no matching rows. */ -static void clear_tables(JOIN *join) + +static void clear_tables(JOIN *join, table_map *cleared_tables) { /* - must clear only the non-const tables, as const tables - are not re-calculated. + must clear only the non-const tables as const tables are not re-calculated. */ for (uint i= 0 ; i < join->table_count ; i++) { - if (!(join->table[i]->map & join->const_table_map)) - mark_as_null_row(join->table[i]); // All fields are NULL + TABLE *table= join->table[i]; + + if (table->null_row) + continue; // Nothing more to do + if (!(table->map & join->const_table_map) || cleared_tables) + { + if (cleared_tables) + { + (*cleared_tables)|= (((table_map) 1) << i); + if (table->s->null_bytes) + { + /* + Remember null bits for the record so that we can restore the + original const record in unclear_tables() + */ + memcpy(table->record[1], table->null_flags, table->s->null_bytes); + } + } + mark_as_null_row(table); // All fields are NULL + } } } + +/** + Reverse null marking for tables and restore null bits. + + We have to do this because the tables may be re-used in a sub query + and the subquery will assume that the const tables contains the original + data before clear_tables(). +*/ + +static void unclear_tables(JOIN *join, table_map *cleared_tables) +{ + for (uint i= 0 ; i < join->table_count ; i++) + { + if ((*cleared_tables) & (((table_map) 1) << i)) + { + TABLE *table= join->table[i]; + if (table->s->null_bytes) + memcpy(table->null_flags, table->record[1], table->s->null_bytes); + unmark_as_null_row(table); + } + } +} + + /***************************************************************************** Make som simple condition optimization: If there is a test 'field = const' change all refs to 'field' to 'const' @@ -19194,6 +19243,7 @@ do_select(JOIN *join, Procedure *procedure) if (join->only_const_tables() && !join->need_tmp) { Next_select_func end_select= setup_end_select_func(join, NULL); + /* HAVING will be checked after processing aggregate functions, But WHERE should checked here (we alredy have read tables). @@ -19220,12 +19270,29 @@ do_select(JOIN *join, Procedure *procedure) } else if (join->send_row_on_empty_set()) { + table_map cleared_tables= (table_map) 0; + if (end_select == end_send_group) + { + /* + Was a grouping query but we did not find any rows. In this case + we clear all tables to get null in any referenced fields, + like in case of: + SELECT MAX(a) AS f1, a AS f2 FROM t1 WHERE VALUE(a) IS NOT NULL + */ + clear_tables(join, &cleared_tables); + } if (!join->having || join->having->val_int()) { List *columns_list= (procedure ? &join->procedure_fields_list : join->fields); rc= join->result->send_data(*columns_list) > 0; } + /* + We have to remove the null markings from the tables as this table + may be part of a sub query that is re-evaluated + */ + if (cleared_tables) + unclear_tables(join, &cleared_tables); } /* An error can happen when evaluating the conds @@ -20197,8 +20264,8 @@ join_read_const_table(THD *thd, JOIN_TAB *tab, POSITION *pos) if ((table->null_row= MY_TEST((*tab->on_expr_ref)->val_int() == 0))) mark_as_null_row(table); } - if (!table->null_row) - table->maybe_null=0; + if (!table->null_row && ! tab->join->mixed_implicit_grouping) + table->maybe_null= 0; { JOIN *join= tab->join; @@ -25331,7 +25398,7 @@ int JOIN::rollup_write_data(uint idx, TMP_TABLE_PARAM *tmp_table_param_arg, TABL void JOIN::clear() { - clear_tables(this); + clear_tables(this, 0); copy_fields(&tmp_table_param); if (sum_funcs) diff --git a/sql/table.h b/sql/table.h index 3c31a6364df..6a4c6eb2a03 100644 --- a/sql/table.h +++ b/sql/table.h @@ -3145,6 +3145,12 @@ inline void mark_as_null_row(TABLE *table) bfill(table->null_flags,table->s->null_bytes,255); } +inline void unmark_as_null_row(TABLE *table) +{ + table->null_row=0; + table->status= STATUS_NO_RECORD; +} + bool is_simple_order(ORDER *order); class Open_tables_backup; From 415409579af68ee2e3c55d5294d7bb5e6397b03f Mon Sep 17 00:00:00 2001 From: Monty Date: Mon, 1 Mar 2021 14:44:18 +0200 Subject: [PATCH 08/16] MDEV-24958 Server crashes in my_strtod ... with DEFAULT(blob) Fixes also: MDEV-24942 Server crashes in _ma_rec_pack... with DEFAULT() on BLOB This was caused by two different bugs, both related to that the default value for the blob was not calculated before it was used: - There where now Item_default_value::..result() wrappers, which is needed as item in HAVING uses these. This causes crashes when using a reference to a DEFAULT(blob_field) in HAVING. It also caused wrong results when used with other fields with default value expressions that are not constants. - create_tmp_field() did not take into account that blob fields with default expressions are not yet initialized. Fixed by treating Item_default_value(blob) like a normal item expression. --- mysql-test/main/having.result | 33 ++++++++++++++++++++++++ mysql-test/main/having.test | 20 +++++++++++++++ sql/item.cc | 47 ++++++++++++++++++++++++++++++++--- sql/item.h | 10 ++++++++ sql/sql_select.cc | 24 ++++++++++++++++-- 5 files changed, 129 insertions(+), 5 deletions(-) diff --git a/mysql-test/main/having.result b/mysql-test/main/having.result index f37cc48772e..233843de36f 100644 --- a/mysql-test/main/having.result +++ b/mysql-test/main/having.result @@ -846,3 +846,36 @@ t r DROP TABLE t1; DROP FUNCTION next_seq_value; DROP TABLE series; +# +# MDEV-24958 Server crashes in my_strtod / +# Value_source::Converter_strntod::Converter_strntod with DEFAULT(blob) +# +# MDEV-24942 Server crashes in _ma_rec_pack / _ma_write_blob_record with +# DEFAULT() on BLOB +# +CREATE TABLE t1 (id INT, f MEDIUMTEXT NOT NULL DEFAULT 'A'); +INSERT INTO t1 VALUES (1,'foo'),(2,'bar'); +SELECT f FROM t1 GROUP BY id ORDER BY DEFAULT(f); +f +foo +bar +SELECT DEFAULT(f) AS h FROM t1 HAVING h > 5; +h +Warnings: +Warning 1292 Truncated incorrect DOUBLE value: 'A' +SELECT DEFAULT(f) AS h FROM t1 HAVING h >= 0; +h +A +A +Warnings: +Warning 1292 Truncated incorrect DOUBLE value: 'A' +SELECT DEFAULT(f) AS h FROM t1 HAVING h >= 'A'; +h +A +A +alter table t1 add column b int default (rand()+1+3); +select default(b) AS h FROM t1 HAVING h > "2"; +h +# +# +drop table t1; diff --git a/mysql-test/main/having.test b/mysql-test/main/having.test index 179af14559f..8b0cc244f51 100644 --- a/mysql-test/main/having.test +++ b/mysql-test/main/having.test @@ -890,3 +890,23 @@ SELECT t, next_seq_value() r FROM t1 FORCE INDEX(t) DROP TABLE t1; DROP FUNCTION next_seq_value; DROP TABLE series; + +--echo # +--echo # MDEV-24958 Server crashes in my_strtod / +--echo # Value_source::Converter_strntod::Converter_strntod with DEFAULT(blob) +--echo # +--echo # MDEV-24942 Server crashes in _ma_rec_pack / _ma_write_blob_record with +--echo # DEFAULT() on BLOB +--echo # + +CREATE TABLE t1 (id INT, f MEDIUMTEXT NOT NULL DEFAULT 'A'); +INSERT INTO t1 VALUES (1,'foo'),(2,'bar'); +SELECT f FROM t1 GROUP BY id ORDER BY DEFAULT(f); +SELECT DEFAULT(f) AS h FROM t1 HAVING h > 5; +SELECT DEFAULT(f) AS h FROM t1 HAVING h >= 0; +SELECT DEFAULT(f) AS h FROM t1 HAVING h >= 'A'; + +alter table t1 add column b int default (rand()+1+3); +--replace_column 1 # +select default(b) AS h FROM t1 HAVING h > "2"; +drop table t1; diff --git a/sql/item.cc b/sql/item.cc index 522fef57699..7b15c7ddb43 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -9341,7 +9341,6 @@ bool Item_default_value::fix_fields(THD *thd, Item **items) } thd->column_usage= save_column_usage; - real_arg= arg->real_item(); if (real_arg->type() != FIELD_ITEM) { @@ -9364,7 +9363,7 @@ bool Item_default_value::fix_fields(THD *thd, Item **items) def_field->reset_fields(); // If non-constant default value expression or a blob if (def_field->default_value && - (def_field->default_value->flags || def_field->flags & BLOB_FLAG)) + (def_field->default_value->flags || (def_field->flags & BLOB_FLAG))) { uchar *newptr= (uchar*) thd->alloc(1+def_field->pack_length()); if (!newptr) @@ -9461,11 +9460,53 @@ int Item_default_value::save_in_field(Field *field_arg, bool no_conversions) return Item_field::save_in_field(field_arg, no_conversions); } +double Item_default_value::val_result() +{ + calculate(); + return Item_field::val_result(); +} + +longlong Item_default_value::val_int_result() +{ + calculate(); + return Item_field::val_int_result(); +} + +String *Item_default_value::str_result(String* tmp) +{ + calculate(); + return Item_field::str_result(tmp); +} + +bool Item_default_value::val_bool_result() +{ + calculate(); + return Item_field::val_bool_result(); +} + +bool Item_default_value::is_null_result() +{ + calculate(); + return Item_field::is_null_result(); +} + +my_decimal *Item_default_value::val_decimal_result(my_decimal *decimal_value) +{ + calculate(); + return Item_field::val_decimal_result(decimal_value); +} + +bool Item_default_value::get_date_result(MYSQL_TIME *ltime,ulonglong fuzzydate) +{ + calculate(); + return Item_field::get_date_result(ltime, fuzzydate); +} + table_map Item_default_value::used_tables() const { if (!field || !field->default_value) return static_cast(0); - if (!field->default_value->expr) // not fully parsed field + if (!field->default_value->expr) // not fully parsed field return static_cast(RAND_TABLE_BIT); return field->default_value->expr->used_tables(); } diff --git a/sql/item.h b/sql/item.h index 9200dd80ee8..9fa384f0947 100644 --- a/sql/item.h +++ b/sql/item.h @@ -5859,6 +5859,16 @@ public: longlong val_int(); my_decimal *val_decimal(my_decimal *decimal_value); bool get_date(MYSQL_TIME *ltime,ulonglong fuzzydate); + + /* Result variants */ + double val_result(); + longlong val_int_result(); + String *str_result(String* tmp); + my_decimal *val_decimal_result(my_decimal *val); + bool val_bool_result(); + bool is_null_result(); + bool get_date_result(MYSQL_TIME *ltime,ulonglong fuzzydate); + bool send(Protocol *protocol, st_value *buffer); int save_in_field(Field *field_arg, bool no_conversions); bool save_in_param(THD *thd, Item_param *param) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 7658d843d8b..ce820946908 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -17224,7 +17224,13 @@ Field *Item::create_field_for_schema(THD *thd, TABLE *table) the record in the original table. If modify_item is 0 then fill_record() will update the temporary table - + @param table_cant_handle_bit_fields + Set to 1 if the temporary table cannot handle bit + fields. Only set for heap tables when the bit field + is part of an index. + @param make_copy_field + Set when using with rollup when we want to have + an exact copy of the field. @retval 0 on error @retval @@ -17261,8 +17267,22 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type, my_error(ER_OUT_OF_RESOURCES, MYF(ME_FATALERROR)); return result; } - case Item::FIELD_ITEM: case Item::DEFAULT_VALUE_ITEM: + { + Field *field= ((Item_default_value*) item)->field; + if (field->default_value && (field->flags & BLOB_FLAG)) + { + /* + We have to use a copy function when using a blob with default value + as the we have to calcuate the default value before we can use it. + */ + return create_tmp_field_from_item(thd, item, table, + (make_copy_field ? 0 : copy_func), + modify_item); + } + } + /* Fall through */ + case Item::FIELD_ITEM: case Item::CONTEXTUALLY_TYPED_VALUE_ITEM: case Item::INSERT_VALUE_ITEM: case Item::TRIGGER_FIELD_ITEM: From 3f15d3bad986c36317be75607c6a928ef4978028 Mon Sep 17 00:00:00 2001 From: Monty Date: Mon, 1 Mar 2021 17:51:44 +0200 Subject: [PATCH 09/16] Fixed unit test to not 'bail out' if some tests are not compiled. Before the changes two things could happen: - "path required name explain_filename path" error - unit test never finishead (as it tried to execute just /bin/sh as a test case) --- mysql-test/suite/unit/suite.pm | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/mysql-test/suite/unit/suite.pm b/mysql-test/suite/unit/suite.pm index c8180c59240..43c9e115de6 100644 --- a/mysql-test/suite/unit/suite.pm +++ b/mysql-test/suite/unit/suite.pm @@ -20,7 +20,6 @@ sub start_test { ($path, $args) = ($cmd, , [ ]) } - my $oldpwd=getcwd(); chdir $::opt_vardir; my $proc=My::SafeProcess->new @@ -44,15 +43,17 @@ sub start_test { my (@ctest_list)= `cd "$bin" && ctest $ctest_vs --show-only --verbose`; return "No ctest" if $?; + $ENV{MYSQL_TEST_PLUGINDIR}=$::plugindir; + my ($command, %tests, $prefix); for (@ctest_list) { chomp; - if (/^\d+: Test command: +/) { - $command= $'; + if (/^\d+: Test command: +([^ \t]+)/) { + $command= $1; $prefix= /libmariadb/ ? 'conc_' : ''; - } elsif (/^ +Test +#\d+: +/) { - if ($command ne "NOT_AVAILABLE") { - $tests{$prefix.$'}=$command; + } elsif (/^ +Test +#\d+: ([^ \t]+)/) { + if ($command ne "NOT_AVAILABLE" && $command ne "/bin/sh") { + $tests{$prefix.$1}=$command; } } } From c25e6f91da2eb04fe222aac1c79c6a5c1f94c760 Mon Sep 17 00:00:00 2001 From: Monty Date: Mon, 1 Mar 2021 22:08:47 +0200 Subject: [PATCH 10/16] Fixed typo in comment --- sql/sql_table.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 54e382a6c24..61b6023c6a1 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -7759,7 +7759,7 @@ static bool mysql_inplace_alter_table(THD *thd, Rename to the new name (if needed) will be handled separately below. TODO: remove this check of thd->is_error() (now it intercept - errors in some val_*() methoids and bring some single place to + errors in some val_*() methods and bring some single place to such error interception). */ if (mysql_rename_table(db_type, &alter_ctx->new_db, &alter_ctx->tmp_name, From 0f81ca6a0bb21fbba4bca93a7555f7c8e6357b47 Mon Sep 17 00:00:00 2001 From: Igor Babaev Date: Mon, 1 Mar 2021 09:40:33 -0800 Subject: [PATCH 11/16] MDEV-24919 Crash with subselect formed by table value constructor and used in set function If a subselect is formed by a table value constructor (TVC) then the following transformation is applied at the prepare stage: VALUES (v1), ... (vn) => SELECT * FROM (VALUES (v1), ... (vn)) tvc_x. The transformation is performed by the function wrap_tvc() that resets THD::LEX::current select to the top level select of the result of the transformation. After the call of wrap_tvc() in the function Item_subselect::wrap_tvc_into_select() the field THD::LEX::current must be reset to the same select as before the call. It was not done. As a result if the subselect formed by a TVC was an argument of a set function then an assertion was hit in the function Item_sum::check_sum_func(). Approved by Oleksandr Byelkin --- mysql-test/main/table_value_constr.result | 6 ++++++ mysql-test/main/table_value_constr.test | 6 ++++++ sql/sql_tvc.cc | 15 +++++---------- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/mysql-test/main/table_value_constr.result b/mysql-test/main/table_value_constr.result index 603f21a772d..d2965ab1f6c 100644 --- a/mysql-test/main/table_value_constr.result +++ b/mysql-test/main/table_value_constr.result @@ -2881,4 +2881,10 @@ NULL deallocate prepare stmt; drop view v1; drop table t1,t2,t3; +# +# MDEV-24919: subselect formed by TVC and used in set function +# +select sum((values(1))); +sum((values(1))) +1 End of 10.3 tests diff --git a/mysql-test/main/table_value_constr.test b/mysql-test/main/table_value_constr.test index 2246a19d306..88d0ac2fbbf 100644 --- a/mysql-test/main/table_value_constr.test +++ b/mysql-test/main/table_value_constr.test @@ -1516,4 +1516,10 @@ deallocate prepare stmt; drop view v1; drop table t1,t2,t3; +--echo # +--echo # MDEV-24919: subselect formed by TVC and used in set function +--echo # + +select sum((values(1))); + --echo End of 10.3 tests diff --git a/sql/sql_tvc.cc b/sql/sql_tvc.cc index 0a771b592e4..cb056b0e15f 100644 --- a/sql/sql_tvc.cc +++ b/sql/sql_tvc.cc @@ -648,7 +648,7 @@ st_select_lex *wrap_tvc(THD *thd, st_select_lex *tvc_sl, st_select_lex *parent_select) { LEX *lex= thd->lex; - select_result *save_result= thd->lex->result; + select_result *save_result= lex->result; uint8 save_derived_tables= lex->derived_tables; thd->lex->result= NULL; @@ -729,13 +729,13 @@ st_select_lex *wrap_tvc(THD *thd, st_select_lex *tvc_sl, if (arena) thd->restore_active_arena(arena, &backup); - thd->lex->result= save_result; + lex->result= save_result; return wrapper_sl; err: if (arena) thd->restore_active_arena(arena, &backup); - thd->lex->result= save_result; + lex->result= save_result; lex->derived_tables= save_derived_tables; return 0; } @@ -819,14 +819,9 @@ Item_subselect::wrap_tvc_into_select(THD *thd, st_select_lex *tvc_sl) { if (engine->engine_type() == subselect_engine::SINGLE_SELECT_ENGINE) ((subselect_single_select_engine *) engine)->change_select(wrapper_sl); - lex->current_select= wrapper_sl; - return wrapper_sl; - } - else - { - lex->current_select= parent_select; - return 0; } + lex->current_select= parent_select; + return wrapper_sl; } From fc77431624b8d4dc966cd3855c16bee856b05645 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Tue, 2 Mar 2021 19:09:44 +0700 Subject: [PATCH 12/16] MDEV-25006: Failed assertion on executing EXPLAIN DELETE statement as a prepared statement Attempt to execute EXPLAIN statement on multi-table DELETE statement leads to firing firing of the assertion DBUG_ASSERT(! is_set()); in the method Diagnostics_area::set_eof_status. For example, above mentioned assertion failure happens in case any of the following statements EXPLAIN DELETE FROM t1.* USING t1 EXPLAIN DELETE b FROM t1 AS a JOIN t1 AS b are executed in prepared statement mode provided the table t1 does exist. This assertion is hit by the reason that a status of Diagnostics_area is set twice. The first time it is set from the function do_select() when the method multi_delete::send_eof() called. The second time it is set when the method Explain_query::send_explain() calls the method select_send::send_eof (this method invokes the method Diagnostics_area::set_eof_status that finally hits assertion) The second invocation for a setter method of the class Diagnostics_area is correct and run to send a response containing explain data. But first invocation of a setter method of the class Diagnostics_area is wrong since the function do_select() shouldn't be called at all for handling of the EXPLAIN statement. The reason by that the function do_select() is called during handling of the EXPLAIN statement is that the flag SELECT_DESCRIBE not set in the data member JOIN::select_options. The flag SELECT_DESCRIBE if is copied from values select_lex->options. During parsing of EXPLAIN statement this flag is set but latter reset from the function reinit_stmt_before_use() that is called on execution of prepared statement. void reinit_stmt_before_use(THD *thd, LEX *lex) { ... for (; sl; sl= sl->next_select_in_list()) { if (sl->changed_elements & TOUCHED_SEL_COND) { /* remove option which was put by mysql_explain_union() */ sl->options&= ~SELECT_DESCRIBE; ... } ... } So, to fix the issue the flag SELECT_DESCRIBE is set forcibly at the mysql_select() function in case thd->lex->describe set, that is in case EXPLAIN being executed. --- mysql-test/r/ps.result | 17 +++++++++++++++++ mysql-test/t/ps.test | 14 ++++++++++++++ sql/sql_select.cc | 3 +++ 3 files changed, 34 insertions(+) diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index 076c7d9abda..17284a3cad8 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -5402,5 +5402,22 @@ EXISTS(SELECT 1 FROM t1 GROUP BY a IN (select a from t1)) 0 DROP TABLE t1; # +# MDEV-25006: Failed assertion on executing EXPLAIN DELETE statement as a prepared statement +# +CREATE TABLE t1(c1 CHAR(255) PRIMARY KEY); +PREPARE stmt FROM 'EXPLAIN DELETE b FROM t1 AS a JOIN t1 AS b'; +EXECUTE stmt; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE a system NULL NULL NULL NULL 0 const row not found +1 SIMPLE b system NULL NULL NULL NULL 0 const row not found +DROP TABLE t1; +CREATE TABLE t1(a INT); +PREPARE stmt FROM 'EXPLAIN DELETE FROM t1.* USING t1'; +EXECUTE stmt; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 system NULL NULL NULL NULL 0 const row not found +DEALLOCATE PREPARE stmt; +DROP TABLE t1; +# # End of 10.2 tests # diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index 1b1f88094ff..a267c8b0e42 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -4912,6 +4912,20 @@ EXECUTE stmt; EXECUTE stmt; DROP TABLE t1; +--echo # +--echo # MDEV-25006: Failed assertion on executing EXPLAIN DELETE statement as a prepared statement +--echo # + +CREATE TABLE t1(c1 CHAR(255) PRIMARY KEY); +PREPARE stmt FROM 'EXPLAIN DELETE b FROM t1 AS a JOIN t1 AS b'; +EXECUTE stmt; +DROP TABLE t1; +CREATE TABLE t1(a INT); +PREPARE stmt FROM 'EXPLAIN DELETE FROM t1.* USING t1'; +EXECUTE stmt; +DEALLOCATE PREPARE stmt; +DROP TABLE t1; + --echo # --echo # End of 10.2 tests --echo # diff --git a/sql/sql_select.cc b/sql/sql_select.cc index aa3474dff5f..f30ce088bc4 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -3801,6 +3801,9 @@ mysql_select(THD *thd, } else { + if (thd->lex->describe) + select_options|= SELECT_DESCRIBE; + /* When in EXPLAIN, delay deleting the joins so that they are still available when we're producing EXPLAIN EXTENDED warning text. From 82efe4a15a985c3902e80eb7e1a70841c08d9f2e Mon Sep 17 00:00:00 2001 From: Monty Date: Tue, 2 Mar 2021 14:13:39 +0200 Subject: [PATCH 13/16] MDEV-23843 Assertions in Diagnostics_area upon table operations under FTWRL Fixed binary logging in ANALYZE TABLE to work as optimize table --- mysql-test/main/flush_and_binlog.result | 11 +++++++++++ mysql-test/main/flush_and_binlog.test | 14 ++++++++++++++ sql/sql_admin.cc | 2 ++ 3 files changed, 27 insertions(+) diff --git a/mysql-test/main/flush_and_binlog.result b/mysql-test/main/flush_and_binlog.result index b9560964046..a1d73c6590f 100644 --- a/mysql-test/main/flush_and_binlog.result +++ b/mysql-test/main/flush_and_binlog.result @@ -20,3 +20,14 @@ ERROR HY000: Lock wait timeout exceeded; try restarting transaction connection default; disconnect con1; unlock tables; +# Second test from MDEV-23843 +CREATE TABLE t (a INT); +FLUSH TABLES WITH READ LOCK; +connect con1,localhost,root,,; +SET lock_wait_timeout= 1; +ANALYZE TABLE t; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +disconnect con1; +connection default; +UNLOCK TABLES; +DROP TABLE t; diff --git a/mysql-test/main/flush_and_binlog.test b/mysql-test/main/flush_and_binlog.test index 373b900b451..a28d8e365dd 100644 --- a/mysql-test/main/flush_and_binlog.test +++ b/mysql-test/main/flush_and_binlog.test @@ -27,3 +27,17 @@ FLUSH TABLES; --connection default --disconnect con1 unlock tables; + +--echo # Second test from MDEV-23843 + +CREATE TABLE t (a INT); +FLUSH TABLES WITH READ LOCK; +--connect (con1,localhost,root,,) +SET lock_wait_timeout= 1; +--error ER_LOCK_WAIT_TIMEOUT +ANALYZE TABLE t; +# Cleanup +--disconnect con1 +--connection default +UNLOCK TABLES; +DROP TABLE t; diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index c9185045315..e59dffc10aa 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -1396,7 +1396,9 @@ bool Sql_cmd_analyze_table::execute(THD *thd) /* Presumably, ANALYZE and binlog writing doesn't require synchronization */ + thd->get_stmt_da()->set_overwrite_status(true); res= write_bin_log(thd, TRUE, thd->query(), thd->query_length()); + thd->get_stmt_da()->set_overwrite_status(false); } m_lex->first_select_lex()->table_list.first= first_table; m_lex->query_tables= first_table; From 676987c4a14069d415c56d0f259aaaa7ca742c23 Mon Sep 17 00:00:00 2001 From: Monty Date: Tue, 2 Mar 2021 14:08:38 +0200 Subject: [PATCH 14/16] MDEV-24532 Table corruption ER_NO_SUCH_TABLE_IN_ENGINE .. on table with foreign key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When doing a truncate on an Innodb under lock tables, InnoDB would rename the old table to #sql-... and recreate a new 't1' table. The table lock would still be on the #sql-table. When doing ALTER TABLE, Innodb would do the changes on the #sql table (which would disappear on close). When the SQL layer, as part of inline alter table, would close the original t1 table (#sql in InnoDB) and then reopen the t1 table, Innodb would notice that this does not match it's own (old) t1 table and generate an error. Fixed by adding code in truncate table that if we are under lock tables and truncating an InnoDB table, we would close, reopen and lock the table after truncate. This will remove the #sql table and ensure that lock tables is using the new empty table. Reviewer: Marko Mäkelä --- mysql-test/suite/innodb/r/truncate_foreign.result | 11 +++++++++++ mysql-test/suite/innodb/t/truncate_foreign.test | 13 +++++++++++++ sql/handler.h | 6 ++++++ sql/sql_truncate.cc | 9 +++++++++ storage/innobase/handler/ha_innodb.cc | 3 ++- 5 files changed, 41 insertions(+), 1 deletion(-) diff --git a/mysql-test/suite/innodb/r/truncate_foreign.result b/mysql-test/suite/innodb/r/truncate_foreign.result index fc09b74d62f..12a41860708 100644 --- a/mysql-test/suite/innodb/r/truncate_foreign.result +++ b/mysql-test/suite/innodb/r/truncate_foreign.result @@ -57,3 +57,14 @@ disconnect dml; connection default; SET DEBUG_SYNC = RESET; DROP TABLE child, parent; +# +# MDEV-24532 Table corruption ER_NO_SUCH_TABLE_IN_ENGINE or +# ER_CRASHED_ON_USAGE after ALTER on table with foreign key +# +CREATE TABLE t1 (a INT, b INT, PRIMARY KEY (a)) ENGINE=InnoDB; +ALTER TABLE t1 ADD FOREIGN KEY (b) REFERENCES t1 (a) ON UPDATE CASCADE; +LOCK TABLE t1 WRITE; +TRUNCATE TABLE t1; +ALTER TABLE t1 ADD c INT; +UNLOCK TABLES; +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/truncate_foreign.test b/mysql-test/suite/innodb/t/truncate_foreign.test index d9d647e69f0..1c150e5db40 100644 --- a/mysql-test/suite/innodb/t/truncate_foreign.test +++ b/mysql-test/suite/innodb/t/truncate_foreign.test @@ -67,3 +67,16 @@ connection default; SET DEBUG_SYNC = RESET; DROP TABLE child, parent; + +--echo # +--echo # MDEV-24532 Table corruption ER_NO_SUCH_TABLE_IN_ENGINE or +--echo # ER_CRASHED_ON_USAGE after ALTER on table with foreign key +--echo # + +CREATE TABLE t1 (a INT, b INT, PRIMARY KEY (a)) ENGINE=InnoDB; +ALTER TABLE t1 ADD FOREIGN KEY (b) REFERENCES t1 (a) ON UPDATE CASCADE; +LOCK TABLE t1 WRITE; +TRUNCATE TABLE t1; +ALTER TABLE t1 ADD c INT; +UNLOCK TABLES; +DROP TABLE t1; diff --git a/sql/handler.h b/sql/handler.h index 6113b748696..e0e0604176d 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1419,6 +1419,12 @@ handlerton *ha_default_tmp_handlerton(THD *thd); // MySQL compatibility. Unused. #define HTON_SUPPORTS_FOREIGN_KEYS (1 << 0) //Foreign key constraint supported. +/* + Table requires and close and reopen after truncate + If the handler has HTON_CAN_RECREATE, this flag is not used +*/ +#define HTON_REQUIRES_CLOSE_AFTER_TRUNCATE (1 << 18) + class Ha_trx_info; struct THD_TRANS diff --git a/sql/sql_truncate.cc b/sql/sql_truncate.cc index ea4d7399ea3..7d1f630b88c 100644 --- a/sql/sql_truncate.cc +++ b/sql/sql_truncate.cc @@ -439,6 +439,15 @@ bool Sql_cmd_truncate_table::truncate_table(THD *thd, TABLE_LIST *table_ref) */ error= handler_truncate(thd, table_ref, FALSE); + if (error == TRUNCATE_OK && thd->locked_tables_mode && + (table_ref->table->file->ht->flags & + HTON_REQUIRES_CLOSE_AFTER_TRUNCATE)) + { + thd->locked_tables_list.mark_table_for_reopen(thd, table_ref->table); + if (unlikely(thd->locked_tables_list.reopen_tables(thd, true))) + thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0); + } + /* All effects of a TRUNCATE TABLE operation are committed even if truncation fails in the case of non transactional tables. Thus, the diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 2c51f0a9fdf..c67bf6bd607 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -3702,7 +3702,8 @@ innobase_init( innobase_hton->flush_logs = innobase_flush_logs; innobase_hton->show_status = innobase_show_status; innobase_hton->flags = - HTON_SUPPORTS_EXTENDED_KEYS | HTON_SUPPORTS_FOREIGN_KEYS; + HTON_SUPPORTS_EXTENDED_KEYS | HTON_SUPPORTS_FOREIGN_KEYS | + HTON_REQUIRES_CLOSE_AFTER_TRUNCATE; #ifdef WITH_WSREP innobase_hton->abort_transaction=wsrep_abort_transaction; From 5bd994b0d56d11bf62717a84172c49ca9ed37de4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 3 Mar 2021 10:13:56 +0200 Subject: [PATCH 15/16] MDEV-24811 Assertion find(table) failed with innodb_evict_tables_on_commit_debug This is a backport of commit 18535a402817d8a2b8452df0f75c15dda9199acb from 10.6. lock_release(): Implement innodb_evict_tables_on_commit_debug. Before releasing any locks, collect the identifiers of tables to be evicted. After releasing all locks, look up for the tables and evict them if it is safe to do so. trx_commit_in_memory(): Invoke trx_update_mod_tables_timestamp() before lock_release(), so that our locks will protect the tables from being evicted. --- storage/innobase/lock/lock0lock.cc | 33 ++++++++++++++++++++ storage/innobase/trx/trx0trx.cc | 50 +++--------------------------- 2 files changed, 37 insertions(+), 46 deletions(-) diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index da3dba73bee..ee57a493119 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -4263,6 +4263,19 @@ lock_check_dict_lock( and release possible other transactions waiting because of these locks. */ void lock_release(trx_t* trx) { +#ifdef UNIV_DEBUG + std::set to_evict; + if (innodb_evict_tables_on_commit_debug && !trx->is_recovered) +# if 1 /* if dict_stats_exec_sql() were not playing dirty tricks */ + if (!mutex_own(&dict_sys->mutex)) +# else /* this would be more proper way to do it */ + if (!trx->dict_operation_lock_mode && !trx->dict_operation) +# endif + for (trx_mod_tables_t::const_iterator it= trx->mod_tables.begin(); + it != trx->mod_tables.end(); ++it) + if (!it->first->is_temporary()) + to_evict.insert(it->first->id); +#endif ulint count = 0; trx_id_t max_trx_id = trx_sys.get_max_trx_id(); @@ -4311,6 +4324,26 @@ void lock_release(trx_t* trx) } lock_mutex_exit(); + +#ifdef UNIV_DEBUG + if (to_evict.empty()) { + return; + } + mutex_enter(&dict_sys->mutex); + lock_mutex_enter(); + for (std::set::const_iterator i = to_evict.begin(); + i != to_evict.end(); ++i) { + if (dict_table_t *table = dict_table_open_on_id( + *i, TRUE, DICT_TABLE_OP_OPEN_ONLY_IF_CACHED)) { + if (!table->get_ref_count() + && !UT_LIST_GET_LEN(table->locks)) { + dict_table_remove_from_cache_low(table, true); + } + } + } + lock_mutex_exit(); + mutex_exit(&dict_sys->mutex); +#endif } /* True if a lock mode is S or X */ diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 609ab63ff4c..90ed4141633 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -1276,22 +1276,6 @@ trx_update_mod_tables_timestamp( const time_t now = time(NULL); trx_mod_tables_t::const_iterator end = trx->mod_tables.end(); -#ifdef UNIV_DEBUG -# if MYSQL_VERSION_ID >= 100405 -# define dict_sys_mutex dict_sys.mutex -# else -# define dict_sys_mutex dict_sys->mutex -# endif - - const bool preserve_tables = !innodb_evict_tables_on_commit_debug - || trx->is_recovered /* avoid trouble with XA recovery */ -# if 1 /* if dict_stats_exec_sql() were not playing dirty tricks */ - || mutex_own(&dict_sys_mutex) -# else /* this would be more proper way to do it */ - || trx->dict_operation_lock_mode || trx->dict_operation -# endif - ; -#endif for (trx_mod_tables_t::const_iterator it = trx->mod_tables.begin(); it != end; @@ -1307,30 +1291,6 @@ trx_update_mod_tables_timestamp( intrusive. */ dict_table_t* table = it->first; table->update_time = now; -#ifdef UNIV_DEBUG - if (preserve_tables || table->get_ref_count() - || UT_LIST_GET_LEN(table->locks)) { - /* do not evict when committing DDL operations - or if some other transaction is holding the - table handle */ - continue; - } - /* recheck while holding the mutex that blocks - table->acquire() */ - mutex_enter(&dict_sys_mutex); - mutex_enter(&lock_sys.mutex); - const bool do_evict = !table->get_ref_count() - && !UT_LIST_GET_LEN(table->locks); - mutex_exit(&lock_sys.mutex); - if (do_evict) { -# if MYSQL_VERSION_ID >= 100405 - dict_sys.remove(table, true); -# else - dict_table_remove_from_cache_low(table, true); -# endif - } - mutex_exit(&dict_sys_mutex); -#endif } trx->mod_tables.clear(); @@ -1398,16 +1358,10 @@ trx_commit_in_memory( while (UNIV_UNLIKELY(trx->is_referenced())) { ut_delay(srv_spin_wait_delay); } - - trx->release_locks(); - trx->id = 0; } else { ut_ad(trx->read_only || !trx->rsegs.m_redo.rseg); - trx->release_locks(); } - DEBUG_SYNC_C("after_trx_committed_in_memory"); - if (trx->read_only || !trx->rsegs.m_redo.rseg) { MONITOR_INC(MONITOR_TRX_RO_COMMIT); } else { @@ -1415,6 +1369,10 @@ trx_commit_in_memory( MONITOR_INC(MONITOR_TRX_RW_COMMIT); trx->is_recovered = false; } + + trx->release_locks(); + trx->id = 0; + DEBUG_SYNC_C("after_trx_committed_in_memory"); } ut_ad(!trx->rsegs.m_redo.undo); From fcc9f8b10cd2f497ff410b592808eedb3ee5f212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Fri, 5 Mar 2021 10:40:16 +0200 Subject: [PATCH 16/16] Remove unused HA_EXTRA_FAKE_START_STMT This is fixup for commit f06a0b5338694755842a58798bb3a9a40da78bfd. --- include/my_base.h | 6 ++---- sql/ha_partition.cc | 3 +-- storage/innobase/handler/ha_innodb.cc | 4 ---- storage/mroonga/ha_mroonga.cpp | 3 --- 4 files changed, 3 insertions(+), 13 deletions(-) diff --git a/include/my_base.h b/include/my_base.h index 44af7b45075..767da14b4d6 100644 --- a/include/my_base.h +++ b/include/my_base.h @@ -1,5 +1,5 @@ /* Copyright (c) 2000, 2012, Oracle and/or its affiliates. - Copyright (c) 1995, 2018, MariaDB Corporation. + Copyright (c) 1995, 2021, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -213,9 +213,7 @@ enum ha_extra_function { /** Start writing rows during ALTER TABLE...ALGORITHM=COPY. */ HA_EXTRA_BEGIN_ALTER_COPY, /** Finish writing rows during ALTER TABLE...ALGORITHM=COPY. */ - HA_EXTRA_END_ALTER_COPY, - /** Fake the start of a statement after wsrep_load_data_splitting hack */ - HA_EXTRA_FAKE_START_STMT + HA_EXTRA_END_ALTER_COPY }; /* Compatible option, to be deleted in 6.0 */ diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 9313282630f..4c4a62e7fc4 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -1,6 +1,6 @@ /* Copyright (c) 2005, 2019, Oracle and/or its affiliates. - Copyright (c) 2009, 2020, MariaDB + Copyright (c) 2009, 2021, MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -9060,7 +9060,6 @@ int ha_partition::extra(enum ha_extra_function operation) case HA_EXTRA_STARTING_ORDERED_INDEX_SCAN: case HA_EXTRA_BEGIN_ALTER_COPY: case HA_EXTRA_END_ALTER_COPY: - case HA_EXTRA_FAKE_START_STMT: DBUG_RETURN(loop_partitions(extra_cb, &operation)); default: { diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 76723d62761..50d7b2d2002 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -15318,10 +15318,6 @@ ha_innobase::extra( case HA_EXTRA_END_ALTER_COPY: m_prebuilt->table->skip_alter_undo = 0; break; - case HA_EXTRA_FAKE_START_STMT: - trx_register_for_2pc(m_prebuilt->trx); - m_prebuilt->sql_stat_start = true; - break; default:/* Do nothing */ ; } diff --git a/storage/mroonga/ha_mroonga.cpp b/storage/mroonga/ha_mroonga.cpp index 128bc9b3583..fdca803ad96 100644 --- a/storage/mroonga/ha_mroonga.cpp +++ b/storage/mroonga/ha_mroonga.cpp @@ -550,9 +550,6 @@ static const char *mrn_inspect_extra_function(enum ha_extra_function operation) case HA_EXTRA_END_ALTER_COPY: inspected = "HA_EXTRA_END_ALTER_COPY"; break; - case HA_EXTRA_FAKE_START_STMT: - inspected = "HA_EXTRA_FAKE_START_STMT"; - break; #ifdef MRN_HAVE_HA_EXTRA_EXPORT case HA_EXTRA_EXPORT: inspected = "HA_EXTRA_EXPORT";