From 2b8b7394a129ab27225a1284bab253a6714aaf03 Mon Sep 17 00:00:00 2001 From: mkaruza Date: Sun, 28 Jun 2020 23:03:38 +0200 Subject: [PATCH 01/26] MDEV-22222: Assertion `state() == s_executing || state() == s_preparing || state() == s_prepared || state() == s_must_abort || state() == s_aborting || state() == s_cert_failed || state() == s_must_replay' failed in wsrep::transaction::before_rollback LOCK TABLE will do implicit commit, we need to properly handle transaction after commit. --- .../galera_lock_tables_in_transaction.result | 12 +++++++++++ .../t/galera_lock_tables_in_transaction.test | 21 +++++++++++++++++++ sql/sql_parse.cc | 6 ++++++ 3 files changed, 39 insertions(+) create mode 100644 mysql-test/suite/galera/r/galera_lock_tables_in_transaction.result create mode 100644 mysql-test/suite/galera/t/galera_lock_tables_in_transaction.test diff --git a/mysql-test/suite/galera/r/galera_lock_tables_in_transaction.result b/mysql-test/suite/galera/r/galera_lock_tables_in_transaction.result new file mode 100644 index 00000000000..68691a4efd2 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_lock_tables_in_transaction.result @@ -0,0 +1,12 @@ +connection node_2; +connection node_1; +CREATE TABLE t1 (a INT) ENGINE=InnoDB; +START TRANSACTION; +INSERT INTO t1 VALUES (1); +LOCK TABLES t2 READ; +ERROR 42S02: Table 'test.t2' doesn't exist +START TRANSACTION; +INSERT INTO t1 VALUES (1); +LOCK TABLES t1 READ; +UNLOCK TABLES; +DROP TABLE t1; diff --git a/mysql-test/suite/galera/t/galera_lock_tables_in_transaction.test b/mysql-test/suite/galera/t/galera_lock_tables_in_transaction.test new file mode 100644 index 00000000000..5cb7347639c --- /dev/null +++ b/mysql-test/suite/galera/t/galera_lock_tables_in_transaction.test @@ -0,0 +1,21 @@ +# +# Check `LOCK TABLES` command with or without existing table in database. +# Test case for MDEV-22222 / MDEV-22223 +# + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +CREATE TABLE t1 (a INT) ENGINE=InnoDB; + +START TRANSACTION; +INSERT INTO t1 VALUES (1); +--error ER_NO_SUCH_TABLE +LOCK TABLES t2 READ; + +START TRANSACTION; +INSERT INTO t1 VALUES (1); +LOCK TABLES t1 READ; +UNLOCK TABLES; + +DROP TABLE t1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index a7f2f472b0a..69ffbed1ca6 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4998,6 +4998,12 @@ mysql_execute_command(THD *thd) if (res) goto error; +#ifdef WITH_WSREP + /* Clean up the previous transaction on implicit commit. */ + if (wsrep_on(thd) && !wsrep_not_committed(thd) && wsrep_after_statement(thd)) + goto error; +#endif + /* We can't have any kind of table locks while backup is active */ if (thd->current_backup_stage != BACKUP_FINISHED) { From 3f2044ae99633ce6d9c756bb6b045efc0707b4b5 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 26 Jun 2020 13:42:04 +0300 Subject: [PATCH 02/26] MDEV-22535 TABLE::initialize_quick_structures() takes 0.5% in oltp_read_only - Removed not needed bzero in void TABLE::initialize_quick_structures(). - Replaced bzero with TRASH_ALLOC() to have this change verfied with memory checkers - Added missing table->quick_keys.is_set in table_cond_selectivity() --- sql/sql_select.cc | 2 +- sql/table.cc | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index e7f593326a4..1738cf5d18f 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -8573,7 +8573,7 @@ double table_cond_selectivity(JOIN *join, uint idx, JOIN_TAB *s, /* Check if we have a prefix of key=const that matches a quick select. */ - if (!is_hash_join_key_no(key)) + if (!is_hash_join_key_no(key) && table->quick_keys.is_set(key)) { key_part_map quick_key_map= (key_part_map(1) << table->quick_key_parts[key]) - 1; if (table->quick_rows[key] && diff --git a/sql/table.cc b/sql/table.cc index 1877c3b7c76..c0cfc5ca3db 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -9358,10 +9358,10 @@ bool TABLE::export_structure(THD *thd, Row_definition_list *defs) void TABLE::initialize_quick_structures() { - bzero(quick_rows, sizeof(quick_rows)); - bzero(quick_key_parts, sizeof(quick_key_parts)); - bzero(quick_costs, sizeof(quick_costs)); - bzero(quick_n_ranges, sizeof(quick_n_ranges)); + TRASH_ALLOC(quick_rows, sizeof(quick_rows)); + TRASH_ALLOC(quick_key_parts, sizeof(quick_key_parts)); + TRASH_ALLOC(quick_costs, sizeof(quick_costs)); + TRASH_ALLOC(quick_n_ranges, sizeof(quick_n_ranges)); } /* From 29f9e679adc90adf5d3c6e08da947789c9c2ac8b Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 14 Aug 2019 23:46:47 +0300 Subject: [PATCH 03/26] Don't copy uninitialized bytes when copying varstrings When using field_conv(), which is called in case of field1=field2 copy in fill_records(), full varstring's was copied, including unitialized bytes. This caused valgrind to compilain about usage of unitialized bytes when using Aria static length records. Fixed by not using memcpy when copying varstrings but instead just copy the real bytes. --- sql/field.cc | 11 +++++++++++ sql/field.h | 7 +------ sql/handler.h | 3 ++- storage/maria/ha_maria.cc | 2 +- storage/myisam/ha_myisam.cc | 5 +++-- 5 files changed, 18 insertions(+), 10 deletions(-) diff --git a/sql/field.cc b/sql/field.cc index 155d8a4eb0d..1e2a711c956 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -7613,6 +7613,17 @@ int Field_varstring::save_field_metadata(uchar *metadata_ptr) return 2; } + +bool Field_varstring::memcpy_field_possible(const Field *from) const +{ + return (Field_str::memcpy_field_possible(from) && + !compression_method() == !from->compression_method() && + length_bytes == ((Field_varstring*) from)->length_bytes && + (table->file && !(table->file->ha_table_flags() & + HA_RECORD_MUST_BE_CLEAN_ON_WRITE))); +} + + int Field_varstring::store(const char *from,size_t length,CHARSET_INFO *cs) { ASSERT_COLUMN_MARKED_FOR_WRITE_OR_COMPUTED; diff --git a/sql/field.h b/sql/field.h index db78efe0754..34d6684571b 100644 --- a/sql/field.h +++ b/sql/field.h @@ -3463,12 +3463,7 @@ public: length_bytes : 0); } Copy_func *get_copy_func(const Field *from) const; - bool memcpy_field_possible(const Field *from) const - { - return Field_str::memcpy_field_possible(from) && - !compression_method() == !from->compression_method() && - length_bytes == ((Field_varstring*) from)->length_bytes; - } + bool memcpy_field_possible(const Field *from) const; int store(const char *to,size_t length,CHARSET_INFO *charset); using Field_str::store; #ifdef HAVE_valgrind_or_MSAN diff --git a/sql/handler.h b/sql/handler.h index af9fa068891..16fcab1e755 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -200,7 +200,8 @@ enum enum_alter_inplace_result { #define HA_HAS_NEW_CHECKSUM (1ULL << 38) #define HA_CAN_VIRTUAL_COLUMNS (1ULL << 39) #define HA_MRR_CANT_SORT (1ULL << 40) -#define HA_RECORD_MUST_BE_CLEAN_ON_WRITE (1ULL << 41) /* unused */ +/* All of VARCHAR is stored, including bytes after real varchar data */ +#define HA_RECORD_MUST_BE_CLEAN_ON_WRITE (1ULL << 41) /* This storage engine supports condition pushdown diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index ec2a88de4df..c0163473f3a 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -1162,7 +1162,7 @@ int ha_maria::open(const char *name, int mode, uint test_if_locked) that all bytes in the row is properly reset. */ if (file->s->data_file_type == STATIC_RECORD && - (file->s->has_varchar_fields | file->s->has_null_fields)) + (file->s->has_varchar_fields || file->s->has_null_fields)) int_table_flags|= HA_RECORD_MUST_BE_CLEAN_ON_WRITE; for (i= 0; i < table->s->keys; i++) diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index fd2e8fd9425..0d9a2c6c18e 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -863,8 +863,9 @@ int ha_myisam::open(const char *name, int mode, uint test_if_locked) the full row to ensure we don't get any errors from valgrind and that all bytes in the row is properly reset. */ - if ((file->s->options & HA_OPTION_PACK_RECORD) && - (file->s->has_varchar_fields | file->s->has_null_fields)) + if (!(file->s->options & + (HA_OPTION_PACK_RECORD | HA_OPTION_COMPRESS_RECORD)) && + (file->s->has_varchar_fields || file->s->has_null_fields)) int_table_flags|= HA_RECORD_MUST_BE_CLEAN_ON_WRITE; for (i= 0; i < table->s->keys; i++) From 65f831d17c84900c1faea49164688e2f5ce59563 Mon Sep 17 00:00:00 2001 From: Monty Date: Sun, 28 Jun 2020 20:07:32 +0300 Subject: [PATCH 04/26] Fixed bugs found by valgrind - Some of the bug fixes are backports from 10.5! - The fix in innobase/fil/fil0fil.cc is just a backport to get less error messages in mysqld.1.err when running with valgrind. - Renamed HAVE_valgrind_or_MSAN to HAVE_valgrind --- client/mysqltest.cc | 11 ++++--- include/my_sys.h | 2 ++ include/my_valgrind.h | 40 +++++++++++------------ mysql-test/main/sp-big.result | 4 +-- mysql-test/main/sp-big.test | 13 +++++++- sql/field.cc | 2 +- sql/field.h | 7 ++-- sql/ha_partition.cc | 2 +- sql/item_cmpfunc.cc | 2 ++ sql/item_subselect.cc | 2 +- sql/opt_range.cc | 1 + sql/session_tracker.cc | 5 +-- sql/sql_select.cc | 44 ++++++++++++++++++-------- sql/table.cc | 8 ++--- sql/unireg.cc | 4 +-- storage/innobase/btr/btr0cur.cc | 8 ++--- storage/innobase/buf/buf0buddy.cc | 4 +-- storage/innobase/buf/buf0lru.cc | 12 +++---- storage/innobase/data/data0data.cc | 4 +-- storage/innobase/fil/fil0fil.cc | 3 +- storage/innobase/handler/ha_innodb.cc | 8 ++--- storage/innobase/include/dict0stats.ic | 4 +-- storage/innobase/include/srv0mon.h | 6 ++-- storage/innobase/page/page0cur.cc | 8 ++--- storage/innobase/row/row0ftsort.cc | 4 +-- storage/innobase/row/row0ins.cc | 4 +-- storage/innobase/row/row0log.cc | 32 +++++++++---------- storage/innobase/row/row0merge.cc | 22 ++++++------- storage/innobase/row/row0sel.cc | 20 ++++++------ storage/innobase/row/row0upd.cc | 4 +-- storage/innobase/trx/trx0trx.cc | 2 +- storage/tokudb/ha_tokudb.cc | 5 +++ 32 files changed, 171 insertions(+), 126 deletions(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 1fed08ea0af..316e52d95b4 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -10179,7 +10179,7 @@ void append_replace_regex(char* expr, char *expr_end, struct st_replace_regex* r /* Allow variable for the *entire* list of replacements */ if (*p == '$') { - const char *v_end; + const char *v_end= 0; VAR *val= var_get(p, &v_end, 0, 1); if (val) @@ -10820,7 +10820,7 @@ REPLACE *init_replace(char * *from, char * *to,uint count, for (i=1 ; i <= found_sets ; i++) { pos=from[found_set[i-1].table_offset]; - rep_str[i].found= !memcmp(pos, "\\^", 3) ? 2 : 1; + rep_str[i].found= !strncmp(pos, "\\^", 3) ? 2 : 1; rep_str[i].replace_string=to_array[found_set[i-1].table_offset]; rep_str[i].to_offset=found_set[i-1].found_offset-start_at_word(pos); rep_str[i].from_offset=found_set[i-1].found_offset-replace_len(pos)+ @@ -11132,13 +11132,16 @@ void replace_dynstr_append_mem(DYNAMIC_STRING *ds, const char *val, size_t len) { /* Convert to lower case, and do this first */ char *c= lower; - for (const char *v= val; *v; v++) + for (const char *v= val, *end_v= v + len; v < end_v; v++) *c++= my_tolower(charset_info, *v); *c= '\0'; /* Copy from this buffer instead */ } else - memcpy(lower, val, len+1); + { + memcpy(lower, val, len); + lower[len]= 0; + } fix_win_paths(lower, len); val= lower; } diff --git a/include/my_sys.h b/include/my_sys.h index 61551d38337..60f5613eec1 100644 --- a/include/my_sys.h +++ b/include/my_sys.h @@ -535,6 +535,7 @@ static inline int my_b_read(IO_CACHE *info, uchar *Buffer, size_t Count) static inline int my_b_write(IO_CACHE *info, const uchar *Buffer, size_t Count) { + MEM_CHECK_DEFINED(Buffer, Count); if (info->write_pos + Count <= info->write_end) { memcpy(info->write_pos, Buffer, Count); @@ -556,6 +557,7 @@ static inline int my_b_get(IO_CACHE *info) static inline my_bool my_b_write_byte(IO_CACHE *info, uchar chr) { + MEM_CHECK_DEFINED(&chr, 1); if (info->write_pos >= info->write_end) if (my_b_flush_io_cache(info, 1)) return 1; diff --git a/include/my_valgrind.h b/include/my_valgrind.h index 1de6714700b..c181a8c6b35 100644 --- a/include/my_valgrind.h +++ b/include/my_valgrind.h @@ -24,15 +24,19 @@ # define __SANITIZE_ADDRESS__ 1 #endif -#ifdef HAVE_valgrind -#define IF_VALGRIND(A,B) A -#else -#define IF_VALGRIND(A,B) B -#endif - -#if defined(HAVE_VALGRIND_MEMCHECK_H) && defined(HAVE_valgrind) +#if __has_feature(memory_sanitizer) +# include +# define HAVE_valgrind +# define MEM_UNDEFINED(a,len) __msan_allocated_memory(a,len) +# define MEM_MAKE_DEFINED(a,len) __msan_unpoison(a,len) +# define MEM_NOACCESS(a,len) ((void) 0) +# define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) +# define MEM_CHECK_DEFINED(a,len) __msan_check_mem_is_initialized(a,len) +# define MEM_GET_VBITS(a,b,len) __msan_copy_shadow(b,a,len) +# define MEM_SET_VBITS(a,b,len) __msan_copy_shadow(a,b,len) +# define REDZONE_SIZE 8 +#elif defined(HAVE_VALGRIND_MEMCHECK_H) && defined(HAVE_valgrind) # include -# define HAVE_valgrind_or_MSAN # define MEM_UNDEFINED(a,len) VALGRIND_MAKE_MEM_UNDEFINED(a,len) # define MEM_MAKE_DEFINED(a,len) VALGRIND_MAKE_MEM_DEFINED(a,len) # define MEM_NOACCESS(a,len) VALGRIND_MAKE_MEM_NOACCESS(a,len) @@ -53,17 +57,6 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define MEM_GET_VBITS(a,b,len) ((void) 0) # define MEM_SET_VBITS(a,b,len) ((void) 0) # define REDZONE_SIZE 8 -#elif __has_feature(memory_sanitizer) -# include -# define HAVE_valgrind_or_MSAN -# define MEM_UNDEFINED(a,len) __msan_allocated_memory(a,len) -# define MEM_MAKE_DEFINED(a,len) __msan_unpoison(a,len) -# define MEM_NOACCESS(a,len) ((void) 0) -# define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) -# define MEM_CHECK_DEFINED(a,len) __msan_check_mem_is_initialized(a,len) -# define MEM_GET_VBITS(a,b,len) __msan_copy_shadow(b,a,len) -# define MEM_SET_VBITS(a,b,len) __msan_copy_shadow(a,b,len) -# define REDZONE_SIZE 8 #else # define MEM_UNDEFINED(a,len) ((void) (a), (void) (len)) # define MEM_MAKE_DEFINED(a,len) ((void) 0) @@ -73,7 +66,14 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ # define MEM_GET_VBITS(a,b,len) ((void) 0) # define MEM_SET_VBITS(a,b,len) ((void) 0) # define REDZONE_SIZE 0 -#endif /* HAVE_VALGRIND_MEMCHECK_H */ +#endif /* __has_feature(memory_sanitizer) */ + +#ifdef HAVE_valgrind +#define IF_VALGRIND(A,B) A +#else +#define IF_VALGRIND(A,B) B +#endif + #ifdef TRASH_FREED_MEMORY /* diff --git a/mysql-test/main/sp-big.result b/mysql-test/main/sp-big.result index e12136eb36d..611ac9b74e9 100644 --- a/mysql-test/main/sp-big.result +++ b/mysql-test/main/sp-big.result @@ -80,8 +80,8 @@ end// insert t1 select seq, seq, 1, 1, seq, seq, seq from seq_1_to_2000; set @before=unix_timestamp(); call select_test(); -select unix_timestamp() - @before < 60; -unix_timestamp() - @before < 60 +select unix_timestamp() - @before < @time; +unix_timestamp() - @before < @time 1 drop procedure select_test; drop table t1; diff --git a/mysql-test/main/sp-big.test b/mysql-test/main/sp-big.test index 4220541697e..043e737105a 100644 --- a/mysql-test/main/sp-big.test +++ b/mysql-test/main/sp-big.test @@ -112,6 +112,17 @@ delimiter ;// insert t1 select seq, seq, 1, 1, seq, seq, seq from seq_1_to_2000; set @before=unix_timestamp(); call select_test(); -select unix_timestamp() - @before < 60; + +--let $time=60 +if ($VALGRIND_TEST) +{ + --let $time=600 +} + +--disable_query_log +--eval set @time=$time; +--enable_query_log + +select unix_timestamp() - @before < @time; drop procedure select_test; drop table t1; diff --git a/sql/field.cc b/sql/field.cc index 1e2a711c956..8accfb35b0b 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -7686,7 +7686,7 @@ my_decimal *Field_varstring::val_decimal(my_decimal *decimal_value) } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind void Field_varstring::mark_unused_memory_as_defined() { uint used_length= get_length(); diff --git a/sql/field.h b/sql/field.h index 34d6684571b..83cdb903b20 100644 --- a/sql/field.h +++ b/sql/field.h @@ -826,7 +826,7 @@ public: return store(ls.str, (uint) ls.length, cs); } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind /** Mark unused memory in the field as defined. Mainly used to ensure that if we write full field to disk (for example in @@ -3466,7 +3466,7 @@ public: bool memcpy_field_possible(const Field *from) const; int store(const char *to,size_t length,CHARSET_INFO *charset); using Field_str::store; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind void mark_unused_memory_as_defined(); #endif double val_real(void); @@ -4395,7 +4395,8 @@ public: :Type_handler_hybrid_field_type(&type_handler_null), compression_method_ptr(0), comment(null_clex_str), - on_update(NULL), length(0), invisible(VISIBLE), decimals(0), + on_update(NULL), length(0), invisible(VISIBLE), char_length(0), + decimals(0), flags(0), pack_length(0), key_length(0), unireg_check(Field::NONE), interval(0), charset(&my_charset_bin), srid(0), geom_type(Field::GEOM_GEOMETRY), diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 8165474ccea..e19f21de006 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -2548,7 +2548,7 @@ register_query_cache_dependant_tables(THD *thd, sub_elem= subpart_it++; part= i * num_subparts + j; /* we store the end \0 as part of the key */ - end= strmov(engine_pos, sub_elem->partition_name); + end= strmov(engine_pos, sub_elem->partition_name) + 1; length= (uint)(end - engine_key); /* Copy the suffix also to query cache key */ memcpy(query_cache_key_end, engine_key_end, (end - engine_key_end)); diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index ee371b8f896..a0a3631b15b 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -1176,6 +1176,8 @@ longlong Item_func_truth::val_int() bool Item_in_optimizer::is_top_level_item() { + if (invisible_mode()) + return FALSE; return ((Item_in_subselect *)args[1])->is_top_level_item(); } diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index ce2427bce91..c0a98aab183 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -717,7 +717,7 @@ bool Item_subselect::exec() push_warning_printf(thd, Sql_condition::WARN_LEVEL_NOTE, ER_UNKNOWN_ERROR, "DBUG: Item_subselect::exec %.*s", - print.length(),print.c_ptr()); + print.length(),print.ptr()); ); /* Do not execute subselect in case of a fatal error diff --git a/sql/opt_range.cc b/sql/opt_range.cc index a936184de0d..0edc1665ac9 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -2457,6 +2457,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, param.imerge_cost_buff_size= 0; param.using_real_indexes= TRUE; param.remove_jump_scans= TRUE; + param.is_ror_scan= 0; param.remove_false_where_parts= remove_false_parts_of_where; param.force_default_mrr= ordered_output; param.possible_keys.clear_all(); diff --git a/sql/session_tracker.cc b/sql/session_tracker.cc index 65d600b9b5a..7133e45ab11 100644 --- a/sql/session_tracker.cc +++ b/sql/session_tracker.cc @@ -380,9 +380,10 @@ bool Session_sysvars_tracker::enable(THD *thd) bool Session_sysvars_tracker::update(THD *thd, set_var *var) { vars_list tool_list; + size_t length= 1; void *copy= var->save_result.string_value.str ? my_memdup(var->save_result.string_value.str, - var->save_result.string_value.length + 1, + length= var->save_result.string_value.length + 1, MYF(MY_WME | MY_THREAD_SPECIFIC)) : my_strdup("", MYF(MY_WME | MY_THREAD_SPECIFIC)); @@ -402,7 +403,7 @@ bool Session_sysvars_tracker::update(THD *thd, set_var *var) m_parsed= true; orig_list.copy(&tool_list, thd); orig_list.construct_var_list(thd->variables.session_track_system_variables, - var->save_result.string_value.length + 1); + length); return false; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 1738cf5d18f..88a1cdedeac 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -6176,9 +6176,11 @@ add_ft_keys(DYNAMIC_ARRAY *keyuse_array, keyuse.keypart= FT_KEYPART; keyuse.used_tables=cond_func->key_item()->used_tables(); keyuse.optimize= 0; + keyuse.ref_table_rows= 0; keyuse.keypart_map= 0; keyuse.sj_pred_no= UINT_MAX; keyuse.validity_ref= 0; + keyuse.null_rejecting= FALSE; return insert_dynamic(keyuse_array,(uchar*) &keyuse); } @@ -17966,25 +17968,31 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, inherit the default value that is defined for the field referred by the Item_field object from which 'field' has been created. */ - const Field *orig_field= default_field[i]; + Field *orig_field= default_field[i]; /* Get the value from default_values */ if (orig_field->is_null_in_record(orig_field->table->s->default_values)) field->set_null(); else { + /* + Copy default value. We have to use field_conv() for copy, instead of + memcpy(), because bit_fields may be stored differently + */ + my_ptrdiff_t ptr_diff= (orig_field->table->s->default_values - + orig_field->table->record[0]); field->set_notnull(); - memcpy(field->ptr, - orig_field->ptr_in_record(orig_field->table->s->default_values), - field->pack_length_in_rec()); + orig_field->move_field_offset(ptr_diff); + field_conv(field, orig_field); + orig_field->move_field_offset(-ptr_diff); } - } + } if (from_field[i]) { /* Not a table Item */ copy->set(field,from_field[i],save_sum_fields); copy++; } - length=field->pack_length(); + length=field->pack_length_in_rec(); pos+= length; /* Make entry for create table */ @@ -18006,6 +18014,9 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, // fix table name in field entry field->set_table_name(&table->alias); } + /* Handle group_null_items */ + bzero(pos, table->s->reclength - (pos - table->record[0])); + MEM_CHECK_DEFINED(table->record[0], table->s->reclength); param->copy_field_end=copy; param->recinfo= recinfo; // Pointer to after last field @@ -18275,8 +18286,9 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, goto err; } - // Make empty record so random data is not written to disk - empty_record(table); + /* record[0] and share->default_values should now have been set up */ + MEM_CHECK_DEFINED(table->record[0], table->s->reclength); + MEM_CHECK_DEFINED(share->default_values, table->s->reclength); thd->mem_root= mem_root_save; @@ -18571,7 +18583,11 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo, (*recinfo)->type= FIELD_CHECK; (*recinfo)->length= MARIA_UNIQUE_HASH_LENGTH; (*recinfo)++; - share->reclength+= MARIA_UNIQUE_HASH_LENGTH; + + /* Avoid warnings from valgrind */ + bzero(table->record[0]+ share->reclength, MARIA_UNIQUE_HASH_LENGTH); + bzero(share->default_values+ share->reclength, MARIA_UNIQUE_HASH_LENGTH); + share->reclength+= MARIA_UNIQUE_HASH_LENGTH; } else { @@ -18765,7 +18781,10 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo, (*recinfo)->type= FIELD_CHECK; (*recinfo)->length=MI_UNIQUE_HASH_LENGTH; (*recinfo)++; - share->reclength+=MI_UNIQUE_HASH_LENGTH; + /* Avoid warnings from valgrind */ + bzero(table->record[0]+ share->reclength, MI_UNIQUE_HASH_LENGTH); + bzero(share->default_values+ share->reclength, MI_UNIQUE_HASH_LENGTH); + share->reclength+= MI_UNIQUE_HASH_LENGTH; } else { @@ -19357,11 +19376,11 @@ bool instantiate_tmp_table(TABLE *table, KEY *keyinfo, If it is not heap (in-memory) table then convert index to unique constrain. */ + MEM_CHECK_DEFINED(table->record[0], table->s->reclength); if (create_internal_tmp_table(table, keyinfo, start_recinfo, recinfo, options)) return TRUE; - // Make empty record so random data is not written to disk - empty_record(table); + MEM_CHECK_DEFINED(table->record[0], table->s->reclength); } if (open_tmp_table(table)) return TRUE; @@ -27698,7 +27717,6 @@ AGGR_OP::prepare_tmp_table() join->select_options)) return true; (void) table->file->extra(HA_EXTRA_WRITE_CACHE); - empty_record(table); } /* If it wasn't already, start index scan for grouping using table index. */ if (!table->file->inited && table->group && diff --git a/sql/table.cc b/sql/table.cc index c0cfc5ca3db..ec2d86e232e 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1644,8 +1644,7 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, share->rec_buff_length= rec_buff_length; if (!(record= (uchar *) alloc_root(&share->mem_root, rec_buff_length))) goto err; /* purecov: inspected */ - MEM_NOACCESS(record, rec_buff_length); - MEM_UNDEFINED(record, share->reclength); + MEM_NOACCESS(record + share->reclength, rec_buff_length - share->reclength); share->default_values= record; memcpy(record, frm_image + record_offset, share->reclength); @@ -3273,7 +3272,6 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, if (!(record= (uchar*) alloc_root(&outparam->mem_root, share->rec_buff_length * records))) goto err; /* purecov: inspected */ - MEM_NOACCESS(record, share->rec_buff_length * records); } for (i= 0; i < 3;) @@ -3282,8 +3280,10 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, if (++i < records) record+= share->rec_buff_length; } + /* Mark bytes between records as not accessable to catch overrun bugs */ for (i= 0; i < records; i++) - MEM_UNDEFINED(outparam->record[i], share->reclength); + MEM_NOACCESS(outparam->record[i] + share->reclength, + share->rec_buff_length - share->reclength); if (!(field_ptr = (Field **) alloc_root(&outparam->mem_root, (uint) ((share->fields+1)* diff --git a/sql/unireg.cc b/sql/unireg.cc index b7eb1cd3457..cccb09a9fb9 100644 --- a/sql/unireg.cc +++ b/sql/unireg.cc @@ -436,8 +436,8 @@ LEX_CUSTRING build_frm_image(THD *thd, const LEX_CSTRING *table, pos+= create_info->comment.length; } - memcpy(frm_ptr + filepos, forminfo, 288); - pos= frm_ptr + filepos + 288; + memcpy(frm_ptr + filepos, forminfo, FRM_FORMINFO_SIZE); + pos= frm_ptr + filepos + FRM_FORMINFO_SIZE; if (pack_fields(&pos, create_fields, create_info, data_offset)) goto err; diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index e7650b20507..d9684528195 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -1179,12 +1179,12 @@ btr_cur_search_to_nth_level_func( ut_ad(!(index->type & DICT_FTS)); ut_ad(index->page != FIL_NULL); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&cursor->up_match, sizeof cursor->up_match); MEM_UNDEFINED(&cursor->up_bytes, sizeof cursor->up_bytes); MEM_UNDEFINED(&cursor->low_match, sizeof cursor->low_match); MEM_UNDEFINED(&cursor->low_bytes, sizeof cursor->low_bytes); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ #ifdef UNIV_DEBUG cursor->up_match = ULINT_UNDEFINED; cursor->low_match = ULINT_UNDEFINED; @@ -3295,12 +3295,12 @@ btr_cur_optimistic_insert( const page_size_t& page_size = block->page.size; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind if (page_size.is_compressed()) { MEM_CHECK_DEFINED(page, page_size.logical()); MEM_CHECK_DEFINED(block->page.zip.data, page_size.physical()); } -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ leaf = page_is_leaf(page); diff --git a/storage/innobase/buf/buf0buddy.cc b/storage/innobase/buf/buf0buddy.cc index ed36bcb9703..f1849dc28bf 100644 --- a/storage/innobase/buf/buf0buddy.cc +++ b/storage/innobase/buf/buf0buddy.cc @@ -393,9 +393,9 @@ buf_buddy_block_free( HASH_DELETE(buf_page_t, hash, buf_pool->zip_hash, fold, bpage); ut_d(memset(buf, 0, srv_page_size)); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(buf, srv_page_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ block = (buf_block_t*) bpage; buf_page_mutex_enter(block); diff --git a/storage/innobase/buf/buf0lru.cc b/storage/innobase/buf/buf0lru.cc index c81975a1f2c..3b058ee4fd8 100644 --- a/storage/innobase/buf/buf0lru.cc +++ b/storage/innobase/buf/buf0lru.cc @@ -1609,13 +1609,13 @@ func_exit: order to avoid bogus Valgrind or MSAN warnings.*/ buf_block_t* block = reinterpret_cast(bpage); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_MAKE_DEFINED(block->frame, srv_page_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ btr_search_drop_page_hash_index(block); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(block->frame, srv_page_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ buf_pool_mutex_enter(buf_pool); @@ -1660,9 +1660,9 @@ buf_LRU_block_free_non_file_page( buf_block_set_state(block, BUF_BLOCK_NOT_USED); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(block->frame, srv_page_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ /* Wipe page_no and space_id */ memset(block->frame + FIL_PAGE_OFFSET, 0xfe, 4); memset(block->frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID, 0xfe, 4); diff --git a/storage/innobase/data/data0data.cc b/storage/innobase/data/data0data.cc index 49bb8715a51..47e58f1614a 100644 --- a/storage/innobase/data/data0data.cc +++ b/storage/innobase/data/data0data.cc @@ -229,7 +229,7 @@ dtuple_validate( const dtuple_t* tuple) /*!< in: tuple */ { ut_ad(tuple->magic_n == DATA_TUPLE_MAGIC_N); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind const ulint n_fields = dtuple_get_n_fields(tuple); for (ulint i = 0; i < n_fields; i++) { @@ -240,7 +240,7 @@ dtuple_validate( dfield_get_len(field)); } } -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ ut_ad(dtuple_check_typed(tuple)); return(TRUE); diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 8928e4af5dc..4fa5fa4aa98 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -2281,7 +2281,8 @@ fil_check_pending_ops(const fil_space_t* space, ulint count) if (ulint n_pending_ops = my_atomic_loadlint(&space->n_pending_ops)) { - if (count > 5000) { + /* Give a warning every 10 second, starting after 1 second */ + if ((count % 500) == 50) { ib::warn() << "Trying to close/delete/truncate" " tablespace '" << space->name << "' but there are " << n_pending_ops diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index f4d3b49c4a4..46b53d87bf3 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -7334,9 +7334,9 @@ build_template_field( ut_ad(clust_index->table == index->table); templ = prebuilt->mysql_template + prebuilt->n_template++; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(templ, sizeof *templ); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ templ->is_virtual = !field->stored_in_db(); if (!templ->is_virtual) { @@ -8454,9 +8454,9 @@ calc_row_difference( /* The field has changed */ ufield = uvect->fields + n_changed; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(ufield, sizeof *ufield); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ /* Let us use a dummy dfield to make the conversion from the MySQL column format to the InnoDB format */ diff --git a/storage/innobase/include/dict0stats.ic b/storage/innobase/include/dict0stats.ic index 98024935e16..3853af2d409 100644 --- a/storage/innobase/include/dict0stats.ic +++ b/storage/innobase/include/dict0stats.ic @@ -187,7 +187,7 @@ dict_stats_deinit( table->stat_initialized = FALSE; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&table->stat_n_rows, sizeof table->stat_n_rows); MEM_UNDEFINED(&table->stat_clustered_index_size, sizeof table->stat_clustered_index_size); @@ -220,7 +220,7 @@ dict_stats_deinit( &index->stat_n_leaf_pages, sizeof(index->stat_n_leaf_pages)); } -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ dict_table_stats_unlock(table, RW_X_LATCH); } diff --git a/storage/innobase/include/srv0mon.h b/storage/innobase/include/srv0mon.h index 84e8ece2d77..eaf47789486 100644 --- a/storage/innobase/include/srv0mon.h +++ b/storage/innobase/include/srv0mon.h @@ -654,14 +654,14 @@ Use MONITOR_DEC if appropriate mutex protection exists. } \ } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind # define MONITOR_CHECK_DEFINED(value) do { \ mon_type_t m = value; \ MEM_CHECK_DEFINED(&m, sizeof m); \ } while (0) -#else /* HAVE_valgrind_or_MSAN */ +#else /* HAVE_valgrind */ # define MONITOR_CHECK_DEFINED(value) (void) 0 -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ #define MONITOR_INC_VALUE(monitor, value) \ MONITOR_CHECK_DEFINED(value); \ diff --git a/storage/innobase/page/page0cur.cc b/storage/innobase/page/page0cur.cc index 46b2c73cf37..149f3be83d9 100644 --- a/storage/innobase/page/page0cur.cc +++ b/storage/innobase/page/page0cur.cc @@ -1248,7 +1248,7 @@ page_cur_insert_rec_low( /* 1. Get the size of the physical record in the page */ rec_size = rec_offs_size(offsets); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind { const void* rec_start = rec - rec_offs_extra_size(offsets); @@ -1263,7 +1263,7 @@ page_cur_insert_rec_low( /* The variable-length header must be valid. */ MEM_CHECK_DEFINED(rec_start, extra_size); } -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ /* 2. Try to find suitable space from page memory management */ @@ -1478,7 +1478,7 @@ page_cur_insert_rec_zip( /* 1. Get the size of the physical record in the page */ rec_size = rec_offs_size(offsets); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind { const void* rec_start = rec - rec_offs_extra_size(offsets); @@ -1493,7 +1493,7 @@ page_cur_insert_rec_zip( /* The variable-length header must be valid. */ MEM_CHECK_DEFINED(rec_start, extra_size); } -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ const bool reorg_before_insert = page_has_garbage(page) && rec_size > page_get_max_insert_size(page, 1) diff --git a/storage/innobase/row/row0ftsort.cc b/storage/innobase/row/row0ftsort.cc index c5b6276caf5..dde46b7aae1 100644 --- a/storage/innobase/row/row0ftsort.cc +++ b/storage/innobase/row/row0ftsort.cc @@ -998,14 +998,14 @@ exit: goto func_exit; } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(block[i], srv_sort_buf_size); if (crypt_block[i]) { MEM_UNDEFINED(crypt_block[i], srv_sort_buf_size); } -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ } buf[i] = row_merge_buf_empty(buf[i]); diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index f659cd4a0a1..adef0e53266 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -1243,10 +1243,10 @@ row_ins_foreign_check_on_constraint( update->info_bits = 0; update->n_fields = foreign->n_fields; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(update->fields, update->n_fields * sizeof *update->fields); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ bool affects_fulltext = false; diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/row0log.cc index 97cd7c2a92b..43deb344346 100644 --- a/storage/innobase/row/row0log.cc +++ b/storage/innobase/row/row0log.cc @@ -372,9 +372,9 @@ row_log_online_op( goto err_exit; } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ ut_ad(log->tail.bytes < srv_sort_buf_size); avail_size = srv_sort_buf_size - log->tail.bytes; @@ -459,10 +459,10 @@ write_failed: index->type |= DICT_CORRUPT; } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.block, srv_sort_buf_size); MEM_UNDEFINED(buf, srv_sort_buf_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ memcpy(log->tail.block, log->tail.buf + avail_size, mrec_size - avail_size); @@ -472,9 +472,9 @@ write_failed: ut_ad(b == log->tail.block + log->tail.bytes); } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ err_exit: mutex_exit(&log->mutex); } @@ -506,9 +506,9 @@ row_log_table_open( { mutex_enter(&log->mutex); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ if (log->error != DB_SUCCESS) { err_exit: @@ -600,10 +600,10 @@ row_log_table_close_func( write_failed: log->error = DB_ONLINE_LOG_TOO_BIG; } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.block, srv_sort_buf_size); MEM_UNDEFINED(buf, srv_sort_buf_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ memcpy(log->tail.block, log->tail.buf + avail, size - avail); log->tail.bytes = size - avail; } else { @@ -612,9 +612,9 @@ write_failed: } log->tail.total += size; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ err_exit: mutex_exit(&log->mutex); @@ -2785,9 +2785,9 @@ row_log_table_apply_ops( ut_ad(new_trx_id_col > 0); ut_ad(new_trx_id_col != ULINT_UNDEFINED); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&mrec_end, sizeof mrec_end); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ offsets = static_cast(ut_malloc_nokey(i * sizeof *offsets)); rec_offs_set_n_alloc(offsets, i); @@ -3696,9 +3696,9 @@ row_log_apply_ops( ut_ad(!index->is_committed()); ut_ad(rw_lock_own(dict_index_get_lock(index), RW_LOCK_X)); ut_ad(index->online_log); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&mrec_end, sizeof mrec_end); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ offsets = static_cast(ut_malloc_nokey(i * sizeof *offsets)); rec_offs_set_n_alloc(offsets, i); diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index 3d21d1d2efc..2bfbbd0ba09 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -1027,11 +1027,11 @@ row_merge_buf_write( ut_a(b < &block[srv_sort_buf_size]); ut_a(b == &block[0] + buf->total_size); *b++ = 0; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind /* The rest of the block is uninitialized. Initialize it to avoid bogus warnings. */ memset(b, 0xff, &block[srv_sort_buf_size] - b); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ DBUG_LOG("ib_merge_sort", "write " << reinterpret_cast(b) << ',' << of->fd << ',' << of->offset << " EOF"); @@ -1424,9 +1424,9 @@ row_merge_write_rec( return(NULL); } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&block[0], srv_sort_buf_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ /* Copy the rest. */ b = &block[0]; @@ -1477,7 +1477,7 @@ row_merge_write_eof( DBUG_RETURN(NULL); } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&block[0], srv_sort_buf_size); #endif DBUG_RETURN(&block[0]); @@ -2680,10 +2680,10 @@ write_buffers: break; } -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED( &block[0], srv_sort_buf_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ } } merge_buf[i] = row_merge_buf_empty(buf); @@ -3203,9 +3203,9 @@ row_merge( foffs0 = 0; foffs1 = ihalf; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(run_offset, *num_run * sizeof *run_offset); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ for (; foffs0 < ihalf && foffs1 < file->offset; foffs0++, foffs1++) { @@ -3286,9 +3286,9 @@ row_merge( *tmpfd = file->fd; *file = of; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&block[0], 3 * srv_sort_buf_size); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ return(DB_SUCCESS); } diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index c3dc4e14094..c4c05888eb8 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -982,11 +982,11 @@ row_sel_get_clust_rec( switch (err) { case DB_SUCCESS: case DB_SUCCESS_LOCKED_REC: -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind /* Declare the variable uninitialized. It should be set to DB_SUCCESS at func_exit. */ MEM_UNDEFINED(&err, sizeof err); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ break; default: goto err_exit; @@ -2742,9 +2742,9 @@ row_sel_field_store_in_mysql_format_func( ut_ad(len != UNIV_SQL_NULL); MEM_CHECK_DEFINED(data, len); MEM_CHECK_ADDRESSABLE(dest, templ->mysql_col_len); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(dest, templ->mysql_col_len); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ switch (templ->type) { const byte* field_end; @@ -3653,9 +3653,9 @@ row_sel_copy_cached_field_for_mysql( row_mysql_read_true_varchar( &len, cache, templ->mysql_length_bytes); len += templ->mysql_length_bytes; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(buf, templ->mysql_col_len); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ } else { len = templ->mysql_col_len; } @@ -3724,9 +3724,9 @@ row_sel_dequeue_cached_row_for_mysql( /* The record is long. Copy it field by field, in case there are some long VARCHAR column of which only a small length is being used. */ -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(buf, prebuilt->mysql_prefix_len); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ /* First copy the NULL bits. */ ut_memcpy(buf, cached_rec, prebuilt->null_bitmap_len); @@ -3810,10 +3810,10 @@ row_sel_fetch_last_buf( } ut_ad(prebuilt->fetch_cache_first == 0); -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(prebuilt->fetch_cache[prebuilt->n_fetch_cached], prebuilt->mysql_row_len); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ return(prebuilt->fetch_cache[prebuilt->n_fetch_cached]); } diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index ee432fdddb7..f63347e8589 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -1869,9 +1869,9 @@ row_upd_changes_ord_field_binary_func( /* Silence a compiler warning without silencing a Valgrind error. */ dfield_len = 0; -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind MEM_UNDEFINED(&dfield_len, sizeof dfield_len); -#endif /* HAVE_valgrind_or_MSAN */ +#endif /* HAVE_valgrind */ /* See if the column is stored externally. */ buf = row_ext_lookup(ext, col_no, &dfield_len); diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index 2e761cd7a16..c9b559c7307 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -458,7 +458,7 @@ void trx_free(trx_t*& trx) MEM_UNDEFINED(&trx->state, sizeof trx->state); MEM_UNDEFINED(&trx->mysql_thd, sizeof trx->mysql_thd); #endif -#ifdef HAVE_valgrind_or_MSAN +#ifdef HAVE_valgrind /* Unpoison the memory for innodb_monitor_set_option; it is operating also on the freed transaction objects. We checked that these were initialized in diff --git a/storage/tokudb/ha_tokudb.cc b/storage/tokudb/ha_tokudb.cc index 4a101f7e589..2f13e4cdbb9 100644 --- a/storage/tokudb/ha_tokudb.cc +++ b/storage/tokudb/ha_tokudb.cc @@ -6123,6 +6123,11 @@ void ha_tokudb::position(const uchar * record) { // memcpy(ref, &key.size, sizeof(uint32_t)); } + /* + tokudb doesn't always write the last byte. Don't that cause problems with + MariaDB + */ + MEM_MAKE_DEFINED(ref, ref_length); TOKUDB_HANDLER_DBUG_VOID_RETURN; } From b6ec1e8bbf0ffca2d715aded694722e0c4b5d484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 2 Jul 2020 16:52:13 +0300 Subject: [PATCH 05/26] MDEV-20377 post-fix: Introduce MEM_MAKE_ADDRESSABLE In AddressSanitizer, we only want memory poisoning to happen in connection with custom memory allocation or freeing. The primary use of MEM_UNDEFINED is for declaring memory uninitialized in Valgrind or MemorySanitizer. We do not want MEM_UNDEFINED to have the unwanted side effect that AddressSanitizer would no longer be able to complain about accessing unallocated memory. MEM_UNDEFINED(): Define as no-op for AddressSanitizer. MEM_MAKE_ADDRESSABLE(): Define as MEM_UNDEFINED() or ASAN_UNPOISON_MEMORY_REGION(). MEM_CHECK_ADDRESSABLE(): Wrap also __asan_region_is_poisoned(). --- include/my_valgrind.h | 29 +++++++++++++++------------ mysys/my_alloc.c | 2 +- sql/table.cc | 1 + storage/innobase/btr/btr0cur.cc | 2 -- storage/innobase/buf/buf0buddy.cc | 2 -- storage/innobase/buf/buf0lru.cc | 8 +------- storage/innobase/handler/ha_innodb.cc | 4 ---- storage/innobase/include/mem0mem.ic | 2 +- storage/innobase/include/ut0pool.h | 14 ++++++------- storage/innobase/os/os0proc.cc | 2 +- storage/innobase/page/page0zip.cc | 4 ++-- storage/innobase/row/row0ins.cc | 2 -- storage/innobase/row/row0log.cc | 18 ++--------------- storage/innobase/row/row0merge.cc | 10 --------- storage/innobase/row/row0sel.cc | 10 --------- storage/innobase/row/row0upd.cc | 2 -- storage/innobase/sync/sync0arr.cc | 2 +- storage/innobase/trx/trx0trx.cc | 8 +++----- 18 files changed, 35 insertions(+), 87 deletions(-) diff --git a/include/my_valgrind.h b/include/my_valgrind.h index c181a8c6b35..62794a2d70c 100644 --- a/include/my_valgrind.h +++ b/include/my_valgrind.h @@ -28,6 +28,7 @@ # include # define HAVE_valgrind # define MEM_UNDEFINED(a,len) __msan_allocated_memory(a,len) +# define MEM_MAKE_ADDRESSABLE(a,len) MEM_UNDEFINED(a,len) # define MEM_MAKE_DEFINED(a,len) __msan_unpoison(a,len) # define MEM_NOACCESS(a,len) ((void) 0) # define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) @@ -38,6 +39,7 @@ #elif defined(HAVE_VALGRIND_MEMCHECK_H) && defined(HAVE_valgrind) # include # define MEM_UNDEFINED(a,len) VALGRIND_MAKE_MEM_UNDEFINED(a,len) +# define MEM_MAKE_ADDRESSABLE(a,len) MEM_UNDEFINED(a,len) # define MEM_MAKE_DEFINED(a,len) VALGRIND_MAKE_MEM_DEFINED(a,len) # define MEM_NOACCESS(a,len) VALGRIND_MAKE_MEM_NOACCESS(a,len) # define MEM_CHECK_ADDRESSABLE(a,len) VALGRIND_CHECK_MEM_IS_ADDRESSABLE(a,len) @@ -49,16 +51,19 @@ # include /* How to do manual poisoning: https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ -# define MEM_UNDEFINED(a,len) ASAN_UNPOISON_MEMORY_REGION(a,len) +# define MEM_UNDEFINED(a,len) ((void) 0) +# define MEM_MAKE_ADDRESSABLE(a,len) ASAN_UNPOISON_MEMORY_REGION(a,len) # define MEM_MAKE_DEFINED(a,len) ((void) 0) # define MEM_NOACCESS(a,len) ASAN_POISON_MEMORY_REGION(a,len) -# define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) +# define MEM_CHECK_ADDRESSABLE(a,len) \ + assert(!__asan_region_is_poisoned((void*) a,len)) # define MEM_CHECK_DEFINED(a,len) ((void) 0) # define MEM_GET_VBITS(a,b,len) ((void) 0) # define MEM_SET_VBITS(a,b,len) ((void) 0) # define REDZONE_SIZE 8 #else -# define MEM_UNDEFINED(a,len) ((void) (a), (void) (len)) +# define MEM_UNDEFINED(a,len) ((void) 0) +# define MEM_MAKE_ADDRESSABLE(a,len) ((void) 0) # define MEM_MAKE_DEFINED(a,len) ((void) 0) # define MEM_NOACCESS(a,len) ((void) 0) # define MEM_CHECK_ADDRESSABLE(a,len) ((void) 0) @@ -74,24 +79,22 @@ https://github.com/google/sanitizers/wiki/AddressSanitizerManualPoisoning */ #define IF_VALGRIND(A,B) B #endif - #ifdef TRASH_FREED_MEMORY /* - TRASH_FILL() has to call MEM_UNDEFINED() to cancel any effect of TRASH_FREE(). + _TRASH_FILL() has to call MEM_MAKE_ADDRESSABLE() to cancel any effect of + TRASH_FREE(). This can happen in the case one does TRASH_ALLOC(A,B) ; TRASH_FREE(A,B) ; TRASH_ALLOC(A,B) to reuse the same memory in an internal memory allocator like MEM_ROOT. - For my_malloc() and safemalloc() the extra MEM_UNDEFINED is bit of an - overkill. - TRASH_FILL() is an internal function and should not be used externally. + _TRASH_FILL() is an internal function and should not be used externally. */ -#define TRASH_FILL(A,B,C) do { const size_t trash_tmp= (B); MEM_UNDEFINED(A, trash_tmp); memset(A, C, trash_tmp); } while (0) +#define _TRASH_FILL(A,B,C) do { const size_t trash_tmp= (B); MEM_MAKE_ADDRESSABLE(A, trash_tmp); memset(A, C, trash_tmp); } while (0) #else -#define TRASH_FILL(A,B,C) do { MEM_UNDEFINED((A), (B)); } while (0) +#define _TRASH_FILL(A,B,C) do { MEM_UNDEFINED((A), (B)); } while (0) #endif -/** Note that some memory became allocated or uninitialized. */ -#define TRASH_ALLOC(A,B) do { TRASH_FILL(A,B,0xA5); MEM_UNDEFINED(A,B); } while(0) +/** Note that some memory became allocated and/or uninitialized. */ +#define TRASH_ALLOC(A,B) do { _TRASH_FILL(A,B,0xA5); MEM_MAKE_ADDRESSABLE(A,B); } while(0) /** Note that some memory became freed. (Prohibit further access to it.) */ -#define TRASH_FREE(A,B) do { TRASH_FILL(A,B,0x8F); MEM_NOACCESS(A,B); } while(0) +#define TRASH_FREE(A,B) do { _TRASH_FILL(A,B,0x8F); MEM_NOACCESS(A,B); } while(0) #endif /* MY_VALGRIND_INCLUDED */ diff --git a/mysys/my_alloc.c b/mysys/my_alloc.c index b24e9c21623..c5fe75c1410 100644 --- a/mysys/my_alloc.c +++ b/mysys/my_alloc.c @@ -212,7 +212,7 @@ void *alloc_root(MEM_ROOT *mem_root, size_t length) uchar* point; reg1 USED_MEM *next= 0; reg2 USED_MEM **prev; - size_t original_length = length; + size_t original_length __attribute__((unused)) = length; DBUG_ENTER("alloc_root"); DBUG_PRINT("enter",("root: %p name: %s", mem_root, mem_root->name)); DBUG_ASSERT(alloc_root_inited(mem_root)); diff --git a/sql/table.cc b/sql/table.cc index ec2d86e232e..7aa7abfa006 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1644,6 +1644,7 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, share->rec_buff_length= rec_buff_length; if (!(record= (uchar *) alloc_root(&share->mem_root, rec_buff_length))) goto err; /* purecov: inspected */ + /* Mark bytes after record as not accessable to catch overrun bugs */ MEM_NOACCESS(record + share->reclength, rec_buff_length - share->reclength); share->default_values= record; memcpy(record, frm_image + record_offset, share->reclength); diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index d9684528195..2d0f92aa499 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -1179,12 +1179,10 @@ btr_cur_search_to_nth_level_func( ut_ad(!(index->type & DICT_FTS)); ut_ad(index->page != FIL_NULL); -#ifdef HAVE_valgrind MEM_UNDEFINED(&cursor->up_match, sizeof cursor->up_match); MEM_UNDEFINED(&cursor->up_bytes, sizeof cursor->up_bytes); MEM_UNDEFINED(&cursor->low_match, sizeof cursor->low_match); MEM_UNDEFINED(&cursor->low_bytes, sizeof cursor->low_bytes); -#endif /* HAVE_valgrind */ #ifdef UNIV_DEBUG cursor->up_match = ULINT_UNDEFINED; cursor->low_match = ULINT_UNDEFINED; diff --git a/storage/innobase/buf/buf0buddy.cc b/storage/innobase/buf/buf0buddy.cc index f1849dc28bf..65ef42ef13e 100644 --- a/storage/innobase/buf/buf0buddy.cc +++ b/storage/innobase/buf/buf0buddy.cc @@ -393,9 +393,7 @@ buf_buddy_block_free( HASH_DELETE(buf_page_t, hash, buf_pool->zip_hash, fold, bpage); ut_d(memset(buf, 0, srv_page_size)); -#ifdef HAVE_valgrind MEM_UNDEFINED(buf, srv_page_size); -#endif /* HAVE_valgrind */ block = (buf_block_t*) bpage; buf_page_mutex_enter(block); diff --git a/storage/innobase/buf/buf0lru.cc b/storage/innobase/buf/buf0lru.cc index 3b058ee4fd8..bfd0ada1f05 100644 --- a/storage/innobase/buf/buf0lru.cc +++ b/storage/innobase/buf/buf0lru.cc @@ -806,7 +806,7 @@ buf_LRU_get_free_only( assert_block_ahi_empty(block); buf_block_set_state(block, BUF_BLOCK_READY_FOR_USE); - MEM_UNDEFINED(block->frame, srv_page_size); + MEM_MAKE_ADDRESSABLE(block->frame, srv_page_size); ut_ad(buf_pool_from_block(block) == buf_pool); @@ -1609,13 +1609,9 @@ func_exit: order to avoid bogus Valgrind or MSAN warnings.*/ buf_block_t* block = reinterpret_cast(bpage); -#ifdef HAVE_valgrind MEM_MAKE_DEFINED(block->frame, srv_page_size); -#endif /* HAVE_valgrind */ btr_search_drop_page_hash_index(block); -#ifdef HAVE_valgrind MEM_UNDEFINED(block->frame, srv_page_size); -#endif /* HAVE_valgrind */ buf_pool_mutex_enter(buf_pool); @@ -1660,9 +1656,7 @@ buf_LRU_block_free_non_file_page( buf_block_set_state(block, BUF_BLOCK_NOT_USED); -#ifdef HAVE_valgrind MEM_UNDEFINED(block->frame, srv_page_size); -#endif /* HAVE_valgrind */ /* Wipe page_no and space_id */ memset(block->frame + FIL_PAGE_OFFSET, 0xfe, 4); memset(block->frame + FIL_PAGE_ARCH_LOG_NO_OR_SPACE_ID, 0xfe, 4); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 46b53d87bf3..7234e73f28c 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -7334,9 +7334,7 @@ build_template_field( ut_ad(clust_index->table == index->table); templ = prebuilt->mysql_template + prebuilt->n_template++; -#ifdef HAVE_valgrind MEM_UNDEFINED(templ, sizeof *templ); -#endif /* HAVE_valgrind */ templ->is_virtual = !field->stored_in_db(); if (!templ->is_virtual) { @@ -8454,9 +8452,7 @@ calc_row_difference( /* The field has changed */ ufield = uvect->fields + n_changed; -#ifdef HAVE_valgrind MEM_UNDEFINED(ufield, sizeof *ufield); -#endif /* HAVE_valgrind */ /* Let us use a dummy dfield to make the conversion from the MySQL column format to the InnoDB format */ diff --git a/storage/innobase/include/mem0mem.ic b/storage/innobase/include/mem0mem.ic index c1e7348a548..9236bbef05d 100644 --- a/storage/innobase/include/mem0mem.ic +++ b/storage/innobase/include/mem0mem.ic @@ -203,7 +203,7 @@ mem_heap_alloc( mem_block_set_free(block, free + MEM_SPACE_NEEDED(n)); buf = buf + REDZONE_SIZE; - MEM_UNDEFINED(buf, n - REDZONE_SIZE); + MEM_MAKE_ADDRESSABLE(buf, n - REDZONE_SIZE); return(buf); } diff --git a/storage/innobase/include/ut0pool.h b/storage/innobase/include/ut0pool.h index 703a07a23f2..4a5f58f6fae 100644 --- a/storage/innobase/include/ut0pool.h +++ b/storage/innobase/include/ut0pool.h @@ -89,13 +89,12 @@ struct Pool { ut_ad(elem->m_pool == this); #ifdef __SANITIZE_ADDRESS__ /* Unpoison the memory for AddressSanitizer */ - MEM_UNDEFINED(&elem->m_type, sizeof elem->m_type); + MEM_MAKE_ADDRESSABLE(&elem->m_type, + sizeof elem->m_type); #endif -#ifdef HAVE_valgrind - /* Declare the contents as initialized for Valgrind; + /* Declare the contents initialized; we checked this in mem_free(). */ MEM_MAKE_DEFINED(&elem->m_type, sizeof elem->m_type); -#endif Factory::destroy(&elem->m_type); } @@ -134,14 +133,13 @@ struct Pool { if (elem) { # ifdef __SANITIZE_ADDRESS__ /* Unpoison the memory for AddressSanitizer */ - MEM_UNDEFINED(&elem->m_type, sizeof elem->m_type); + MEM_MAKE_ADDRESSABLE(&elem->m_type, + sizeof elem->m_type); # endif -# ifdef HAVE_valgrind - /* Declare the memory initialized for Valgrind. + /* Declare the memory initialized. The trx_t that are released to the pool are actually initialized; we checked that by MEM_CHECK_DEFINED() in mem_free() below. */ -# endif MEM_MAKE_DEFINED(&elem->m_type, sizeof elem->m_type); } #endif diff --git a/storage/innobase/os/os0proc.cc b/storage/innobase/os/os0proc.cc index 508a13de2ca..4a4076f4a1f 100644 --- a/storage/innobase/os/os0proc.cc +++ b/storage/innobase/os/os0proc.cc @@ -162,7 +162,7 @@ os_mem_free_large( // And we must unpoison it by ourself as specified in documentation // for __asan_poison_memory_region() in sanitizer/asan_interface.h // munmap() doesn't do it for us automatically. - MEM_UNDEFINED(ptr, size); + MEM_MAKE_ADDRESSABLE(ptr, size); #endif /* __SANITIZE_ADDRESS__ */ #ifdef HAVE_LINUX_LARGE_PAGES diff --git a/storage/innobase/page/page0zip.cc b/storage/innobase/page/page0zip.cc index ecfea3a2e90..f1f10bcd58d 100644 --- a/storage/innobase/page/page0zip.cc +++ b/storage/innobase/page/page0zip.cc @@ -1577,11 +1577,11 @@ err_exit: ut_ad(buf + c_stream.total_out == c_stream.next_out); ut_ad((ulint) (storage - c_stream.next_out) >= c_stream.avail_out); -#ifdef HAVE_valgrind +#if defined HAVE_valgrind && !__has_feature(memory_sanitizer) /* Valgrind believes that zlib does not initialize some bits in the last 7 or 8 bytes of the stream. Make Valgrind happy. */ MEM_MAKE_DEFINED(buf, c_stream.total_out); -#endif /* HAVE_valgrind */ +#endif /* HAVE_valgrind && !memory_sanitizer */ /* Zero out the area reserved for the modification log. Space for the end marker of the modification log is not diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index adef0e53266..d6596d586ab 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -1243,10 +1243,8 @@ row_ins_foreign_check_on_constraint( update->info_bits = 0; update->n_fields = foreign->n_fields; -#ifdef HAVE_valgrind MEM_UNDEFINED(update->fields, update->n_fields * sizeof *update->fields); -#endif /* HAVE_valgrind */ bool affects_fulltext = false; diff --git a/storage/innobase/row/row0log.cc b/storage/innobase/row/row0log.cc index 43deb344346..a3bf91a1c74 100644 --- a/storage/innobase/row/row0log.cc +++ b/storage/innobase/row/row0log.cc @@ -372,9 +372,7 @@ row_log_online_op( goto err_exit; } -#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind */ ut_ad(log->tail.bytes < srv_sort_buf_size); avail_size = srv_sort_buf_size - log->tail.bytes; @@ -459,10 +457,8 @@ write_failed: index->type |= DICT_CORRUPT; } -#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.block, srv_sort_buf_size); MEM_UNDEFINED(buf, srv_sort_buf_size); -#endif /* HAVE_valgrind */ memcpy(log->tail.block, log->tail.buf + avail_size, mrec_size - avail_size); @@ -472,9 +468,7 @@ write_failed: ut_ad(b == log->tail.block + log->tail.bytes); } -#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind */ err_exit: mutex_exit(&log->mutex); } @@ -506,9 +500,7 @@ row_log_table_open( { mutex_enter(&log->mutex); -#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind */ if (log->error != DB_SUCCESS) { err_exit: @@ -600,10 +592,9 @@ row_log_table_close_func( write_failed: log->error = DB_ONLINE_LOG_TOO_BIG; } -#ifdef HAVE_valgrind + MEM_UNDEFINED(log->tail.block, srv_sort_buf_size); MEM_UNDEFINED(buf, srv_sort_buf_size); -#endif /* HAVE_valgrind */ memcpy(log->tail.block, log->tail.buf + avail, size - avail); log->tail.bytes = size - avail; } else { @@ -612,9 +603,7 @@ write_failed: } log->tail.total += size; -#ifdef HAVE_valgrind MEM_UNDEFINED(log->tail.buf, sizeof log->tail.buf); -#endif /* HAVE_valgrind */ err_exit: mutex_exit(&log->mutex); @@ -2785,9 +2774,7 @@ row_log_table_apply_ops( ut_ad(new_trx_id_col > 0); ut_ad(new_trx_id_col != ULINT_UNDEFINED); -#ifdef HAVE_valgrind MEM_UNDEFINED(&mrec_end, sizeof mrec_end); -#endif /* HAVE_valgrind */ offsets = static_cast(ut_malloc_nokey(i * sizeof *offsets)); rec_offs_set_n_alloc(offsets, i); @@ -3696,9 +3683,8 @@ row_log_apply_ops( ut_ad(!index->is_committed()); ut_ad(rw_lock_own(dict_index_get_lock(index), RW_LOCK_X)); ut_ad(index->online_log); -#ifdef HAVE_valgrind + MEM_UNDEFINED(&mrec_end, sizeof mrec_end); -#endif /* HAVE_valgrind */ offsets = static_cast(ut_malloc_nokey(i * sizeof *offsets)); rec_offs_set_n_alloc(offsets, i); diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index 2bfbbd0ba09..dcc1396d2da 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -1424,9 +1424,7 @@ row_merge_write_rec( return(NULL); } -#ifdef HAVE_valgrind MEM_UNDEFINED(&block[0], srv_sort_buf_size); -#endif /* HAVE_valgrind */ /* Copy the rest. */ b = &block[0]; @@ -1477,9 +1475,7 @@ row_merge_write_eof( DBUG_RETURN(NULL); } -#ifdef HAVE_valgrind MEM_UNDEFINED(&block[0], srv_sort_buf_size); -#endif DBUG_RETURN(&block[0]); } @@ -2680,10 +2676,8 @@ write_buffers: break; } -#ifdef HAVE_valgrind MEM_UNDEFINED( &block[0], srv_sort_buf_size); -#endif /* HAVE_valgrind */ } } merge_buf[i] = row_merge_buf_empty(buf); @@ -3203,9 +3197,7 @@ row_merge( foffs0 = 0; foffs1 = ihalf; -#ifdef HAVE_valgrind MEM_UNDEFINED(run_offset, *num_run * sizeof *run_offset); -#endif /* HAVE_valgrind */ for (; foffs0 < ihalf && foffs1 < file->offset; foffs0++, foffs1++) { @@ -3286,9 +3278,7 @@ row_merge( *tmpfd = file->fd; *file = of; -#ifdef HAVE_valgrind MEM_UNDEFINED(&block[0], 3 * srv_sort_buf_size); -#endif /* HAVE_valgrind */ return(DB_SUCCESS); } diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index c4c05888eb8..1dbbc018a9c 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -982,11 +982,9 @@ row_sel_get_clust_rec( switch (err) { case DB_SUCCESS: case DB_SUCCESS_LOCKED_REC: -#ifdef HAVE_valgrind /* Declare the variable uninitialized. It should be set to DB_SUCCESS at func_exit. */ MEM_UNDEFINED(&err, sizeof err); -#endif /* HAVE_valgrind */ break; default: goto err_exit; @@ -2742,9 +2740,7 @@ row_sel_field_store_in_mysql_format_func( ut_ad(len != UNIV_SQL_NULL); MEM_CHECK_DEFINED(data, len); MEM_CHECK_ADDRESSABLE(dest, templ->mysql_col_len); -#ifdef HAVE_valgrind MEM_UNDEFINED(dest, templ->mysql_col_len); -#endif /* HAVE_valgrind */ switch (templ->type) { const byte* field_end; @@ -3653,9 +3649,7 @@ row_sel_copy_cached_field_for_mysql( row_mysql_read_true_varchar( &len, cache, templ->mysql_length_bytes); len += templ->mysql_length_bytes; -#ifdef HAVE_valgrind MEM_UNDEFINED(buf, templ->mysql_col_len); -#endif /* HAVE_valgrind */ } else { len = templ->mysql_col_len; } @@ -3724,9 +3718,7 @@ row_sel_dequeue_cached_row_for_mysql( /* The record is long. Copy it field by field, in case there are some long VARCHAR column of which only a small length is being used. */ -#ifdef HAVE_valgrind MEM_UNDEFINED(buf, prebuilt->mysql_prefix_len); -#endif /* HAVE_valgrind */ /* First copy the NULL bits. */ ut_memcpy(buf, cached_rec, prebuilt->null_bitmap_len); @@ -3810,10 +3802,8 @@ row_sel_fetch_last_buf( } ut_ad(prebuilt->fetch_cache_first == 0); -#ifdef HAVE_valgrind MEM_UNDEFINED(prebuilt->fetch_cache[prebuilt->n_fetch_cached], prebuilt->mysql_row_len); -#endif /* HAVE_valgrind */ return(prebuilt->fetch_cache[prebuilt->n_fetch_cached]); } diff --git a/storage/innobase/row/row0upd.cc b/storage/innobase/row/row0upd.cc index f63347e8589..0f700c77c36 100644 --- a/storage/innobase/row/row0upd.cc +++ b/storage/innobase/row/row0upd.cc @@ -1869,9 +1869,7 @@ row_upd_changes_ord_field_binary_func( /* Silence a compiler warning without silencing a Valgrind error. */ dfield_len = 0; -#ifdef HAVE_valgrind MEM_UNDEFINED(&dfield_len, sizeof dfield_len); -#endif /* HAVE_valgrind */ /* See if the column is stored externally. */ buf = row_ext_lookup(ext, col_no, &dfield_len); diff --git a/storage/innobase/sync/sync0arr.cc b/storage/innobase/sync/sync0arr.cc index 65f9353ae77..8e26de10dec 100644 --- a/storage/innobase/sync/sync0arr.cc +++ b/storage/innobase/sync/sync0arr.cc @@ -976,7 +976,7 @@ sync_array_print_long_waits_low( return(false); } -#ifdef HAVE_valgrind +#if defined HAVE_valgrind && !__has_feature(memory_sanitizer) /* Increase the timeouts if running under valgrind because it executes extremely slowly. HAVE_valgrind does not necessary mean that we are running under valgrind but we have no better way to tell. diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index c9b559c7307..e0d8d44a86e 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -453,12 +453,11 @@ void trx_free(trx_t*& trx) #ifdef __SANITIZE_ADDRESS__ /* Unpoison the memory for innodb_monitor_set_option; it is operating also on the freed transaction objects. */ - MEM_UNDEFINED(&trx->mutex, sizeof trx->mutex); + MEM_MAKE_ADDRESSABLE(&trx->mutex, sizeof trx->mutex); /* For innobase_kill_connection() */ - MEM_UNDEFINED(&trx->state, sizeof trx->state); - MEM_UNDEFINED(&trx->mysql_thd, sizeof trx->mysql_thd); + MEM_MAKE_ADDRESSABLE(&trx->state, sizeof trx->state); + MEM_MAKE_ADDRESSABLE(&trx->mysql_thd, sizeof trx->mysql_thd); #endif -#ifdef HAVE_valgrind /* Unpoison the memory for innodb_monitor_set_option; it is operating also on the freed transaction objects. We checked that these were initialized in @@ -467,7 +466,6 @@ void trx_free(trx_t*& trx) /* For innobase_kill_connection() */ MEM_MAKE_DEFINED(&trx->state, sizeof trx->state); MEM_MAKE_DEFINED(&trx->mysql_thd, sizeof trx->mysql_thd); -#endif trx = NULL; } From 53ecc354e398ead01b4dbf2d77170dafc29debd2 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 2 Jul 2020 23:50:56 +0300 Subject: [PATCH 06/26] Fixed errors found by MSAN --- libmysqld/libmysql.c | 1 + mysql-test/main/mysql_upgrade.test | 2 +- mysql-test/suite/csv/read_only.test | 1 + sql/sql_load.cc | 4 ++-- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/libmysqld/libmysql.c b/libmysqld/libmysql.c index 8c80e1a3f54..c4b3a17f508 100644 --- a/libmysqld/libmysql.c +++ b/libmysqld/libmysql.c @@ -1141,6 +1141,7 @@ my_bool STDCALL mysql_embedded(void) void my_net_local_init(NET *net) { net->max_packet= (uint) net_buffer_length; + net->read_timeout= net->write_timeout= 0; my_net_set_read_timeout(net, CLIENT_NET_READ_TIMEOUT); my_net_set_write_timeout(net, CLIENT_NET_WRITE_TIMEOUT); net->retry_count= 1; diff --git a/mysql-test/main/mysql_upgrade.test b/mysql-test/main/mysql_upgrade.test index 0171fe6c7ba..a3d8ba0375c 100644 --- a/mysql-test/main/mysql_upgrade.test +++ b/mysql-test/main/mysql_upgrade.test @@ -17,7 +17,7 @@ let $MYSQLD_DATADIR= `select @@datadir`; file_exists $MYSQLD_DATADIR/mysql_upgrade_info; --echo Run it again - should say already completed ---replace_result $MYSQL_SERVER_VERSION VERSION +--replace_regex /upgraded to .*, use/upgraded to VERSION, use/ --exec $MYSQL_UPGRADE 2>&1 # It should have created a file in the MySQL Servers datadir diff --git a/mysql-test/suite/csv/read_only.test b/mysql-test/suite/csv/read_only.test index 2af209182d0..a3c851a6a70 100644 --- a/mysql-test/suite/csv/read_only.test +++ b/mysql-test/suite/csv/read_only.test @@ -1,3 +1,4 @@ +--source include/not_as_root.inc # # MDEV-11883 MariaDB crashes with out-of-memory when query information_schema # diff --git a/sql/sql_load.cc b/sql/sql_load.cc index d5692196457..09af120e6cf 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -2031,8 +2031,8 @@ int READ_INFO::read_xml(THD *thd) case '=': /* attribute name end - read the value */ //check for tag field and attribute name - if(!memcmp(tag.c_ptr_safe(), STRING_WITH_LEN("field")) && - !memcmp(attribute.c_ptr_safe(), STRING_WITH_LEN("name"))) + if(!strcmp(tag.c_ptr_safe(), "field") && + !strcmp(attribute.c_ptr_safe(), "name")) { /* this is format xx From 6e81ba0c12d1b94c0c046e1062f06e426036dbb2 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 3 Jul 2020 01:16:31 +0300 Subject: [PATCH 07/26] Don't give errors for default value copy in create_tmp_table --- mysql-test/main/type_temporal_innodb.result | 15 +++++++++++++++ mysql-test/main/type_temporal_innodb.test | 19 +++++++++++++++++++ sql/sql_select.cc | 7 +++++++ 3 files changed, 41 insertions(+) diff --git a/mysql-test/main/type_temporal_innodb.result b/mysql-test/main/type_temporal_innodb.result index b869822722d..c129f4810e2 100644 --- a/mysql-test/main/type_temporal_innodb.result +++ b/mysql-test/main/type_temporal_innodb.result @@ -160,3 +160,18 @@ SELECT * FROM t1 WHERE LEAST( UTC_TIME(), d ); d 2012-12-21 DROP TABLE t1; +# +# MDEV-17969 Assertion `name' failed in THD::push_warning_truncated_value_for_field +# +CREATE TABLE t1 (c1 DATE , c2 TIMESTAMP) ENGINE=InnoDB; +INSERT INTO t1 VALUES ('2006-07-17','0000-00-00 00:00:00'); +CREATE TABLE t2 (pk INT, a1 TIME) Engine=InnoDB; +INSERT INTO t2 VALUES (6,'00:00:00'); +SET SESSION sql_mode= 'strict_all_tables,no_zero_date'; +CREATE TABLE tbl SELECT * FROM t1 WHERE t1.c1 = (SELECT c2 FROM t2 WHERE pk = 6); +ERROR 22007: Truncated incorrect datetime value: '0000-00-00 00:00:00' +DROP TABLE t1,t2; +SET sql_mode=DEFAULT; +# +# End of 10.3 tests +# diff --git a/mysql-test/main/type_temporal_innodb.test b/mysql-test/main/type_temporal_innodb.test index 81f2f586c51..94f61d54634 100644 --- a/mysql-test/main/type_temporal_innodb.test +++ b/mysql-test/main/type_temporal_innodb.test @@ -66,3 +66,22 @@ CREATE TABLE t1 (d DATE) ENGINE=InnoDB; INSERT INTO t1 VALUES ('2012-12-21'); SELECT * FROM t1 WHERE LEAST( UTC_TIME(), d ); DROP TABLE t1; + +--echo # +--echo # MDEV-17969 Assertion `name' failed in THD::push_warning_truncated_value_for_field +--echo # + +CREATE TABLE t1 (c1 DATE , c2 TIMESTAMP) ENGINE=InnoDB; +INSERT INTO t1 VALUES ('2006-07-17','0000-00-00 00:00:00'); +CREATE TABLE t2 (pk INT, a1 TIME) Engine=InnoDB; +INSERT INTO t2 VALUES (6,'00:00:00'); +SET SESSION sql_mode= 'strict_all_tables,no_zero_date'; +--error ER_TRUNCATED_WRONG_VALUE +CREATE TABLE tbl SELECT * FROM t1 WHERE t1.c1 = (SELECT c2 FROM t2 WHERE pk = 6); +# ^^^ there is no column c2 in table t2 +DROP TABLE t1,t2; +SET sql_mode=DEFAULT; + +--echo # +--echo # End of 10.3 tests +--echo # diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 88a1cdedeac..9f5a406507c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -17460,6 +17460,7 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, bool using_unique_constraint= false; bool use_packed_rows= false; bool not_all_columns= !(select_options & TMP_TABLE_ALL_COLUMNS); + bool save_abort_on_warning; char *tmpname,path[FN_REFLEN]; uchar *pos, *group_buff, *bitmaps; uchar *null_flags; @@ -17932,6 +17933,11 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, } null_count= (blob_count == 0) ? 1 : 0; hidden_field_count=param->hidden_field_count; + + /* Protect against warnings in field_conv() in the next loop*/ + save_abort_on_warning= thd->abort_on_warning; + thd->abort_on_warning= 0; + for (i=0,reg_field=table->field; i < field_count; i++,reg_field++,recinfo++) { Field *field= *reg_field; @@ -18018,6 +18024,7 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, bzero(pos, table->s->reclength - (pos - table->record[0])); MEM_CHECK_DEFINED(table->record[0], table->s->reclength); + thd->abort_on_warning= save_abort_on_warning; param->copy_field_end=copy; param->recinfo= recinfo; // Pointer to after last field store_record(table,s->default_values); // Make empty default record From e6595a06d63fb40f81f6ec0313a37931cdf0e9c5 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 3 Jul 2020 01:18:51 +0300 Subject: [PATCH 08/26] Don't give errors for default value copy in create_tmp_table --- sql/sql_select.cc | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fed76fbd0fd..326c27eff0a 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -18151,6 +18151,7 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, bool using_unique_constraint= false; bool use_packed_rows= false; bool not_all_columns= !(select_options & TMP_TABLE_ALL_COLUMNS); + bool save_abort_on_warning; char *tmpname,path[FN_REFLEN]; uchar *pos, *group_buff, *bitmaps; uchar *null_flags; @@ -18618,6 +18619,11 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, } null_count= (blob_count == 0) ? 1 : 0; hidden_field_count=param->hidden_field_count; + + /* Protect against warnings in field_conv() in the next loop*/ + save_abort_on_warning= thd->abort_on_warning; + thd->abort_on_warning= 0; + for (i=0,reg_field=table->field; i < field_count; i++,reg_field++,recinfo++) { Field *field= *reg_field; @@ -18704,6 +18710,7 @@ create_tmp_table(THD *thd, TMP_TABLE_PARAM *param, List &fields, bzero(pos, table->s->reclength - (pos - table->record[0])); MEM_CHECK_DEFINED(table->record[0], table->s->reclength); + thd->abort_on_warning= save_abort_on_warning; param->copy_field_end=copy; param->recinfo= recinfo; // Pointer to after last field store_record(table,s->default_values); // Make empty default record From 484931325e7bcecddc6daf1a55c008ddd67497e4 Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 3 Jul 2020 18:37:33 +0300 Subject: [PATCH 09/26] Fix for MSAN from Marko related to buf_pool_resize --- storage/innobase/buf/buf0buf.cc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index e2425ccb6cc..c9cff9b3295 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -1622,6 +1622,8 @@ buf_chunk_init( return(NULL); } + MEM_MAKE_ADDRESSABLE(chunk->mem, chunk->mem_size()); + #ifdef HAVE_LIBNUMA if (srv_numa_interleave) { struct bitmask *numa_mems_allowed = numa_get_mems_allowed(); @@ -2908,6 +2910,9 @@ withdraw_retry: while (chunk < echunk) { buf_block_t* block = chunk->blocks; + MEM_MAKE_ADDRESSABLE(chunk->mem, + chunk->mem_size()); + for (ulint j = chunk->size; j--; block++) { mutex_free(&block->mutex); From 70684afef2ce6d797f78db192c4472260cd22660 Mon Sep 17 00:00:00 2001 From: Qi Wu Date: Fri, 3 Jul 2020 20:34:46 +0300 Subject: [PATCH 10/26] Atomic write support for ScaleFlux NVMe SSD's --- mysys/my_atomic_writes.c | 114 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/mysys/my_atomic_writes.c b/mysys/my_atomic_writes.c index b383af11ba8..34207a6fd07 100644 --- a/mysys/my_atomic_writes.c +++ b/mysys/my_atomic_writes.c @@ -19,7 +19,8 @@ my_bool my_may_have_atomic_write= IF_WIN(1,0); #ifdef __linux__ -my_bool has_shannon_atomic_write= 0, has_fusion_io_atomic_write= 0; +my_bool has_shannon_atomic_write= 0, has_fusion_io_atomic_write= 0, + has_sfx_atomic_write= 0; #include @@ -254,6 +255,109 @@ static my_bool shannon_has_atomic_write(File file, int page_size) } +/*********************************************************************** + ScaleFlux +************************************************************************/ + +#define SFX_GET_ATOMIC_SIZE _IO('N', 0x244) +#define SFX_MAX_DEVICES 32 +#define SFX_NO_ATOMIC_SIZE_YET -2 + +struct sfx_dev +{ + char dev_name[32]; + dev_t st_dev; + int atomic_size; +}; + +static struct sfx_dev sfx_devices[SFX_MAX_DEVICES + 1]; + +/** + Check if the system has a ScaleFlux card + If card exists, record device numbers to allow us to later check if + a given file is on this device. + @return TRUE Card exists +*/ + +static my_bool test_if_sfx_card_exists() +{ + uint sfx_found_devices = 0; + uint dev_num; + + for (dev_num = 0; dev_num < SFX_MAX_DEVICES; dev_num++) + { + struct stat stat_buff; + + sprintf(sfx_devices[sfx_found_devices].dev_name, "/dev/sfdv%dn1", + dev_num); + if (lstat(sfx_devices[sfx_found_devices].dev_name, + &stat_buff) < 0) + break; + + sfx_devices[sfx_found_devices].st_dev= stat_buff.st_rdev; + /* + The atomic size will be checked on first access. This is needed + as a normal user can't open the /dev/sfdvXn1 file + */ + sfx_devices[sfx_found_devices].atomic_size = SFX_NO_ATOMIC_SIZE_YET; + if (++sfx_found_devices == SFX_MAX_DEVICES) + goto end; + } +end: + sfx_devices[sfx_found_devices].st_dev= 0; + return sfx_found_devices > 0; +} + +static my_bool sfx_dev_has_atomic_write(struct sfx_dev *dev, + int page_size) +{ + if (dev->atomic_size == SFX_NO_ATOMIC_SIZE_YET) + { + int fd= open(dev->dev_name, 0); + if (fd < 0) + { + perror("open() failed!"); + dev->atomic_size= 0; /* Don't try again */ + return FALSE; + } + + dev->atomic_size= ioctl(fd, SFX_GET_ATOMIC_SIZE); + close(fd); + } + + return (page_size <= dev->atomic_size); +} + + +/** + Check if a file is on a ScaleFlux device and that it supports atomic_write + @param[in] file OS file handle + @param[in] page_size page size + @return TRUE Atomic write supported + + @notes + This is called only at first open of a file. In this case it's doesn't + matter so much that we loop over all cards. + We update the atomic size on first access. +*/ + +static my_bool sfx_has_atomic_write(File file, int page_size) +{ + struct sfx_dev *dev; + struct stat stat_buff; + + if (fstat(file, &stat_buff) < 0) + { + return 0; + } + + for (dev = sfx_devices; dev->st_dev; dev++) + { + if (stat_buff.st_dev == dev->st_dev) + return sfx_dev_has_atomic_write(dev, page_size); + } + return 0; +} /*********************************************************************** Generic atomic write code ************************************************************************/ @@ -266,7 +370,8 @@ static my_bool shannon_has_atomic_write(File file, int page_size) void my_init_atomic_write(void) { if ((has_shannon_atomic_write= test_if_shannon_card_exists()) || - (has_fusion_io_atomic_write= test_if_fusion_io_card_exists())) + (has_fusion_io_atomic_write= test_if_fusion_io_card_exists()) || + (has_sfx_atomic_write= test_if_sfx_card_exists())) my_may_have_atomic_write= 1; #ifdef TEST_SHANNON printf("%s(): has_shannon_atomic_write=%d, my_may_have_atomic_write=%d\n", @@ -294,6 +399,7 @@ my_bool my_test_if_atomic_write(File handle, int page_size) #endif if (!my_may_have_atomic_write) return 0; + if (has_shannon_atomic_write && shannon_has_atomic_write(handle, page_size)) return 1; @@ -302,6 +408,10 @@ my_bool my_test_if_atomic_write(File handle, int page_size) fusion_io_has_atomic_write(handle, page_size)) return 1; + if (has_sfx_atomic_write && + sfx_has_atomic_write(handle, page_size)) + return 1; + return 0; } From 7d75e4326170ab3b22de0365b4b093ea6bc24285 Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 4 Jul 2020 02:19:02 +0300 Subject: [PATCH 11/26] Disable rpl_parallel2 temporarly until we have a proper fix for it in 10.5 --- mysql-test/suite/rpl/disabled.def | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql-test/suite/rpl/disabled.def b/mysql-test/suite/rpl/disabled.def index 9e52c277726..640c4b56cd0 100644 --- a/mysql-test/suite/rpl/disabled.def +++ b/mysql-test/suite/rpl/disabled.def @@ -19,3 +19,4 @@ rpl_semi_sync_after_sync : fails after MDEV-16172 rpl_slave_grp_exec: MDEV-10514 rpl_auto_increment_update_failure : disabled for now rpl_current_user : waits for MDEV-22374 fix +rpl_parallel2 : waits for MDEV-23089 From d2b852b4ca326f2a47ba1f51cd27e04a9954aa0d Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sun, 14 Jun 2020 11:46:37 +0200 Subject: [PATCH 12/26] cleanup, less #ifdef's --- sql/sql_table.cc | 5 +---- sql/wsrep_mysqld.h | 1 + storage/example/ha_example.cc | 4 ++-- storage/heap/ha_heap.cc | 2 -- storage/heap/heapdef.h | 2 ++ storage/myisam/ha_myisam.cc | 2 -- storage/myisam/myisamdef.h | 2 ++ 7 files changed, 8 insertions(+), 10 deletions(-) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index f091fd29303..d6277e838be 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2443,14 +2443,11 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, } else { -#ifdef WITH_WSREP - if (WSREP(thd) && - !wsrep_should_replicate_ddl(thd, table_type->db_type)) + if (WSREP(thd) && !wsrep_should_replicate_ddl(thd, table_type->db_type)) { error= 1; goto err; } -#endif /* It could happen that table's share in the table definition cache diff --git a/sql/wsrep_mysqld.h b/sql/wsrep_mysqld.h index 3347a94cdd6..51c23085717 100644 --- a/sql/wsrep_mysqld.h +++ b/sql/wsrep_mysqld.h @@ -627,6 +627,7 @@ enum wsrep::streaming_context::fragment_unit wsrep_fragment_unit(ulong unit); #define wsrep_thr_deinit() do {} while(0) #define wsrep_init_globals() do {} while(0) #define wsrep_create_appliers(X) do {} while(0) +#define wsrep_should_replicate_ddl(X,Y) (1) #endif /* WITH_WSREP */ diff --git a/storage/example/ha_example.cc b/storage/example/ha_example.cc index 3bfe1e5e27b..cf259a7e80d 100644 --- a/storage/example/ha_example.cc +++ b/storage/example/ha_example.cc @@ -215,6 +215,8 @@ static void init_example_psi_keys() count= array_elements(all_example_mutexes); mysql_mutex_register(category, all_example_mutexes, count); } +#else +static void init_example_psi_keys() { } #endif @@ -252,9 +254,7 @@ static int example_init_func(void *p) { DBUG_ENTER("example_init_func"); -#ifdef HAVE_PSI_INTERFACE init_example_psi_keys(); -#endif example_hton= (handlerton *)p; example_hton->create= example_create_handler; diff --git a/storage/heap/ha_heap.cc b/storage/heap/ha_heap.cc index ee2c9f0d916..f195264ce7f 100644 --- a/storage/heap/ha_heap.cc +++ b/storage/heap/ha_heap.cc @@ -44,9 +44,7 @@ int heap_init(void *p) { handlerton *heap_hton; -#ifdef HAVE_PSI_INTERFACE init_heap_psi_keys(); -#endif heap_hton= (handlerton *)p; heap_hton->db_type= DB_TYPE_HEAP; diff --git a/storage/heap/heapdef.h b/storage/heap/heapdef.h index 6136a90f0aa..ffd5382b6f7 100644 --- a/storage/heap/heapdef.h +++ b/storage/heap/heapdef.h @@ -110,6 +110,8 @@ extern PSI_memory_key hp_key_memory_HP_KEYDEF; #ifdef HAVE_PSI_INTERFACE void init_heap_psi_keys(); +#else +#define init_heap_psi_keys() do { } while(0) #endif /* HAVE_PSI_INTERFACE */ C_MODE_END diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index 448aab0a092..e7ca765351b 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -2516,9 +2516,7 @@ static int myisam_init(void *p) { handlerton *hton; -#ifdef HAVE_PSI_INTERFACE init_myisam_psi_keys(); -#endif /* Set global variables based on startup options */ if (myisam_recover_options && myisam_recover_options != HA_RECOVER_OFF) diff --git a/storage/myisam/myisamdef.h b/storage/myisam/myisamdef.h index 640a04fb3df..f48d1078000 100644 --- a/storage/myisam/myisamdef.h +++ b/storage/myisam/myisamdef.h @@ -774,6 +774,8 @@ extern PSI_file_key mi_key_file_datatmp, mi_key_file_dfile, mi_key_file_kfile, extern PSI_thread_key mi_key_thread_find_all_keys; void init_myisam_psi_keys(); +#else +#define init_myisam_psi_keys() do { } while(0) #endif /* HAVE_PSI_INTERFACE */ extern PSI_memory_key mi_key_memory_MYISAM_SHARE; From f17f7a43ba7da44608618f9e612f9808d363da23 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 16 Jun 2020 19:37:27 +0200 Subject: [PATCH 13/26] test dropping of a MEMORY table --- mysql-test/suite/heap/drop.result | 3 +++ mysql-test/suite/heap/drop.test | 11 +++++++++++ 2 files changed, 14 insertions(+) create mode 100644 mysql-test/suite/heap/drop.result create mode 100644 mysql-test/suite/heap/drop.test diff --git a/mysql-test/suite/heap/drop.result b/mysql-test/suite/heap/drop.result new file mode 100644 index 00000000000..cbc784fd59b --- /dev/null +++ b/mysql-test/suite/heap/drop.result @@ -0,0 +1,3 @@ +create table t1 (a int) engine=memory; +drop table t1; +drop table t2; diff --git a/mysql-test/suite/heap/drop.test b/mysql-test/suite/heap/drop.test new file mode 100644 index 00000000000..ef61d4400f1 --- /dev/null +++ b/mysql-test/suite/heap/drop.test @@ -0,0 +1,11 @@ +create table t1 (a int) engine=memory; +let $DATADIR= `select @@datadir`; +copy_file $DATADIR/test/t1.frm $DATADIR/test/t2.frm; +# +# drop a newly created MEMORY table +# +drop table t1; +# +# drop a MEMORY table after a server restart (frm only, nothing in memory) +# +drop table t2; From c55c292832e2776d37e06c43174ac006e00143a2 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 13 Jun 2020 21:23:19 +0200 Subject: [PATCH 14/26] introduce hton->drop_table() method first step in moving drop table out of the handler. todo: other methods that don't need an open table for now hton->drop_table is optional, for backward compatibility reasons --- .../suite/parts/r/partition_open.result | 2 +- sql/ha_partition.cc | 22 +++--- sql/handler.cc | 70 +++++++++---------- sql/handler.h | 7 +- sql/sql_base.cc | 22 +++--- sql/sql_select.cc | 21 +++--- sql/sql_table.cc | 2 +- sql/temporary_tables.cc | 6 +- storage/mroonga/ha_mroonga.cpp | 13 +--- 9 files changed, 70 insertions(+), 95 deletions(-) diff --git a/mysql-test/suite/parts/r/partition_open.result b/mysql-test/suite/parts/r/partition_open.result index 98600d98ce3..a8ffac1109e 100644 --- a/mysql-test/suite/parts/r/partition_open.result +++ b/mysql-test/suite/parts/r/partition_open.result @@ -5,4 +5,4 @@ select * from t1 partition (p0); ERROR HY000: Can't find file: './test/t1.MYI' (errno: 2 "No such file or directory") drop table t1; Warnings: -Warning 1017 Can't find file: './test/t1.MYI' (errno: 2 "No such file or directory") +Warning 1017 Can't find file: './test/t1.par' (errno: 2 "No such file or directory") diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index dfbc8d52dcd..f5eab028755 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -811,7 +811,7 @@ create_error: { if (!create_partition_name(name_buff, sizeof(name_buff), path, name_buffer_ptr, NORMAL_PART_NAME, FALSE)) - (void) (*file)->ha_delete_table((const char*) name_buff); + (void) (*file)->delete_table((const char*) name_buff); name_buffer_ptr= strend(name_buffer_ptr) + 1; } handler::delete_table(name); @@ -880,7 +880,7 @@ int ha_partition::drop_partitions(const char *path) error= ret_error; file= m_file[part]; DBUG_PRINT("info", ("Drop subpartition %s", part_name_buff)); - if (unlikely((ret_error= file->ha_delete_table(part_name_buff)))) + if (unlikely((ret_error= file->delete_table(part_name_buff)))) error= ret_error; if (unlikely(deactivate_ddl_log_entry(sub_elem->log_entry-> entry_pos))) @@ -897,7 +897,7 @@ int ha_partition::drop_partitions(const char *path) { file= m_file[i]; DBUG_PRINT("info", ("Drop partition %s", part_name_buff)); - if (unlikely((ret_error= file->ha_delete_table(part_name_buff)))) + if (unlikely((ret_error= file->delete_table(part_name_buff)))) error= ret_error; if (unlikely(deactivate_ddl_log_entry(part_elem->log_entry-> entry_pos))) @@ -989,7 +989,7 @@ int ha_partition::rename_partitions(const char *path) NORMAL_PART_NAME)))) error= ret_error; DBUG_PRINT("info", ("Delete subpartition %s", norm_name_buff)); - if (unlikely((ret_error= file->ha_delete_table(norm_name_buff)))) + if (unlikely((ret_error= file->delete_table(norm_name_buff)))) error= ret_error; else if (unlikely(deactivate_ddl_log_entry(sub_elem->log_entry-> entry_pos))) @@ -1010,7 +1010,7 @@ int ha_partition::rename_partitions(const char *path) else { DBUG_PRINT("info", ("Delete partition %s", norm_name_buff)); - if (unlikely((ret_error= file->ha_delete_table(norm_name_buff)))) + if (unlikely((ret_error= file->delete_table(norm_name_buff)))) error= ret_error; else if (unlikely(deactivate_ddl_log_entry(part_elem->log_entry-> entry_pos))) @@ -1071,7 +1071,7 @@ int ha_partition::rename_partitions(const char *path) { file= m_reorged_file[part_count++]; DBUG_PRINT("info", ("Delete subpartition %s", norm_name_buff)); - if (unlikely((ret_error= file->ha_delete_table(norm_name_buff)))) + if (unlikely((ret_error= file->delete_table(norm_name_buff)))) error= ret_error; else if (unlikely(deactivate_ddl_log_entry(sub_elem->log_entry-> entry_pos))) @@ -1118,7 +1118,7 @@ int ha_partition::rename_partitions(const char *path) { file= m_reorged_file[part_count++]; DBUG_PRINT("info", ("Delete partition %s", norm_name_buff)); - if (unlikely((ret_error= file->ha_delete_table(norm_name_buff)))) + if (unlikely((ret_error= file->delete_table(norm_name_buff)))) error= ret_error; else if (unlikely(deactivate_ddl_log_entry(part_elem->log_entry-> entry_pos))) @@ -1670,7 +1670,7 @@ int ha_partition::prepare_new_partition(TABLE *tbl, error_external_lock: (void) file->ha_close(); error_open: - (void) file->ha_delete_table(part_name); + (void) file->delete_table(part_name); error_create: DBUG_RETURN(error); } @@ -1716,7 +1716,7 @@ void ha_partition::cleanup_new_partition(uint part_count) (*file)->ha_external_unlock(thd); (*file)->ha_close(); - /* Leave the (*file)->ha_delete_table(part_name) to the ddl-log */ + /* Leave the (*file)->delete_table(part_name) to the ddl-log */ file++; part_count--; @@ -2309,7 +2309,7 @@ void ha_partition::update_create_info(HA_CREATE_INFO *create_info) @param table_arg TABLE object @param share New share to use - @note Is used in error handling in ha_delete_table. + @note Is used in error handling in delete_table. All handlers should exist (lock_partitions should not be used) */ @@ -2450,7 +2450,7 @@ uint ha_partition::del_ren_table(const char *from, const char *to) } else // delete branch { - error= (*file)->ha_delete_table(from_buff); + error= (*file)->delete_table(from_buff); } name_buffer_ptr= strend(name_buffer_ptr) + 1; if (unlikely(error)) diff --git a/sql/handler.cc b/sql/handler.cc index c2fb7da384c..8642c773007 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -546,6 +546,26 @@ static void update_discovery_counters(handlerton *hton, int val) engines_with_discover+= val; } +int ha_drop_table(THD *thd, handlerton *hton, const char *path) +{ + if (ha_check_if_updates_are_ignored(thd, hton, "DROP")) + return 0; // Simulate dropped + return hton->drop_table(hton, path); +} + +static int hton_drop_table(handlerton *hton, const char *path) +{ + char tmp_path[FN_REFLEN]; + handler *file= get_new_handler(nullptr, current_thd->mem_root, hton); + if (!file) + return ENOMEM; + path= get_canonical_filename(file, path, tmp_path); + int error= file->delete_table(path); + delete file; + return error; +} + + int ha_finalize_handlerton(st_plugin_int *plugin) { handlerton *hton= (handlerton *)plugin->data; @@ -616,6 +636,7 @@ int ha_initialize_handlerton(st_plugin_int *plugin) hton->tablefile_extensions= no_exts; hton->discover_table_names= hton_ext_based_table_discovery; + hton->drop_table= hton_drop_table; hton->slot= HA_SLOT_UNDEF; /* Historical Requirement */ @@ -2724,29 +2745,19 @@ const char *get_canonical_filename(handler *file, const char *path, The .frm file should be deleted by the caller only if we return <= 0. */ -int ha_delete_table(THD *thd, handlerton *table_type, const char *path, +int ha_delete_table(THD *thd, handlerton *hton, const char *path, const LEX_CSTRING *db, const LEX_CSTRING *alias, bool generate_warning) { - handler *file; - char tmp_path[FN_REFLEN]; int error; - TABLE dummy_table; - TABLE_SHARE dummy_share; bool is_error= thd->is_error(); DBUG_ENTER("ha_delete_table"); - /* table_type is NULL in ALTER TABLE when renaming only .frm files */ - if (table_type == NULL || table_type == view_pseudo_hton || - ! (file=get_new_handler((TABLE_SHARE*)0, thd->mem_root, table_type))) - DBUG_RETURN(-1); + /* hton is NULL in ALTER TABLE when renaming only .frm files */ + if (hton == NULL || hton == view_pseudo_hton) + DBUG_RETURN(0); - bzero((char*) &dummy_table, sizeof(dummy_table)); - bzero((char*) &dummy_share, sizeof(dummy_share)); - dummy_table.s= &dummy_share; - - path= get_canonical_filename(file, path, tmp_path); - if (unlikely((error= file->ha_delete_table(path)))) + if (unlikely((error= hton->drop_table(hton, path)))) { /* It's not an error if the table doesn't exist in the engine. @@ -2757,15 +2768,21 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, if ((!intercept || generate_warning) && ! thd->is_error()) { - /* Fill up strucutures that print_error may need */ + TABLE dummy_table; + TABLE_SHARE dummy_share; + handler *file= get_new_handler(nullptr, thd->mem_root, hton); + bzero((char*) &dummy_table, sizeof(dummy_table)); + bzero((char*) &dummy_share, sizeof(dummy_share)); dummy_share.path.str= (char*) path; dummy_share.path.length= strlen(path); dummy_share.normalized_path= dummy_share.path; dummy_share.db= *db; dummy_share.table_name= *alias; + dummy_table.s= &dummy_share; dummy_table.alias.set(alias->str, alias->length, table_alias_charset); file->change_table_ptr(&dummy_table, &dummy_share); file->print_error(error, MYF(intercept ? ME_WARNING : 0)); + delete file; } if (intercept) { @@ -2775,7 +2792,6 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, error= -1; } } - delete file; DBUG_RETURN(error); } @@ -4586,8 +4602,8 @@ void handler::mark_trx_read_write_internal() if (ha_info->is_started()) { /* - table_share can be NULL in ha_delete_table(). See implementation - of standalone function ha_delete_table() in sql_base.cc. + table_share can be NULL, for example, in ha_delete_table() or + ha_rename_table(). */ if (table_share == NULL || table_share->tmp_table == NO_TMP_TABLE) ha_info->set_trx_read_write(); @@ -4947,22 +4963,6 @@ handler::ha_rename_table(const char *from, const char *to) } -/** - Delete table: public interface. - - @sa handler::delete_table() -*/ - -int -handler::ha_delete_table(const char *name) -{ - if (ha_check_if_updates_are_ignored(ha_thd(), ht, "DROP")) - return 0; // Simulate dropped - mark_trx_read_write(); - return delete_table(name); -} - - /** Drop table in the engine: public interface. diff --git a/sql/handler.h b/sql/handler.h index 61223c4e715..4d9f92d3126 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1484,6 +1484,7 @@ struct handlerton void (*close_cursor_read_view)(handlerton *hton, THD *thd, void *read_view); handler *(*create)(handlerton *hton, TABLE_SHARE *table, MEM_ROOT *mem_root); void (*drop_database)(handlerton *hton, char* path); + int (*drop_table)(handlerton *hton, const char* path); int (*panic)(handlerton *hton, enum ha_panic_function flag); int (*start_consistent_snapshot)(handlerton *hton, THD *thd); bool (*flush_logs)(handlerton *hton); @@ -3453,7 +3454,6 @@ public: int ha_enable_indexes(uint mode); int ha_discard_or_import_tablespace(my_bool discard); int ha_rename_table(const char *from, const char *to); - int ha_delete_table(const char *name); void ha_drop_table(const char *name); int ha_create(const char *name, TABLE *form, HA_CREATE_INFO *info); @@ -4673,13 +4673,14 @@ protected: provide useful functionality. */ virtual int rename_table(const char *from, const char *to); + + +public: /** Delete a table in the engine. Called for base as well as temporary tables. */ virtual int delete_table(const char *name); - -public: bool check_table_binlog_row_based(); bool prepare_for_row_logging(); int prepare_for_insert(bool do_create); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 88a28c470c0..6d53a8ee6e3 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8794,7 +8794,7 @@ fill_record_n_invoke_before_triggers(THD *thd, TABLE *table, Field **ptr, my_bool mysql_rm_tmp_tables(void) { uint i, idx; - char filePath[FN_REFLEN], *tmpdir, filePathCopy[FN_REFLEN]; + char path[FN_REFLEN], *tmpdir, path_copy[FN_REFLEN]; MY_DIR *dirp; FILEINFO *file; TABLE_SHARE share; @@ -8823,23 +8823,17 @@ my_bool mysql_rm_tmp_tables(void) { char *ext= fn_ext(file->name); size_t ext_len= strlen(ext); - size_t filePath_len= my_snprintf(filePath, sizeof(filePath), + size_t path_len= my_snprintf(path, sizeof(path), "%s%c%s", tmpdir, FN_LIBCHAR, file->name); if (!strcmp(reg_ext, ext)) { - handler *handler_file= 0; /* We should cut file extention before deleting of table */ - memcpy(filePathCopy, filePath, filePath_len - ext_len); - filePathCopy[filePath_len - ext_len]= 0; - init_tmp_table_share(thd, &share, "", 0, "", filePathCopy); - if (!open_table_def(thd, &share) && - ((handler_file= get_new_handler(&share, thd->mem_root, - share.db_type())))) - { - handler_file->ha_delete_table(filePathCopy); - delete handler_file; - } + memcpy(path_copy, path, path_len - ext_len); + path_copy[path_len - ext_len]= 0; + init_tmp_table_share(thd, &share, "", 0, "", path_copy); + if (!open_table_def(thd, &share)) + share.db_type()->drop_table(share.db_type(), path_copy); free_table_share(&share); } /* @@ -8847,7 +8841,7 @@ my_bool mysql_rm_tmp_tables(void) So we hide error messages which happnes during deleting of these files(MYF(0)). */ - (void) mysql_file_delete(key_file_misc, filePath, MYF(0)); + (void) mysql_file_delete(key_file_misc, path, MYF(0)); } } my_dirend(dirp); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 9cc4a9c4626..fc9fea42a99 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -19802,8 +19802,7 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, if (is_duplicate) *is_duplicate= FALSE; - if (table->s->db_type() != heap_hton || - error != HA_ERR_RECORD_FILE_FULL) + if (table->s->db_type() != heap_hton || error != HA_ERR_RECORD_FILE_FULL) { /* We don't want this error to be converted to a warning, e.g. in case of @@ -19817,7 +19816,7 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, new_table.s= &share; new_table.s->db_plugin= ha_lock_engine(thd, TMP_ENGINE_HTON); if (unlikely(!(new_table.file= get_new_handler(&share, &new_table.mem_root, - new_table.s->db_type())))) + TMP_ENGINE_HTON)))) DBUG_RETURN(1); // End of memory if (unlikely(new_table.file->set_ha_share_ref(&share.ha_share))) @@ -19908,7 +19907,7 @@ err_killed: (void) table->file->ha_rnd_end(); (void) new_table.file->ha_close(); err1: - new_table.file->ha_delete_table(new_table.s->path.str); + TMP_ENGINE_HTON->drop_table(TMP_ENGINE_HTON, new_table.s->path.str); err2: delete new_table.file; thd_proc_info(thd, save_proc_info); @@ -19931,16 +19930,12 @@ free_tmp_table(THD *thd, TABLE *entry) if (entry->file && entry->is_created()) { + DBUG_ASSERT(entry->db_stat); entry->file->ha_index_or_rnd_end(); - if (entry->db_stat) - { - entry->file->info(HA_STATUS_VARIABLE); - thd->tmp_tables_size+= (entry->file->stats.data_file_length + - entry->file->stats.index_file_length); - entry->file->ha_drop_table(entry->s->path.str); - } - else - entry->file->ha_delete_table(entry->s->path.str); + entry->file->info(HA_STATUS_VARIABLE); + thd->tmp_tables_size+= (entry->file->stats.data_file_length + + entry->file->stats.index_file_length); + entry->file->ha_drop_table(entry->s->path.str); delete entry->file; } diff --git a/sql/sql_table.cc b/sql/sql_table.cc index d6277e838be..de1ebb4f191 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1168,7 +1168,7 @@ static int execute_ddl_log_action(THD *thd, DDL_LOG_ENTRY *ddl_log_entry) } else { - if (unlikely((error= file->ha_delete_table(ddl_log_entry->name)))) + if (unlikely((error= hton->drop_table(hton, ddl_log_entry->name)))) { if (!non_existing_table_error(error)) break; diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc index a8d8113945b..6a507b99ef6 100644 --- a/sql/temporary_tables.cc +++ b/sql/temporary_tables.cc @@ -694,23 +694,19 @@ bool THD::rm_temporary_table(handlerton *base, const char *path) DBUG_ENTER("THD::rm_temporary_table"); bool error= false; - handler *file; char frm_path[FN_REFLEN + 1]; strxnmov(frm_path, sizeof(frm_path) - 1, path, reg_ext, NullS); if (mysql_file_delete(key_file_frm, frm_path, MYF(MY_WME | MY_IGNORE_ENOENT))) error= true; - - file= get_new_handler((TABLE_SHARE*) 0, current_thd->mem_root, base); - if (file && file->ha_delete_table(path)) + if (base->drop_table(base, path)) { error= true; sql_print_warning("Could not remove temporary table: '%s', error: %d", path, my_errno); } - delete file; DBUG_RETURN(error); } diff --git a/storage/mroonga/ha_mroonga.cpp b/storage/mroonga/ha_mroonga.cpp index 6f302e89213..4747495fd22 100644 --- a/storage/mroonga/ha_mroonga.cpp +++ b/storage/mroonga/ha_mroonga.cpp @@ -5031,19 +5031,8 @@ int ha_mroonga::wrapper_delete_table(const char *name, handlerton *wrap_handlerton, const char *table_name) { - int error = 0; MRN_DBUG_ENTER_METHOD(); - - handler *hnd = get_new_handler(NULL, current_thd->mem_root, wrap_handlerton); - if (!hnd) - { - DBUG_RETURN(HA_ERR_OUT_OF_MEM); - } - - error = hnd->ha_delete_table(name); - delete hnd; - - DBUG_RETURN(error); + DBUG_RETURN(wrap_handlerton->drop_table(wrap_handlerton, name)); } int ha_mroonga::generic_delete_table(const char *name, const char *table_name) From b014720b6c05c8f8c08ffad263aff3273ff3d253 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sun, 14 Jun 2020 11:48:50 +0200 Subject: [PATCH 15/26] optimization: use hton->drop_table in few simple cases --- sql/log.cc | 1 + storage/blackhole/ha_blackhole.cc | 1 + storage/example/ha_example.cc | 1 + storage/federated/ha_federated.cc | 1 + storage/federatedx/ha_federatedx.cc | 1 + storage/heap/ha_heap.cc | 12 +++++++++--- storage/myisam/ha_myisam.cc | 6 ++++++ storage/oqgraph/ha_oqgraph.cc | 1 + storage/perfschema/ha_perfschema.cc | 1 + storage/sequence/sequence.cc | 1 + storage/sphinx/ha_sphinx.cc | 1 + 11 files changed, 24 insertions(+), 3 deletions(-) diff --git a/sql/log.cc b/sql/log.cc index abf22e7897a..b6778f34768 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -1692,6 +1692,7 @@ int binlog_init(void *p) binlog_savepoint_rollback_can_release_mdl; binlog_hton->commit= binlog_commit; binlog_hton->rollback= binlog_rollback; + binlog_hton->drop_table= [](handlerton *, const char*) { return 0; }; if (WSREP_ON || opt_bin_log) { binlog_hton->prepare= binlog_prepare; diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc index 98589f1d043..82ac9123d3b 100644 --- a/storage/blackhole/ha_blackhole.cc +++ b/storage/blackhole/ha_blackhole.cc @@ -399,6 +399,7 @@ static int blackhole_init(void *p) blackhole_hton= (handlerton *)p; blackhole_hton->db_type= DB_TYPE_BLACKHOLE_DB; blackhole_hton->create= blackhole_create_handler; + blackhole_hton->drop_table= [](handlerton *, const char*) { return 0; }; blackhole_hton->flags= HTON_CAN_RECREATE | HTON_AUTOMATIC_DELETE_TABLE; mysql_mutex_init(bh_key_mutex_blackhole, diff --git a/storage/example/ha_example.cc b/storage/example/ha_example.cc index cf259a7e80d..f6760c06f2d 100644 --- a/storage/example/ha_example.cc +++ b/storage/example/ha_example.cc @@ -262,6 +262,7 @@ static int example_init_func(void *p) example_hton->table_options= example_table_option_list; example_hton->field_options= example_field_option_list; example_hton->tablefile_extensions= ha_example_exts; + example_hton->drop_table= [](handlerton *, const char*) { return 0; }; DBUG_RETURN(0); } diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index b62b74d12bb..766f9a2b790 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -484,6 +484,7 @@ int federated_db_init(void *p) federated_hton->commit= federated_commit; federated_hton->rollback= federated_rollback; federated_hton->create= federated_create_handler; + federated_hton->drop_table= [](handlerton *, const char*) { return 0; }; federated_hton->flags= (HTON_ALTER_NOT_SUPPORTED | HTON_NO_PARTITION | HTON_AUTOMATIC_DELETE_TABLE); diff --git a/storage/federatedx/ha_federatedx.cc b/storage/federatedx/ha_federatedx.cc index 2370473236e..45029fdf17f 100644 --- a/storage/federatedx/ha_federatedx.cc +++ b/storage/federatedx/ha_federatedx.cc @@ -438,6 +438,7 @@ int federatedx_db_init(void *p) federatedx_hton->rollback= ha_federatedx::rollback; federatedx_hton->discover_table_structure= ha_federatedx::discover_assisted; federatedx_hton->create= federatedx_create_handler; + federatedx_hton->drop_table= [](handlerton *, const char*) { return 0; }; federatedx_hton->flags= (HTON_ALTER_NOT_SUPPORTED | HTON_AUTOMATIC_DELETE_TABLE); federatedx_hton->create_derived= create_federatedx_derived_handler; diff --git a/storage/heap/ha_heap.cc b/storage/heap/ha_heap.cc index f195264ce7f..0e61bff1d8d 100644 --- a/storage/heap/ha_heap.cc +++ b/storage/heap/ha_heap.cc @@ -34,12 +34,18 @@ heap_prepare_hp_create_info(TABLE *table_arg, bool internal_table, HP_CREATE_INFO *hp_create_info); -int heap_panic(handlerton *hton, ha_panic_function flag) +static int heap_panic(handlerton *hton, ha_panic_function flag) { return hp_panic(flag); } +static int heap_drop_table(handlerton *hton, const char *path) +{ + int error= heap_delete_table(path); + return error == ENOENT ? 0 : error; +} + int heap_init(void *p) { handlerton *heap_hton; @@ -50,6 +56,7 @@ int heap_init(void *p) heap_hton->db_type= DB_TYPE_HEAP; heap_hton->create= heap_create_handler; heap_hton->panic= heap_panic; + heap_hton->drop_table= heap_drop_table; heap_hton->flags= HTON_CAN_RECREATE; return 0; @@ -559,8 +566,7 @@ THR_LOCK_DATA **ha_heap::store_lock(THD *thd, int ha_heap::delete_table(const char *name) { - int error= heap_delete_table(name); - return error == ENOENT ? 0 : error; + return heap_drop_table(0, name); } diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index e7ca765351b..e58ee9e31e4 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -2512,6 +2512,11 @@ int myisam_panic(handlerton *hton, ha_panic_function flag) return mi_panic(flag); } +static int myisam_drop_table(handlerton *hton, const char *path) +{ + return mi_delete_table(path); +} + static int myisam_init(void *p) { handlerton *hton; @@ -2529,6 +2534,7 @@ static int myisam_init(void *p) hton= (handlerton *)p; hton->db_type= DB_TYPE_MYISAM; hton->create= myisam_create_handler; + hton->drop_table= myisam_drop_table; hton->panic= myisam_panic; hton->flags= HTON_CAN_RECREATE | HTON_SUPPORT_LOG_TABLES; hton->tablefile_extensions= ha_myisam_exts; diff --git a/storage/oqgraph/ha_oqgraph.cc b/storage/oqgraph/ha_oqgraph.cc index 20a3732ce95..8b219b20ec6 100644 --- a/storage/oqgraph/ha_oqgraph.cc +++ b/storage/oqgraph/ha_oqgraph.cc @@ -192,6 +192,7 @@ static int oqgraph_init(void *p) hton->discover_table_structure= oqgraph_discover_table_structure; hton->close_connection = oqgraph_close_connection; + hton->drop_table= [](handlerton *, const char*) { return 0; }; oqgraph_init_done= TRUE; return 0; diff --git a/storage/perfschema/ha_perfschema.cc b/storage/perfschema/ha_perfschema.cc index c4c59d109f5..d36a7d7322a 100644 --- a/storage/perfschema/ha_perfschema.cc +++ b/storage/perfschema/ha_perfschema.cc @@ -94,6 +94,7 @@ static int pfs_init_func(void *p) pfs_hton= reinterpret_cast (p); pfs_hton->create= pfs_create_handler; + pfs_hton->drop_table= [](handlerton *, const char*) { return 0; }; pfs_hton->show_status= pfs_show_status; pfs_hton->flags= (HTON_ALTER_NOT_SUPPORTED | HTON_TEMPORARY_NOT_SUPPORTED | diff --git a/storage/sequence/sequence.cc b/storage/sequence/sequence.cc index 31522b8f3b7..ddebb1a1a8d 100644 --- a/storage/sequence/sequence.cc +++ b/storage/sequence/sequence.cc @@ -502,6 +502,7 @@ static int init(void *p) handlerton *hton= (handlerton *)p; sequence_hton= hton; hton->create= create_handler; + hton->drop_table= [](handlerton *, const char*) { return 0; }; hton->discover_table= discover_table; hton->discover_table_existence= discover_table_existence; hton->commit= hton->rollback= dummy_commit_rollback; diff --git a/storage/sphinx/ha_sphinx.cc b/storage/sphinx/ha_sphinx.cc index d60a4d229e6..39e07290853 100644 --- a/storage/sphinx/ha_sphinx.cc +++ b/storage/sphinx/ha_sphinx.cc @@ -749,6 +749,7 @@ static int sphinx_init_func ( void * p ) hton->close_connection = sphinx_close_connection; hton->show_status = sphinx_show_status; hton->panic = sphinx_panic; + hton->drop_table= [](handlerton *, const char*) { return 0; }; hton->flags = HTON_CAN_RECREATE | HTON_AUTOMATIC_DELETE_TABLE; #endif } From 2bb5981c202f85c8399485d6a57a69d8c6f627af Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 15 Jun 2020 12:13:44 +0200 Subject: [PATCH 16/26] MDEV-11412 Ensure that table is truly dropped when using DROP TABLE minor post-review fixes * remove duplicate tests * first file indicates table existance even with discovery * don't duplicate trigger dropping code --- mysql-test/main/drop_table_force.result | 13 +------- mysql-test/main/drop_table_force.test | 22 +------------- sql/handler.cc | 13 ++------ sql/sql_table.cc | 40 ++++++------------------- 4 files changed, 13 insertions(+), 75 deletions(-) diff --git a/mysql-test/main/drop_table_force.result b/mysql-test/main/drop_table_force.result index bb4ecc060b0..d3142887ade 100644 --- a/mysql-test/main/drop_table_force.result +++ b/mysql-test/main/drop_table_force.result @@ -8,7 +8,7 @@ db.opt # Test droping table without frm without super privilege create table t1(a int) engine=innodb; create user test identified by '123456'; -grant all privileges on test.t1 to 'test'@'%'identified by '123456' with grant option; +grant all privileges on test.t1 to 'test'@'%'identified by '123456'; connect con_test, localhost, test,'123456', ; connection con_test; drop table t1; @@ -18,10 +18,6 @@ connection default; disconnect con_test; drop user test; db.opt -#Test4: drop table can drop consistent table as well -create table t1(a int) engine=innodb; -drop table t1; -db.opt #Test5: drop table with triger, and with missing frm create table t1(a int)engine=innodb; create trigger t1_trg before insert on t1 for each row begin end; @@ -45,13 +41,6 @@ drop table if exists t1; Warnings: Note 1051 Unknown table 'test.t1' db.opt -#Test8: check compatibility with if exists -create table t1(a int)engine=innodb; -drop table t1; -db.opt -drop table if exists t1; -Warnings: -Note 1051 Unknown table 'test.t1' #Test9: check compatibility with restrict/cascade CREATE TABLE parent (id INT NOT NULL, PRIMARY KEY (id)) ENGINE=INNODB; CREATE TABLE child (id INT, parent_id INT, INDEX par_ind (parent_id), FOREIGN KEY (parent_id) REFERENCES parent(id) ON DELETE CASCADE) ENGINE=INNODB; diff --git a/mysql-test/main/drop_table_force.test b/mysql-test/main/drop_table_force.test index bb735309167..518b4e754c3 100644 --- a/mysql-test/main/drop_table_force.test +++ b/mysql-test/main/drop_table_force.test @@ -32,7 +32,7 @@ create table t1(a int) engine=innodb; # create test user create user test identified by '123456'; -grant all privileges on test.t1 to 'test'@'%'identified by '123456' with grant option; +grant all privileges on test.t1 to 'test'@'%'identified by '123456'; # connect as test connect (con_test, localhost, test,'123456', ); @@ -52,13 +52,6 @@ drop user test; # check files in datadir about t1 --list_files $DATADIR/test/ ---echo #Test4: drop table can drop consistent table as well -create table t1(a int) engine=innodb; -drop table t1; - -# check files in datadir about t1 ---list_files $DATADIR/test/ - --echo #Test5: drop table with triger, and with missing frm # create table t1 with triger and rm frm create table t1(a int)engine=innodb; @@ -107,19 +100,6 @@ drop table if exists t1; # check files in datadir about t1 --list_files $DATADIR/test/ ---echo #Test8: check compatibility with if exists -create table t1(a int)engine=innodb; ---remove_file $DATADIR/test/t1.frm - -# first drop will success -drop table t1; - -# check files in datadir about t1 ---list_files $DATADIR/test/ - -# second drop with if exists will success -drop table if exists t1; - --echo #Test9: check compatibility with restrict/cascade # create table with foreign key reference and rm frm CREATE TABLE parent (id INT NOT NULL, PRIMARY KEY (id)) ENGINE=INNODB; diff --git a/sql/handler.cc b/sql/handler.cc index 8642c773007..28588235477 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -4457,21 +4457,12 @@ int handler::delete_table(const char *name) // For discovery tables, it's ok if first file doesn't exists if (ht->discover_table) - { - abort_if_first_file_error= 0; saved_error= 0; - if (!bas_ext()) - { - DBUG_ASSERT(ht->flags & HTON_AUTOMATIC_DELETE_TABLE); - DBUG_RETURN(0); // Drop succeded - } - } for (const char **ext= bas_ext(); *ext ; ext++) { - int error; - if ((error= mysql_file_delete_with_symlink(key_file_misc, name, *ext, - MYF(0)))) + int err= mysql_file_delete_with_symlink(key_file_misc, name, *ext, MYF(0)); + if (err) { if (my_errno != ENOENT) { diff --git a/sql/sql_table.cc b/sql/sql_table.cc index de1ebb4f191..0d6e2c69695 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2308,7 +2308,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, for (table= tables; table; table= table->next_local) { bool is_trans= 0, frm_was_deleted= 0, temporary_table_was_dropped= 0; - bool table_creation_was_logged= 0, trigger_drop_executed= 0; + bool table_creation_was_logged= 0; bool local_non_tmp_error= 0, frm_exists= 0, wrong_drop_sequence= 0; bool table_dropped= 0; LEX_CSTRING db= table->db; @@ -2363,14 +2363,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, */ if (!dont_log_query && table_creation_was_logged) { - /* - DROP TEMPORARY succeded. For the moment when we only come - here on success (error == 0) - - If there is an error, we don't know the type of the engine - at this point. So, we keep it in the trx-cache. - */ - is_trans= error ? TRUE : is_trans; if (is_trans) trans_tmp_table_deleted= TRUE; else @@ -2416,7 +2408,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, } DEBUG_SYNC(thd, "rm_table_no_locks_before_delete_table"); - error= 0; if (drop_temporary) { /* "DROP TEMPORARY" but a temporary table was not found */ @@ -2551,16 +2542,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, table_dropped= 1; } } - if (likely(!error) || non_existing_table_error(error)) - { - trigger_drop_executed= 1; - - if (Table_triggers_list::drop_all_triggers(thd, &db, - &table->table_name, - MYF(MY_WME | - MY_IGNORE_ENOENT))) - error= error ? error : -1; - } local_non_tmp_error|= MY_TEST(error); } @@ -2568,14 +2549,9 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, If there was no .frm file and the table is not temporary, scan all engines try to drop the table from there. This is to ensure we don't have any partial table files left. - - We check for trigger_drop_executed to ensure we don't again try - to drop triggers when it failed above (after sucecssfully dropping - the table). */ if (non_existing_table_error(error) && !drop_temporary && - table_type != view_pseudo_hton && !trigger_drop_executed && - !wrong_drop_sequence) + table_type != view_pseudo_hton && !wrong_drop_sequence) { int ferror= 0; @@ -2601,16 +2577,18 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, MYF(MY_WME | MY_IGNORE_ENOENT))) ferror= my_errno; } - if (Table_triggers_list::drop_all_triggers(thd, &db, - &table->table_name, - MYF(MY_WME | - MY_IGNORE_ENOENT))) - ferror= -1; } if (!error) error= ferror; } + if (likely(!error) || non_existing_table_error(error)) + { + if (Table_triggers_list::drop_all_triggers(thd, &db, &table->table_name, + MYF(MY_WME | MY_IGNORE_ENOENT))) + error= error ? error : -1; + } + if (error) { char buff[FN_REFLEN]; From 35f566db8d847c707d0ba7f3de239bfeee9ca845 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 15 Jun 2020 14:06:08 +0200 Subject: [PATCH 17/26] cleanup: make dd_frm_type to work as documented remove redundant argument, return all possible enum values --- sql/datadict.cc | 13 ++++--------- sql/datadict.h | 6 ++---- sql/handler.cc | 33 +++++++++++++++++---------------- sql/sql_plugin.cc | 3 +-- sql/sql_show.cc | 3 +-- storage/rocksdb/rdb_datadic.cc | 3 +-- 6 files changed, 26 insertions(+), 35 deletions(-) diff --git a/sql/datadict.cc b/sql/datadict.cc index 5cfda166b2b..e09eee98565 100644 --- a/sql/datadict.cc +++ b/sql/datadict.cc @@ -49,16 +49,13 @@ static int read_string(File file, uchar**to, size_t length) If engine_name is 0, then the function will only test if the file is a view or not - @param[out] is_sequence 1 if table is a SEQUENCE, 0 otherwise - @retval TABLE_TYPE_UNKNOWN error - file can't be opened @retval TABLE_TYPE_NORMAL table @retval TABLE_TYPE_SEQUENCE sequence table @retval TABLE_TYPE_VIEW view */ -Table_type dd_frm_type(THD *thd, char *path, LEX_CSTRING *engine_name, - bool *is_sequence) +Table_type dd_frm_type(THD *thd, char *path, LEX_CSTRING *engine_name) { File file; uchar header[40]; //"TYPE=VIEW\n" it is 10 characters @@ -67,10 +64,8 @@ Table_type dd_frm_type(THD *thd, char *path, LEX_CSTRING *engine_name, uchar dbt; DBUG_ENTER("dd_frm_type"); - *is_sequence= 0; - - if ((file= mysql_file_open(key_file_frm, path, O_RDONLY | O_SHARE, MYF(0))) - < 0) + file= mysql_file_open(key_file_frm, path, O_RDONLY | O_SHARE, MYF(0)); + if (file < 0) DBUG_RETURN(TABLE_TYPE_UNKNOWN); /* @@ -110,7 +105,7 @@ Table_type dd_frm_type(THD *thd, char *path, LEX_CSTRING *engine_name, if (((header[39] >> 4) & 3) == HA_CHOICE_YES) { DBUG_PRINT("info", ("Sequence found")); - *is_sequence= 1; + type= TABLE_TYPE_SEQUENCE; } /* cannot use ha_resolve_by_legacy_type without a THD */ diff --git a/sql/datadict.h b/sql/datadict.h index cbdf788deb6..f4af592247a 100644 --- a/sql/datadict.h +++ b/sql/datadict.h @@ -38,13 +38,11 @@ enum Table_type To check whether it's an frm of a view, use dd_frm_is_view(). */ -enum Table_type dd_frm_type(THD *thd, char *path, LEX_CSTRING *engine_name, - bool *is_sequence); +enum Table_type dd_frm_type(THD *thd, char *path, LEX_CSTRING *engine_name); static inline bool dd_frm_is_view(THD *thd, char *path) { - bool not_used2; - return dd_frm_type(thd, path, NULL, ¬_used2) == TABLE_TYPE_VIEW; + return dd_frm_type(thd, path, NULL) == TABLE_TYPE_VIEW; } bool dd_recreate_table(THD *thd, const char *db, const char *table_name); diff --git a/sql/handler.cc b/sql/handler.cc index 28588235477..d81684f7c5f 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -5837,27 +5837,28 @@ bool ha_table_exists(THD *thd, const LEX_CSTRING *db, { char engine_buf[NAME_CHAR_LEN + 1]; LEX_CSTRING engine= { engine_buf, 0 }; - Table_type type; + Table_type type= dd_frm_type(thd, path, &engine); - if ((type= dd_frm_type(thd, path, &engine, is_sequence)) == - TABLE_TYPE_UNKNOWN) - { - DBUG_PRINT("exit", ("Does not exist")); + switch (type) { + case TABLE_TYPE_UNKNOWN: + DBUG_PRINT("exit", ("Exist, cannot be opened")); DBUG_RETURN(true); // Frm exists - } - if (type != TABLE_TYPE_VIEW) - { - plugin_ref p= plugin_lock_by_name(thd, &engine, - MYSQL_STORAGE_ENGINE_PLUGIN); - *hton= p ? plugin_hton(p) : NULL; - if (*hton) + case TABLE_TYPE_VIEW: + *hton= view_pseudo_hton; + DBUG_PRINT("exit", ("Exist, view")); + DBUG_RETURN(true); // Frm exists + case TABLE_TYPE_SEQUENCE: + *is_sequence= true; + /* fall through */ + case TABLE_TYPE_NORMAL: { - // verify that the table really exists - exists= discover_existence(thd, p, &args); + plugin_ref p= plugin_lock_by_name(thd, &engine, + MYSQL_STORAGE_ENGINE_PLUGIN); + *hton= p ? plugin_hton(p) : NULL; + if (*hton) // verify that the table really exists + exists= discover_existence(thd, p, &args); } } - else - *hton= view_pseudo_hton; } DBUG_PRINT("exit", (exists ? "Exists" : "Does not exist")); DBUG_RETURN(exists); diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index 4750be99d30..4526cac5af1 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -1720,8 +1720,7 @@ int plugin_init(int *argc, char **argv, int flags) { char path[FN_REFLEN + 1]; build_table_filename(path, sizeof(path) - 1, "mysql", "plugin", reg_ext, 0); - bool dummy; - Table_type ttype= dd_frm_type(0, path, &plugin_table_engine_name, &dummy); + Table_type ttype= dd_frm_type(0, path, &plugin_table_engine_name); if (ttype != TABLE_TYPE_NORMAL) plugin_table_engine_name=empty_clex_str; } diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 5eb2d911926..64076197cf8 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -4479,8 +4479,7 @@ static void get_table_engine_for_i_s(THD *thd, char *buf, TABLE_LIST *tl, char path[FN_REFLEN]; build_table_filename(path, sizeof(path) - 1, db->str, table->str, reg_ext, 0); - bool is_sequence; - if (dd_frm_type(thd, path, &engine_name, &is_sequence) == TABLE_TYPE_NORMAL) + if (dd_frm_type(thd, path, &engine_name) == TABLE_TYPE_NORMAL) tl->option= engine_name.str; } } diff --git a/storage/rocksdb/rdb_datadic.cc b/storage/rocksdb/rdb_datadic.cc index 6e73535d1de..13da87ad92c 100644 --- a/storage/rocksdb/rdb_datadic.cc +++ b/storage/rocksdb/rdb_datadic.cc @@ -3793,8 +3793,7 @@ bool Rdb_validate_tbls::check_frm_file(const std::string &fullpath, */ char eng_type_buf[NAME_CHAR_LEN+1]; LEX_CSTRING eng_type_str = {eng_type_buf, 0}; - bool is_sequence; - enum Table_type type = dd_frm_type(nullptr, fullfilename.c_ptr(), &eng_type_str, &is_sequence); + enum Table_type type = dd_frm_type(nullptr, fullfilename.c_ptr(), &eng_type_str); if (type == TABLE_TYPE_UNKNOWN) { // NO_LINT_DEBUG sql_print_warning("RocksDB: Failed to open/read .from file: %s", From 529b6dffe9e620d7cb1da30e1df6eebda66f01ee Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 15 Jun 2020 14:32:53 +0200 Subject: [PATCH 18/26] small cleanup --- sql/handler.cc | 3 +-- sql/sql_table.cc | 44 ++++++++++++++++++++------------------------ 2 files changed, 21 insertions(+), 26 deletions(-) diff --git a/sql/handler.cc b/sql/handler.cc index d81684f7c5f..2259e7a8e49 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -5011,8 +5011,7 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) !(hton->flags & HTON_AUTOMATIC_DELETE_TABLE)) { int error; - error= ha_delete_table(thd, hton, param->path, param->db, - param->alias, 0); + error= ha_delete_table(thd, hton, param->path, param->db, param->alias, 0); if (error > 0 && !non_existing_table_error(error)) param->error= error; if (error == 0) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 0d6e2c69695..b7e9b50cfbe 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2236,10 +2236,9 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, bool dont_free_locks) { TABLE_LIST *table; - char path[FN_REFLEN + 1], unknown_tables_buff[160]; + char path[FN_REFLEN + 1]; LEX_CSTRING alias= null_clex_str; - String unknown_tables(unknown_tables_buff, sizeof(unknown_tables_buff)-1, - system_charset_info); + StringBuffer<160> unknown_tables(system_charset_info); uint not_found_errors= 0; int error= 0; int non_temp_tables_count= 0; @@ -2311,14 +2310,15 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, bool table_creation_was_logged= 0; bool local_non_tmp_error= 0, frm_exists= 0, wrong_drop_sequence= 0; bool table_dropped= 0; - LEX_CSTRING db= table->db; + const LEX_CSTRING db= table->db; + const LEX_CSTRING table_name= table->table_name; handlerton *table_type= 0; size_t path_length= 0; char *path_end= 0; error= 0; DBUG_PRINT("table", ("table_l: '%s'.'%s' table: %p s: %p", - table->db.str, table->table_name.str, table->table, + db.str, table_name.str, table->table, table->table ? table->table->s : NULL)); /* @@ -2381,7 +2381,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, append_identifier(thd, built_ptr_query, &db); built_ptr_query->append("."); } - append_identifier(thd, built_ptr_query, &table->table_name); + append_identifier(thd, built_ptr_query, &table_name); built_ptr_query->append(","); } /* @@ -2396,11 +2396,10 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, { non_temp_tables_count++; - DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, table->db.str, - table->table_name.str, - MDL_SHARED)); + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db.str, + table_name.str, MDL_SHARED)); - alias= (lower_case_table_names == 2) ? table->alias : table->table_name; + alias= (lower_case_table_names == 2) ? table->alias : table_name; /* remove .frm file and engine files */ path_length= build_table_filename(path, sizeof(path) - 1, db.str, alias.str, reg_ext, 0); @@ -2466,12 +2465,11 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, table->table= 0; } else - tdc_remove_table(thd, table->db.str, table->table_name.str); + tdc_remove_table(thd, db.str, table_name.str); /* Check that we have an exclusive lock on the table to be dropped. */ - DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, table->db.str, - table->table_name.str, - MDL_EXCLUSIVE)); + DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db.str, + table_name.str, MDL_EXCLUSIVE)); // Remove extension for delete *path_end= '\0'; @@ -2482,7 +2480,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, thd->replication_flags= 0; error= ha_delete_table(thd, table_type, path, &db, - &table->table_name, !dont_log_query); + &table_name, !dont_log_query); if (!error) table_dropped= 1; @@ -2557,7 +2555,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, /* Remove extension for delete */ *path_end= '\0'; - ferror= ha_delete_table_force(thd, path, &db, &table->table_name); + ferror= ha_delete_table_force(thd, path, &db, &table_name); if (!ferror) { /* Table existed and was deleted */ @@ -2584,22 +2582,21 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, if (likely(!error) || non_existing_table_error(error)) { - if (Table_triggers_list::drop_all_triggers(thd, &db, &table->table_name, + if (Table_triggers_list::drop_all_triggers(thd, &db, &table_name, MYF(MY_WME | MY_IGNORE_ENOENT))) error= error ? error : -1; } if (error) { - char buff[FN_REFLEN]; - String tbl_name(buff, sizeof(buff), system_charset_info); + StringBuffer tbl_name(system_charset_info); uint is_note= (if_exists && (was_view || wrong_drop_sequence) ? ME_NOTE : 0); tbl_name.length(0); tbl_name.append(&db); tbl_name.append('.'); - tbl_name.append(&table->table_name); + tbl_name.append(&table_name); if (!non_existing_table_error(error) || is_note) { @@ -2640,9 +2637,8 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, if (!error && table_dropped) { PSI_CALL_drop_table_share(temporary_table_was_dropped, - table->db.str, (uint)table->db.length, - table->table_name.str, - (uint)table->table_name.length); + db.str, (uint)db.length, + table_name.str, (uint)table_name.length); mysql_audit_drop_table(thd, table); } @@ -2660,7 +2656,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, normal_tables.append("."); } - append_identifier(thd, &normal_tables, &table->table_name); + append_identifier(thd, &normal_tables, &table_name); normal_tables.append(","); } DBUG_PRINT("table", ("table: %p s: %p", table->table, From 79a3f96166c1c6b46bea859a357ec5c2a11d6b63 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 16 Jun 2020 09:34:38 +0200 Subject: [PATCH 19/26] rewrite bug#26704 test case drop-no_root needs DROP DATABASE to fail. But `chmod 000` is not reliable after drop table force anymore. Make DROP DATABASE to fail by creating an extra file in the db dir --- mysql-test/main/drop-no_root.result | 28 ---------- mysql-test/main/drop-no_root.test | 85 ----------------------------- mysql-test/main/drop.result | 13 +++++ mysql-test/main/drop.test | 21 +++++++ 4 files changed, 34 insertions(+), 113 deletions(-) delete mode 100644 mysql-test/main/drop-no_root.result delete mode 100644 mysql-test/main/drop-no_root.test diff --git a/mysql-test/main/drop-no_root.result b/mysql-test/main/drop-no_root.result deleted file mode 100644 index 3e1f2fe2cf0..00000000000 --- a/mysql-test/main/drop-no_root.result +++ /dev/null @@ -1,28 +0,0 @@ - -# -- -# -- Bug#26704: Failing DROP DATABASE brings mysql-client out of sync. -# -- - -DROP DATABASE IF EXISTS mysql_test; - -CREATE DATABASE mysql_test; -CREATE TABLE mysql_test.t1(c INT); -use mysql_test; - -chmod 000 mysql_test/t1.frm - -DROP DATABASE mysql_test; - -SELECT DATABASE(); -DATABASE() -mysql_test - -rm -f mysql_test/t1.MYD mysql_test/t1.MYI -chmod 666 mysql_test/t1.frm -rm -f mysql_test/t1.frm - -DROP DATABASE mysql_test; - -use test; - -# -- End of Bug#26704. diff --git a/mysql-test/main/drop-no_root.test b/mysql-test/main/drop-no_root.test deleted file mode 100644 index 8fb5b3f74a8..00000000000 --- a/mysql-test/main/drop-no_root.test +++ /dev/null @@ -1,85 +0,0 @@ -# This test uses chmod, can't be run with root permissions ---source include/not_as_root.inc - -########################################################################### - ---echo ---echo # -- ---echo # -- Bug#26704: Failing DROP DATABASE brings mysql-client out of sync. ---echo # -- - ---echo ---disable_warnings -DROP DATABASE IF EXISTS mysql_test; ---enable_warnings - ---echo -CREATE DATABASE mysql_test; -CREATE TABLE mysql_test.t1(c INT); - -use mysql_test; - -let $MYSQLD_DATADIR= `select @@datadir`; - ---echo ---echo chmod 000 mysql_test/t1.frm ---chmod 0000 $MYSQLD_DATADIR/mysql_test/t1.frm - -# NOTE: For the DROP DATABASE below we need: -# - disable result log because ER_DB_DROP_RMDIR contains errno, which can be -# different on different platforms. -# - expect different error codes, because Windows and UNIX behaves -# differently (see below). -# -# NOTE: Windows and UNIX behaves differently in this test case: -# -# - on UNIX when t1.frm is chmoded to 000, it is perfectly deleted -# by the first DROP DATABASE, but some other files (t1.MYI and t1.MYD) left -# in the directory. So, we have to explicitly removes them before the -# second DROP DATABASE. -# -# - on Windows when t1.frm is chmoded to 000, it is not deleted by the first -# DROP DATABASE, but all other files in the database directory are deleted. -# Thus, we have to change the t1.frm permissions again and delete it -# explicitly before the second DROP DATABASE. -# -# All those differences do not really matter for the idea of this test case: -# checking that if DROP DATABASE failed, the client is Ok. - ---echo ---disable_result_log ---error ER_DB_DROP_RMDIR,6 -DROP DATABASE mysql_test; ---enable_result_log - ---echo -SELECT DATABASE(); - -# Remove t1.MYI and t1.MYD. On UNIX it should succeed. On Windows, it fails. ---echo ---echo rm -f mysql_test/t1.MYD mysql_test/t1.MYI ---error 0, 1 ---remove_file $MYSQLD_DATADIR/mysql_test/t1.MYD ---error 0, 1 ---remove_file $MYSQLD_DATADIR/mysql_test/t1.MYI - -# Make t1.frm removable: fail on UNIX, succeed on Windows. ---echo chmod 666 mysql_test/t1.frm ---error 0, 1 ---chmod 0666 $MYSQLD_DATADIR/mysql_test/t1.frm - -# Remove t1.frm: fail on UNIX, succeed on Windows. ---echo rm -f mysql_test/t1.frm ---error 0, 1 ---remove_file $MYSQLD_DATADIR/mysql_test/t1.frm - ---echo -DROP DATABASE mysql_test; - ---echo -use test; - ---echo ---echo # -- End of Bug#26704. - -########################################################################### diff --git a/mysql-test/main/drop.result b/mysql-test/main/drop.result index 4755965cbb2..d50ffabc9fa 100644 --- a/mysql-test/main/drop.result +++ b/mysql-test/main/drop.result @@ -131,6 +131,19 @@ create table mysql_test.`#sql-347f_7` (f1 int); create table mysql_test.`#sql-347f_8` (f1 int); drop table mysql_test.`#sql-347f_8`; drop database mysql_test; +create database mysql_test; +use mysql_test; +create table t1(c int); +drop database mysql_test; +ERROR HY000: Error dropping database (can't rmdir './mysql_test', errno: 39 "Directory not empty") +select database(); +database() +mysql_test +drop database mysql_test; +select database(); +database() +NULL +use test; # -- # -- Bug#29958: Weird message on DROP DATABASE if mysql.proc does not diff --git a/mysql-test/main/drop.test b/mysql-test/main/drop.test index 2a2d2715185..17065a32347 100644 --- a/mysql-test/main/drop.test +++ b/mysql-test/main/drop.test @@ -175,6 +175,27 @@ let $MYSQLD_DATADIR= `select @@datadir`; copy_file $MYSQLD_DATADIR/mysql_test/t1.frm $MYSQLD_DATADIR/mysql_test/#sql-347f_6.frm; drop database mysql_test; +# +# Bug#26704: Failing DROP DATABASE brings mysql-client out of sync. +# + +create database mysql_test; +use mysql_test; +create table t1(c int); + +write_file $MYSQLD_DATADIR/mysql_test/do_not_delete; +do_not_delete +EOF + +replace_result $MYSQLD_DATADIR ./ \\ / 41 39; +error ER_DB_DROP_RMDIR; +drop database mysql_test; +select database(); +remove_file $MYSQLD_DATADIR/mysql_test/do_not_delete; +drop database mysql_test; +select database(); +use test; + ########################################################################### --echo From 7c2ba9e9d71c5fb494e262a1b6d1ebf992db282e Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Tue, 16 Jun 2020 10:33:48 +0200 Subject: [PATCH 20/26] MDEV-11412 Ensure that table is truly dropped when using DROP TABLE don't do table discovery on DROP. DROP falls back to "force" approach when a table isn't found and will try to drop in all engines anyway. That is, trying to discover in all engines before the drop is redundant and may be expensive. --- mysql-test/main/drop_combinations.result | 6 +- sql/ha_sequence.cc | 3 +- sql/handler.cc | 40 ++++----- sql/sql_table.cc | 88 +++++++++---------- storage/sequence/sequence.cc | 12 ++- .../test_sql_discovery/test_sql_discovery.cc | 9 ++ 6 files changed, 85 insertions(+), 73 deletions(-) diff --git a/mysql-test/main/drop_combinations.result b/mysql-test/main/drop_combinations.result index eb6d70e0704..140665efd19 100644 --- a/mysql-test/main/drop_combinations.result +++ b/mysql-test/main/drop_combinations.result @@ -168,8 +168,7 @@ drop table if exists t1,s1,v1,t3,t4; Warnings: Warning 1017 Can't find file: './test/t1.MYI' (errno: 2 "No such file or directory") Note 1965 'test.v1' is a view -Note 1965 'test.t3' is a view -Note 1965 'test.t4' is a view +Note 1051 Unknown table 'test.t3,test.t4' drop table if exists s2,v2,t2,t1; Warnings: Note 1965 'test.v2' is a view @@ -493,8 +492,7 @@ Warnings: Warning 1017 Can't find file: './test/s1.MYI' (errno: 2 "No such file or directory") Note 4090 'test.t1' is not a SEQUENCE Note 1965 'test.v1' is a view -Note 1965 'test.t3' is a view -Note 1965 'test.s4' is a view +Note 4091 Unknown SEQUENCE: 'test.t3,test.s4' drop sequence if exists t2,v2,s2,s1; Warnings: Note 4090 'test.t2' is not a SEQUENCE diff --git a/sql/ha_sequence.cc b/sql/ha_sequence.cc index bf3b5ce2cd0..596f8584041 100644 --- a/sql/ha_sequence.cc +++ b/sql/ha_sequence.cc @@ -439,8 +439,7 @@ static int sequence_initialize(void *p) HTON_HIDDEN | HTON_TEMPORARY_NOT_SUPPORTED | HTON_ALTER_NOT_SUPPORTED | - HTON_NO_PARTITION | - HTON_AUTOMATIC_DELETE_TABLE); + HTON_NO_PARTITION); DBUG_RETURN(0); } diff --git a/sql/handler.cc b/sql/handler.cc index 2259e7a8e49..e3ce0436a79 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -45,6 +45,7 @@ #include "sql_audit.h" #include "ha_sequence.h" #include "rowid_filter.h" +#include "mysys_err.h" #ifdef WITH_PARTITION_STORAGE_ENGINE #include "ha_partition.h" @@ -2771,18 +2772,20 @@ int ha_delete_table(THD *thd, handlerton *hton, const char *path, TABLE dummy_table; TABLE_SHARE dummy_share; handler *file= get_new_handler(nullptr, thd->mem_root, hton); - bzero((char*) &dummy_table, sizeof(dummy_table)); - bzero((char*) &dummy_share, sizeof(dummy_share)); - dummy_share.path.str= (char*) path; - dummy_share.path.length= strlen(path); - dummy_share.normalized_path= dummy_share.path; - dummy_share.db= *db; - dummy_share.table_name= *alias; - dummy_table.s= &dummy_share; - dummy_table.alias.set(alias->str, alias->length, table_alias_charset); - file->change_table_ptr(&dummy_table, &dummy_share); - file->print_error(error, MYF(intercept ? ME_WARNING : 0)); - delete file; + if (file) { + bzero((char*) &dummy_table, sizeof(dummy_table)); + bzero((char*) &dummy_share, sizeof(dummy_share)); + dummy_share.path.str= (char*) path; + dummy_share.path.length= strlen(path); + dummy_share.normalized_path= dummy_share.path; + dummy_share.db= *db; + dummy_share.table_name= *alias; + dummy_table.s= &dummy_share; + dummy_table.alias.set(alias->str, alias->length, table_alias_charset); + file->change_table_ptr(&dummy_table, &dummy_share); + file->print_error(error, MYF(intercept ? ME_WARNING : 0)); + delete file; + } } if (intercept) { @@ -4455,10 +4458,6 @@ int handler::delete_table(const char *name) bool some_file_deleted= 0; DBUG_ENTER("handler::delete_table"); - // For discovery tables, it's ok if first file doesn't exists - if (ht->discover_table) - saved_error= 0; - for (const char **ext= bas_ext(); *ext ; ext++) { int err= mysql_file_delete_with_symlink(key_file_misc, name, *ext, MYF(0)); @@ -4522,7 +4521,9 @@ void handler::drop_table(const char *name) bool non_existing_table_error(int error) { - return (error == ENOENT || error == HA_ERR_NO_SUCH_TABLE || + return (error == ENOENT || + (error == EE_DELETE && my_errno == ENOENT) || + error == HA_ERR_NO_SUCH_TABLE || error == HA_ERR_UNSUPPORTED || error == ER_NO_SUCH_TABLE || error == ER_NO_SUCH_TABLE_IN_ENGINE || @@ -5002,12 +5003,11 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) /* We have to ignore HEAP tables as these may not have been created yet - We also remove engines that is using discovery (as these will recrate - any missing .frm if needed) and tables marked with + We also remove engines marked with HTON_AUTOMATIC_DELETE_TABLE as for these we can't check if the table ever existed. */ - if (!hton->discover_table && hton->db_type != DB_TYPE_HEAP && + if (hton->db_type != DB_TYPE_HEAP && !(hton->flags & HTON_AUTOMATIC_DELETE_TABLE)) { int error; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index b7e9b50cfbe..2237d7bc70d 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2246,7 +2246,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, bool trans_tmp_table_deleted= 0, non_trans_tmp_table_deleted= 0; bool non_tmp_table_deleted= 0; bool is_drop_tmp_if_exists_added= 0; - bool was_view= 0, was_table= 0, is_sequence, log_if_exists= if_exists; + bool was_view= 0, was_table= 0, log_if_exists= if_exists; const char *object_to_drop= (drop_sequence) ? "SEQUENCE" : "TABLE"; String normal_tables; String built_trans_tmp_query, built_non_trans_tmp_query; @@ -2306,13 +2306,14 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, for (table= tables; table; table= table->next_local) { - bool is_trans= 0, frm_was_deleted= 0, temporary_table_was_dropped= 0; + bool is_trans= 0, temporary_table_was_dropped= 0; bool table_creation_was_logged= 0; - bool local_non_tmp_error= 0, frm_exists= 0, wrong_drop_sequence= 0; + bool local_non_tmp_error= 0, wrong_drop_sequence= 0; bool table_dropped= 0; const LEX_CSTRING db= table->db; const LEX_CSTRING table_name= table->table_name; - handlerton *table_type= 0; + handlerton *hton= 0; + Table_type table_type; size_t path_length= 0; char *path_end= 0; @@ -2410,13 +2411,32 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, if (drop_temporary) { /* "DROP TEMPORARY" but a temporary table was not found */ + unknown_tables.append(&db); + unknown_tables.append('.'); + unknown_tables.append(&table_name); + unknown_tables.append(','); error= ENOENT; + not_found_errors++; + continue; } - else if (((frm_exists= ha_table_exists(thd, &db, &alias, &table_type, - &is_sequence)) == 0 && - table_type == 0) || - (!drop_view && (was_view= (table_type == view_pseudo_hton))) || - (drop_sequence && !is_sequence)) + + { + char engine_buf[NAME_CHAR_LEN + 1]; + LEX_CSTRING engine= { engine_buf, 0 }; + + table_type= dd_frm_type(thd, path, &engine); + if (table_type == TABLE_TYPE_NORMAL || table_type == TABLE_TYPE_SEQUENCE) + { + plugin_ref p= plugin_lock_by_name(thd, &engine, + MYSQL_STORAGE_ENGINE_PLUGIN); + hton= p ? plugin_hton(p) : NULL; + } + // note that for TABLE_TYPE_VIEW and TABLE_TYPE_UNKNOWN hton == NULL + } + + was_view= table_type == TABLE_TYPE_VIEW; + if ((table_type == TABLE_TYPE_UNKNOWN) || (was_view && !drop_view) || + (table_type != TABLE_TYPE_SEQUENCE && drop_sequence)) { /* One of the following cases happened: @@ -2424,34 +2444,19 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, . "DROP TABLE" statement, but it's a view. . "DROP SEQUENCE", but it's not a sequence */ - wrong_drop_sequence= drop_sequence && table_type; + wrong_drop_sequence= drop_sequence && hton; was_table|= wrong_drop_sequence; local_non_tmp_error= 1; - error= -1; - if ((!frm_exists && !table_type)) // no .frm - error= ENOENT; + error= table_type == TABLE_TYPE_UNKNOWN ? ENOENT : -1; } else { - if (WSREP(thd) && !wsrep_should_replicate_ddl(thd, table_type->db_type)) + if (WSREP(thd) && hton && !wsrep_should_replicate_ddl(thd, hton->db_type)) { error= 1; goto err; } - /* - It could happen that table's share in the table definition cache - is the only thing that keeps the engine plugin loaded - (if it is uninstalled and waits for the ref counter to drop to 0). - - In this case, the tdc_remove_table() below will release and unload - the plugin. And ha_delete_table() will get a dangling pointer. - - Let's lock the plugin till the end of the statement. - */ - if (table_type && table_type != view_pseudo_hton) - ha_lock_engine(thd, table_type); - if (thd->locked_tables_mode == LTM_LOCK_TABLES || thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES) { @@ -2474,13 +2479,12 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, // Remove extension for delete *path_end= '\0'; - if (table_type && table_type != view_pseudo_hton && - table_type->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) + if (hton && hton->flags & HTON_TABLE_MAY_NOT_EXIST_ON_SLAVE) log_if_exists= 1; thd->replication_flags= 0; - error= ha_delete_table(thd, table_type, path, &db, - &table_name, !dont_log_query); + bool enoent_warning= !dont_log_query && !(hton && hton->discover_table); + error= ha_delete_table(thd, hton, path, &db, &table_name, enoent_warning); if (!error) table_dropped= 1; @@ -2508,8 +2512,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, { int frm_delete_error= 0; /* Delete the table definition file */ - if (table_type && table_type != view_pseudo_hton && - (table_type->discover_table || error)) + if (hton && (hton->discover_table || error)) { /* Table type is using discovery and may not need a .frm file @@ -2527,7 +2530,6 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, frm_delete_error= my_errno; DBUG_ASSERT(frm_delete_error); } - frm_was_deleted= 1; // We tried to delete .frm if (frm_delete_error) { @@ -2548,10 +2550,10 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, scan all engines try to drop the table from there. This is to ensure we don't have any partial table files left. */ - if (non_existing_table_error(error) && !drop_temporary && - table_type != view_pseudo_hton && !wrong_drop_sequence) + if (non_existing_table_error(error) && !wrong_drop_sequence) { int ferror= 0; + DBUG_ASSERT(!was_view); /* Remove extension for delete */ *path_end= '\0'; @@ -2567,14 +2569,10 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, { ferror= 0; // Ignore table not found - /* Delete the table definition file */ - if (!frm_was_deleted) - { - strmov(path_end, reg_ext); - if (mysql_file_delete(key_file_frm, path, - MYF(MY_WME | MY_IGNORE_ENOENT))) - ferror= my_errno; - } + /* Delete the frm file again (just in case it was rediscovered) */ + strmov(path_end, reg_ext); + if (mysql_file_delete(key_file_frm, path, MYF(MY_WME|MY_IGNORE_ENOENT))) + ferror= my_errno; } if (!error) error= ferror; @@ -2642,7 +2640,7 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, mysql_audit_drop_table(thd, table); } - if (!dont_log_query && !drop_temporary && + if (!dont_log_query && (!error || table_dropped || non_existing_table_error(error))) { non_tmp_table_deleted|= (if_exists || table_dropped); diff --git a/storage/sequence/sequence.cc b/storage/sequence/sequence.cc index ddebb1a1a8d..c8f3e76b873 100644 --- a/storage/sequence/sequence.cc +++ b/storage/sequence/sequence.cc @@ -497,19 +497,27 @@ int ha_seq_group_by_handler::next_row() Initialize the interface between the sequence engine and MariaDB *****************************************************************************/ +static int drop_table(handlerton *hton, const char *path) +{ + const char *name= strrchr(path, FN_LIBCHAR)+1; + ulonglong from, to, step; + if (parse_table_name(name, strlen(name), &from, &to, &step)) + return ENOENT; + return 0; +} + static int init(void *p) { handlerton *hton= (handlerton *)p; sequence_hton= hton; hton->create= create_handler; - hton->drop_table= [](handlerton *, const char*) { return 0; }; + hton->drop_table= drop_table; hton->discover_table= discover_table; hton->discover_table_existence= discover_table_existence; hton->commit= hton->rollback= dummy_commit_rollback; hton->savepoint_set= hton->savepoint_rollback= hton->savepoint_release= dummy_savepoint; hton->create_group_by= create_group_by_handler; - hton->flags= HTON_AUTOMATIC_DELETE_TABLE; return 0; } diff --git a/storage/test_sql_discovery/test_sql_discovery.cc b/storage/test_sql_discovery/test_sql_discovery.cc index 9e7a22368fc..0758d5f503f 100644 --- a/storage/test_sql_discovery/test_sql_discovery.cc +++ b/storage/test_sql_discovery/test_sql_discovery.cc @@ -147,11 +147,20 @@ static int discover_table(handlerton *hton, THD* thd, TABLE_SHARE *share) sql, strlen(sql)); } +static int drop_table(handlerton *hton, const char *path) +{ + const char *name= strrchr(path, FN_LIBCHAR)+1; + const char *sql= THDVAR(current_thd, statement); + return !sql || strncmp(sql, name, strlen(name)) || sql[strlen(name)] != ':' + ? ENOENT : 0; +} + static int init(void *p) { handlerton *hton = (handlerton *)p; hton->create = create_handler; hton->discover_table = discover_table; + hton->drop_table= drop_table; return 0; } From 4876651e0f4b2ee0e16b5f86c7f65464e6ac6906 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 22 Jun 2020 13:21:18 +0200 Subject: [PATCH 21/26] remove mysql_declare_plugin declaration from some plugins --- plugin/feedback/feedback.cc | 19 ---------------- .../metadata_lock_info/metadata_lock_info.cc | 22 ------------------- storage/blackhole/ha_blackhole.cc | 17 -------------- storage/example/ha_example.cc | 17 -------------- storage/federated/ha_federated.cc | 17 -------------- storage/heap/ha_heap.cc | 17 -------------- storage/myisam/ha_myisam.cc | 17 -------------- storage/perfschema/ha_perfschema.cc | 18 --------------- 8 files changed, 144 deletions(-) diff --git a/plugin/feedback/feedback.cc b/plugin/feedback/feedback.cc index 03cc4ab465a..eb3f562f329 100644 --- a/plugin/feedback/feedback.cc +++ b/plugin/feedback/feedback.cc @@ -409,24 +409,6 @@ static struct st_mysql_information_schema feedback = } // namespace feedback -mysql_declare_plugin(feedback) -{ - MYSQL_INFORMATION_SCHEMA_PLUGIN, - &feedback::feedback, - "FEEDBACK", - "Sergei Golubchik", - "MariaDB User Feedback Plugin", - PLUGIN_LICENSE_GPL, - feedback::init, - feedback::free, - 0x0101, - NULL, - feedback::settings, - NULL, - 0 -} -mysql_declare_plugin_end; -#ifdef MARIA_PLUGIN_INTERFACE_VERSION maria_declare_plugin(feedback) { MYSQL_INFORMATION_SCHEMA_PLUGIN, @@ -444,4 +426,3 @@ maria_declare_plugin(feedback) MariaDB_PLUGIN_MATURITY_STABLE } maria_declare_plugin_end; -#endif diff --git a/plugin/metadata_lock_info/metadata_lock_info.cc b/plugin/metadata_lock_info/metadata_lock_info.cc index 46c31ca3a96..a85048de47f 100644 --- a/plugin/metadata_lock_info/metadata_lock_info.cc +++ b/plugin/metadata_lock_info/metadata_lock_info.cc @@ -125,7 +125,6 @@ static int i_s_metadata_lock_info_deinit( static struct st_mysql_information_schema i_s_metadata_lock_info_plugin = { MYSQL_INFORMATION_SCHEMA_INTERFACE_VERSION }; -#ifdef MARIADB_BASE_VERSION maria_declare_plugin(metadata_lock_info) { MYSQL_INFORMATION_SCHEMA_PLUGIN, @@ -143,24 +142,3 @@ maria_declare_plugin(metadata_lock_info) MariaDB_PLUGIN_MATURITY_STABLE } maria_declare_plugin_end; -#else -mysql_declare_plugin(metadata_lock_info) -{ - MYSQL_INFORMATION_SCHEMA_PLUGIN, - &i_s_metadata_lock_info_plugin, - "METADATA_LOCK_INFO", - "Kentoku Shiba", - "Metadata locking viewer", - PLUGIN_LICENSE_GPL, - i_s_metadata_lock_info_init, - i_s_metadata_lock_info_deinit, - 0x0001, - NULL, - NULL, - NULL, -#if MYSQL_VERSION_ID >= 50600 - 0, -#endif -} -mysql_declare_plugin_end; -#endif diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc index 82ac9123d3b..c4eba8b4171 100644 --- a/storage/blackhole/ha_blackhole.cc +++ b/storage/blackhole/ha_blackhole.cc @@ -423,23 +423,6 @@ static int blackhole_fini(void *p) struct st_mysql_storage_engine blackhole_storage_engine= { MYSQL_HANDLERTON_INTERFACE_VERSION }; -mysql_declare_plugin(blackhole) -{ - MYSQL_STORAGE_ENGINE_PLUGIN, - &blackhole_storage_engine, - "BLACKHOLE", - "MySQL AB", - "/dev/null storage engine (anything you write to it disappears)", - PLUGIN_LICENSE_GPL, - blackhole_init, /* Plugin Init */ - blackhole_fini, /* Plugin Deinit */ - 0x0100 /* 1.0 */, - NULL, /* status variables */ - NULL, /* system variables */ - NULL, /* config options */ - 0, /* flags */ -} -mysql_declare_plugin_end; maria_declare_plugin(blackhole) { MYSQL_STORAGE_ENGINE_PLUGIN, diff --git a/storage/example/ha_example.cc b/storage/example/ha_example.cc index f6760c06f2d..30484a41087 100644 --- a/storage/example/ha_example.cc +++ b/storage/example/ha_example.cc @@ -1094,23 +1094,6 @@ static struct st_mysql_show_var func_status[]= struct st_mysql_daemon unusable_example= { MYSQL_DAEMON_INTERFACE_VERSION }; -mysql_declare_plugin(example) -{ - MYSQL_STORAGE_ENGINE_PLUGIN, - &example_storage_engine, - "EXAMPLE", - "Brian Aker, MySQL AB", - "Example storage engine", - PLUGIN_LICENSE_GPL, - example_init_func, /* Plugin Init */ - NULL, /* Plugin Deinit */ - 0x0001 /* 0.1 */, - func_status, /* status variables */ - example_system_variables, /* system variables */ - NULL, /* config options */ - 0, /* flags */ -} -mysql_declare_plugin_end; maria_declare_plugin(example) { MYSQL_STORAGE_ENGINE_PLUGIN, diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index 766f9a2b790..c0e5c330edb 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -3390,23 +3390,6 @@ int ha_federated::execute_simple_query(const char *query, int len) struct st_mysql_storage_engine federated_storage_engine= { MYSQL_HANDLERTON_INTERFACE_VERSION }; -mysql_declare_plugin(federated) -{ - MYSQL_STORAGE_ENGINE_PLUGIN, - &federated_storage_engine, - "FEDERATED", - "Patrick Galbraith and Brian Aker, MySQL AB", - "Federated MySQL storage engine", - PLUGIN_LICENSE_GPL, - federated_db_init, /* Plugin Init */ - federated_done, /* Plugin Deinit */ - 0x0100 /* 1.0 */, - NULL, /* status variables */ - NULL, /* system variables */ - NULL, /* config options */ - 0, /* flags */ -} -mysql_declare_plugin_end; maria_declare_plugin(federated) { MYSQL_STORAGE_ENGINE_PLUGIN, diff --git a/storage/heap/ha_heap.cc b/storage/heap/ha_heap.cc index 0e61bff1d8d..38996dd0c93 100644 --- a/storage/heap/ha_heap.cc +++ b/storage/heap/ha_heap.cc @@ -839,23 +839,6 @@ int ha_heap::find_unique_row(uchar *record, uint unique_idx) struct st_mysql_storage_engine heap_storage_engine= { MYSQL_HANDLERTON_INTERFACE_VERSION }; -mysql_declare_plugin(heap) -{ - MYSQL_STORAGE_ENGINE_PLUGIN, - &heap_storage_engine, - "MEMORY", - "MySQL AB", - "Hash based, stored in memory, useful for temporary tables", - PLUGIN_LICENSE_GPL, - heap_init, - NULL, - 0x0100, /* 1.0 */ - NULL, /* status variables */ - NULL, /* system variables */ - NULL, /* config options */ - 0, /* flags */ -} -mysql_declare_plugin_end; maria_declare_plugin(heap) { MYSQL_STORAGE_ENGINE_PLUGIN, diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index e58ee9e31e4..92fb49bea8d 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -2648,23 +2648,6 @@ bool ha_myisam::rowid_filter_push(Rowid_filter* rowid_filter) struct st_mysql_storage_engine myisam_storage_engine= { MYSQL_HANDLERTON_INTERFACE_VERSION }; -mysql_declare_plugin(myisam) -{ - MYSQL_STORAGE_ENGINE_PLUGIN, - &myisam_storage_engine, - "MyISAM", - "MySQL AB", - "MyISAM storage engine", - PLUGIN_LICENSE_GPL, - myisam_init, /* Plugin Init */ - NULL, /* Plugin Deinit */ - 0x0100, /* 1.0 */ - NULL, /* status variables */ - myisam_sysvars, /* system variables */ - NULL, - 0, -} -mysql_declare_plugin_end; maria_declare_plugin(myisam) { MYSQL_STORAGE_ENGINE_PLUGIN, diff --git a/storage/perfschema/ha_perfschema.cc b/storage/perfschema/ha_perfschema.cc index d36a7d7322a..1509936fe53 100644 --- a/storage/perfschema/ha_perfschema.cc +++ b/storage/perfschema/ha_perfschema.cc @@ -219,24 +219,6 @@ struct st_mysql_storage_engine pfs_storage_engine= const char* pfs_engine_name= "PERFORMANCE_SCHEMA"; -mysql_declare_plugin(perfschema) -{ - MYSQL_STORAGE_ENGINE_PLUGIN, - &pfs_storage_engine, - pfs_engine_name, - "Marc Alff, Oracle", /* Formerly Sun Microsystems, formerly MySQL */ - "Performance Schema", - PLUGIN_LICENSE_GPL, - pfs_init_func, /* Plugin Init */ - pfs_done_func, /* Plugin Deinit */ - 0x0001 /* 0.1 */, - pfs_status_vars, /* status variables */ - NULL, /* system variables */ - NULL, /* config options */ - 0, /* flags */ -} -mysql_declare_plugin_end; - maria_declare_plugin(perfschema) { MYSQL_STORAGE_ENGINE_PLUGIN, From 6c52931680a4a24da04d7de7b74321d9ff507745 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 22 Jun 2020 22:14:25 +0200 Subject: [PATCH 22/26] replace HTON_AUTOMATIC_DELETE_TABLE with return -1 from drop_table() --- sql/handler.cc | 28 +++++++++------------------- sql/handler.h | 12 +++++------- sql/log.cc | 7 ++----- sql/temporary_tables.cc | 2 +- storage/blackhole/ha_blackhole.cc | 4 ++-- storage/federated/ha_federated.cc | 5 ++--- storage/federatedx/ha_federatedx.cc | 5 ++--- storage/heap/ha_heap.cc | 2 +- storage/perfschema/ha_perfschema.cc | 9 +++------ storage/sphinx/ha_sphinx.cc | 4 ++-- storage/spider/spd_table.cc | 2 +- 11 files changed, 30 insertions(+), 50 deletions(-) diff --git a/sql/handler.cc b/sql/handler.cc index e3ce0436a79..d601bbb8c87 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -2758,14 +2758,14 @@ int ha_delete_table(THD *thd, handlerton *hton, const char *path, if (hton == NULL || hton == view_pseudo_hton) DBUG_RETURN(0); - if (unlikely((error= hton->drop_table(hton, path)))) + error= hton->drop_table(hton, path); + if (error > 0) { /* It's not an error if the table doesn't exist in the engine. warn the user, but still report DROP being a success */ bool intercept= non_existing_table_error(error); - DBUG_ASSERT(error > 0); if ((!intercept || generate_warning) && ! thd->is_error()) { @@ -5001,24 +5001,14 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) handlerton *hton = plugin_hton(plugin); st_force_drop_table_params *param = (st_force_drop_table_params *)arg; - /* - We have to ignore HEAP tables as these may not have been created yet - We also remove engines marked with - HTON_AUTOMATIC_DELETE_TABLE as for these we can't check if the table - ever existed. - */ - if (hton->db_type != DB_TYPE_HEAP && - !(hton->flags & HTON_AUTOMATIC_DELETE_TABLE)) + int error; + error= ha_delete_table(thd, hton, param->path, param->db, param->alias, 0); + if (error > 0 && !non_existing_table_error(error)) + param->error= error; + if (error == 0) { - int error; - error= ha_delete_table(thd, hton, param->path, param->db, param->alias, 0); - if (error > 0 && !non_existing_table_error(error)) - param->error= error; - if (error == 0) - { - param->error= 0; - return TRUE; // Table was deleted - } + param->error= 0; + return TRUE; // Table was deleted } return FALSE; } diff --git a/sql/handler.h b/sql/handler.h index 4d9f92d3126..e8315b6ad9a 100644 --- a/sql/handler.h +++ b/sql/handler.h @@ -1484,6 +1484,11 @@ struct handlerton void (*close_cursor_read_view)(handlerton *hton, THD *thd, void *read_view); handler *(*create)(handlerton *hton, TABLE_SHARE *table, MEM_ROOT *mem_root); void (*drop_database)(handlerton *hton, char* path); + /* + return 0 if dropped successfully, + -1 if nothing was done by design (as in e.g. blackhole) + an error code (e.g. HA_ERR_NO_SUCH_TABLE) otherwise + */ int (*drop_table)(handlerton *hton, const char* path); int (*panic)(handlerton *hton, enum ha_panic_function flag); int (*start_consistent_snapshot)(handlerton *hton, THD *thd); @@ -1791,13 +1796,6 @@ handlerton *ha_default_tmp_handlerton(THD *thd); */ #define HTON_TRANSACTIONAL_AND_NON_TRANSACTIONAL (1 << 17) -/* - The engine doesn't keep track of tables, delete_table() is not - needed and delete_table() always returns 0 (table deleted). This flag - mainly used to skip storage engines in case of ha_delete_table_force() -*/ -#define HTON_AUTOMATIC_DELETE_TABLE (1 << 18) - class Ha_trx_info; struct THD_TRANS diff --git a/sql/log.cc b/sql/log.cc index b6778f34768..5f4fd6bbcab 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -1692,7 +1692,7 @@ int binlog_init(void *p) binlog_savepoint_rollback_can_release_mdl; binlog_hton->commit= binlog_commit; binlog_hton->rollback= binlog_rollback; - binlog_hton->drop_table= [](handlerton *, const char*) { return 0; }; + binlog_hton->drop_table= [](handlerton *, const char*) { return -1; }; if (WSREP_ON || opt_bin_log) { binlog_hton->prepare= binlog_prepare; @@ -1702,10 +1702,7 @@ int binlog_init(void *p) // recover needs to be set to make xa{commit,rollback}_handlerton effective binlog_hton->recover= binlog_xa_recover_dummy; } - binlog_hton->flags= (HTON_NOT_USER_SELECTABLE | - HTON_HIDDEN | - HTON_NO_ROLLBACK | - HTON_AUTOMATIC_DELETE_TABLE); + binlog_hton->flags= HTON_NOT_USER_SELECTABLE | HTON_HIDDEN | HTON_NO_ROLLBACK; return 0; } diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc index 6a507b99ef6..1a8b5c471bd 100644 --- a/sql/temporary_tables.cc +++ b/sql/temporary_tables.cc @@ -700,7 +700,7 @@ bool THD::rm_temporary_table(handlerton *base, const char *path) if (mysql_file_delete(key_file_frm, frm_path, MYF(MY_WME | MY_IGNORE_ENOENT))) error= true; - if (base->drop_table(base, path)) + if (base->drop_table(base, path) > 0) { error= true; sql_print_warning("Could not remove temporary table: '%s', error: %d", diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc index c4eba8b4171..0134032351e 100644 --- a/storage/blackhole/ha_blackhole.cc +++ b/storage/blackhole/ha_blackhole.cc @@ -399,8 +399,8 @@ static int blackhole_init(void *p) blackhole_hton= (handlerton *)p; blackhole_hton->db_type= DB_TYPE_BLACKHOLE_DB; blackhole_hton->create= blackhole_create_handler; - blackhole_hton->drop_table= [](handlerton *, const char*) { return 0; }; - blackhole_hton->flags= HTON_CAN_RECREATE | HTON_AUTOMATIC_DELETE_TABLE; + blackhole_hton->drop_table= [](handlerton *, const char*) { return -1; }; + blackhole_hton->flags= HTON_CAN_RECREATE; mysql_mutex_init(bh_key_mutex_blackhole, &blackhole_mutex, MY_MUTEX_INIT_FAST); diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index c0e5c330edb..871a254f8c0 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -484,9 +484,8 @@ int federated_db_init(void *p) federated_hton->commit= federated_commit; federated_hton->rollback= federated_rollback; federated_hton->create= federated_create_handler; - federated_hton->drop_table= [](handlerton *, const char*) { return 0; }; - federated_hton->flags= (HTON_ALTER_NOT_SUPPORTED | HTON_NO_PARTITION | - HTON_AUTOMATIC_DELETE_TABLE); + federated_hton->drop_table= [](handlerton *, const char*) { return -1; }; + federated_hton->flags= HTON_ALTER_NOT_SUPPORTED | HTON_NO_PARTITION; /* Support for transactions disabled until WL#2952 fixes it. diff --git a/storage/federatedx/ha_federatedx.cc b/storage/federatedx/ha_federatedx.cc index 45029fdf17f..dfcbf156f88 100644 --- a/storage/federatedx/ha_federatedx.cc +++ b/storage/federatedx/ha_federatedx.cc @@ -438,9 +438,8 @@ int federatedx_db_init(void *p) federatedx_hton->rollback= ha_federatedx::rollback; federatedx_hton->discover_table_structure= ha_federatedx::discover_assisted; federatedx_hton->create= federatedx_create_handler; - federatedx_hton->drop_table= [](handlerton *, const char*) { return 0; }; - federatedx_hton->flags= (HTON_ALTER_NOT_SUPPORTED | - HTON_AUTOMATIC_DELETE_TABLE); + federatedx_hton->drop_table= [](handlerton *, const char*) { return -1; }; + federatedx_hton->flags= HTON_ALTER_NOT_SUPPORTED; federatedx_hton->create_derived= create_federatedx_derived_handler; federatedx_hton->create_select= create_federatedx_select_handler; diff --git a/storage/heap/ha_heap.cc b/storage/heap/ha_heap.cc index 38996dd0c93..895b6edfcf2 100644 --- a/storage/heap/ha_heap.cc +++ b/storage/heap/ha_heap.cc @@ -43,7 +43,7 @@ static int heap_panic(handlerton *hton, ha_panic_function flag) static int heap_drop_table(handlerton *hton, const char *path) { int error= heap_delete_table(path); - return error == ENOENT ? 0 : error; + return error == ENOENT ? -1 : error; } int heap_init(void *p) diff --git a/storage/perfschema/ha_perfschema.cc b/storage/perfschema/ha_perfschema.cc index 1509936fe53..f54d46ce979 100644 --- a/storage/perfschema/ha_perfschema.cc +++ b/storage/perfschema/ha_perfschema.cc @@ -94,13 +94,10 @@ static int pfs_init_func(void *p) pfs_hton= reinterpret_cast (p); pfs_hton->create= pfs_create_handler; - pfs_hton->drop_table= [](handlerton *, const char*) { return 0; }; + pfs_hton->drop_table= [](handlerton *, const char*) { return -1; }; pfs_hton->show_status= pfs_show_status; - pfs_hton->flags= (HTON_ALTER_NOT_SUPPORTED | - HTON_TEMPORARY_NOT_SUPPORTED | - HTON_NO_PARTITION | - HTON_NO_BINLOG_ROW_OPT | - HTON_AUTOMATIC_DELETE_TABLE); + pfs_hton->flags= HTON_ALTER_NOT_SUPPORTED | HTON_TEMPORARY_NOT_SUPPORTED | + HTON_NO_PARTITION | HTON_NO_BINLOG_ROW_OPT; /* As long as the server implementation keeps using legacy_db_type, diff --git a/storage/sphinx/ha_sphinx.cc b/storage/sphinx/ha_sphinx.cc index 39e07290853..ca68a5feb60 100644 --- a/storage/sphinx/ha_sphinx.cc +++ b/storage/sphinx/ha_sphinx.cc @@ -749,8 +749,8 @@ static int sphinx_init_func ( void * p ) hton->close_connection = sphinx_close_connection; hton->show_status = sphinx_show_status; hton->panic = sphinx_panic; - hton->drop_table= [](handlerton *, const char*) { return 0; }; - hton->flags = HTON_CAN_RECREATE | HTON_AUTOMATIC_DELETE_TABLE; + hton->drop_table= [](handlerton *, const char*) { return -1; }; + hton->flags = HTON_CAN_RECREATE; #endif } SPH_RET(0); diff --git a/storage/spider/spd_table.cc b/storage/spider/spd_table.cc index f4b0177c794..e77e1a9261b 100644 --- a/storage/spider/spd_table.cc +++ b/storage/spider/spd_table.cc @@ -7249,7 +7249,7 @@ int spider_db_init( DBUG_ENTER("spider_db_init"); spider_hton_ptr = spider_hton; - spider_hton->flags = HTON_NO_FLAGS | HTON_AUTOMATIC_DELETE_TABLE; + spider_hton->flags = HTON_NO_FLAGS; #ifdef HTON_CAN_READ_CONNECT_STRING_IN_PARTITION spider_hton->flags |= HTON_CAN_READ_CONNECT_STRING_IN_PARTITION; #endif From 4227dd2ac6659cf65e2fc583ae6060e301cd673c Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 25 Jun 2020 18:51:45 +0200 Subject: [PATCH 23/26] continue DROP TEMPORARY TABLE t1, t2, t3 after error. normal DROP TABLE with many tables continues after an error, trying to drop as many tables as possible. But DROP TEMPORARY TABLE was aborting on the first error. Change it to behave as DROP TABLE does. --- mysql-test/main/temp_table.result | 21 ++++++++++++++++++-- mysql-test/main/temp_table.test | 33 ++++++++++++++++++++++++++----- sql/sql_table.cc | 13 ++++-------- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/mysql-test/main/temp_table.result b/mysql-test/main/temp_table.result index 293b6d5cd77..64a5d9b681b 100644 --- a/mysql-test/main/temp_table.result +++ b/mysql-test/main/temp_table.result @@ -1,5 +1,3 @@ -drop table if exists t1,t2; -drop view if exists v1; # # test basic creation of temporary tables together with normal table # @@ -602,3 +600,22 @@ DROP TEMPORARY TABLE t1; # # End of 10.2 tests # +create function f1() returns int +begin +drop temporary table t1, t2; +return 1; +end; +$$ +create temporary table t1 (a int); +create temporary table t2 (a int); +insert t1 values (2); +insert t2 values (3); +select a,f1() from t1; +ERROR HY000: Can't reopen table: 't1' +drop function f1; +drop temporary table t1; +drop temporary table t2; +ERROR 42S02: Unknown table 'test.t2' +# +# End of 10.5 tests +# diff --git a/mysql-test/main/temp_table.test b/mysql-test/main/temp_table.test index dc5fe7f3cd0..ccaa5fb93e8 100644 --- a/mysql-test/main/temp_table.test +++ b/mysql-test/main/temp_table.test @@ -6,11 +6,6 @@ # Test of temporary tables # ---disable_warnings -drop table if exists t1,t2; -drop view if exists v1; ---enable_warnings - --echo # --echo # test basic creation of temporary tables together with normal table --echo # @@ -658,3 +653,31 @@ DROP TEMPORARY TABLE t1; --echo # --echo # End of 10.2 tests --echo # + +# +# DROP TEMPORARY TABLE fails in the middle +# +delimiter $$; +create function f1() returns int +begin + drop temporary table t1, t2; + return 1; +end; +$$ +delimiter ;$$ + +create temporary table t1 (a int); +create temporary table t2 (a int); +insert t1 values (2); +insert t2 values (3); +--error ER_CANT_REOPEN_TABLE +select a,f1() from t1; +drop function f1; +drop temporary table t1; +--error ER_BAD_TABLE_ERROR +drop temporary table t2; + +--echo # +--echo # End of 10.5 tests +--echo # + diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 2237d7bc70d..87257bc9ab4 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2338,17 +2338,12 @@ int mysql_rm_table_no_locks(THD *thd, TABLE_LIST *tables, bool if_exists, { table_creation_was_logged= table->table->s->table_creation_was_logged; if (thd->drop_temporary_table(table->table, &is_trans, true)) - { - /* - This is a very unlikely scenaro as dropping a temporary table - should always work. Would be better if we tried to drop all - temporary tables before giving the error. - */ error= 1; - goto err; + else + { + table->table= 0; + temporary_table_was_dropped= 1; } - table->table= 0; - temporary_table_was_dropped= 1; } if ((drop_temporary && if_exists) || temporary_table_was_dropped) From 478591608eb02bd1b9d87a602a76a7fe3f67177b Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Thu, 25 Jun 2020 19:21:45 +0200 Subject: [PATCH 24/26] optimize ha_delete_table_force first try discovering engines, then the rest. otherwise every DROP TABLE non_existent; will do lots of i/o trying to remove .MYI/.MYD/.MAI/.MAD/.CSV/etc files this matches the old behavior where DROP TABLE always tried to discover the table before dropping. --- sql/handler.cc | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/sql/handler.cc b/sql/handler.cc index d601bbb8c87..ace58869145 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -4986,6 +4986,7 @@ struct st_force_drop_table_params const LEX_CSTRING *db; const LEX_CSTRING *alias; int error; + bool discovering; }; @@ -5001,14 +5002,17 @@ static my_bool delete_table_force(THD *thd, plugin_ref plugin, void *arg) handlerton *hton = plugin_hton(plugin); st_force_drop_table_params *param = (st_force_drop_table_params *)arg; - int error; - error= ha_delete_table(thd, hton, param->path, param->db, param->alias, 0); - if (error > 0 && !non_existing_table_error(error)) - param->error= error; - if (error == 0) + if (param->discovering == (hton->discover_table != NULL)) { - param->error= 0; - return TRUE; // Table was deleted + int error; + error= ha_delete_table(thd, hton, param->path, param->db, param->alias, 0); + if (error > 0 && !non_existing_table_error(error)) + param->error= error; + if (error == 0) + { + param->error= 0; + return TRUE; // Table was deleted + } } return FALSE; } @@ -5032,15 +5036,23 @@ int ha_delete_table_force(THD *thd, const char *path, const LEX_CSTRING *db, Table_exists_error_handler no_such_table_handler; DBUG_ENTER("ha_delete_table_force"); - param.path= path; - param.db= db; - param.alias= alias; - param.error= -1; // Table not found + param.path= path; + param.db= db; + param.alias= alias; + param.error= -1; // Table not found + param.discovering= true; thd->push_internal_handler(&no_such_table_handler); if (plugin_foreach(thd, delete_table_force, MYSQL_STORAGE_ENGINE_PLUGIN, ¶m)) param.error= 0; // Delete succeded + else + { + param.discovering= false; + if (plugin_foreach(thd, delete_table_force, MYSQL_STORAGE_ENGINE_PLUGIN, + ¶m)) + param.error= 0; // Delete succeded + } thd->pop_internal_handler(); DBUG_RETURN(param.error); } From 2d00e003b27936561021aa4fd800e8d338b5ac9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Sat, 4 Jul 2020 13:56:38 +0300 Subject: [PATCH 25/26] After-merge fixes for ASAN The merge commit 0fd89a1a89da73cec8e87e1f007637eaec51dcc0 of commit b6ec1e8bbf0ffca2d715aded694722e0c4b5d484 was slightly incomplete. ReadView::mem_valid(): Use the correct primitive MEM_MAKE_ADDRESSABLE(), because MEM_UNDEFINED() now has no effect on ASAN. recv_sys_t::alloc(), recv_sys_t::add(): Use MEM_MAKE_ADDRESSABLE() instead of MEM_UNDEFINED(), to get the correct behaviour for ASAN. For Valgrind and MSAN, there is no change in behaviour. recv_sys_t::free(), recv_sys_t::clear(): Before freeing memory to buf_pool.free_list, invoke MEM_MAKE_ADDRESSABLE() on the entire buf_block_t::frame, to cancel the effect of MEM_NOACCESS() in recv_sys_t::alloc(). --- storage/innobase/include/read0types.h | 15 +++++++++------ storage/innobase/include/ut0pool.h | 20 ++++++++++++++------ storage/innobase/log/log0recv.cc | 8 +++++--- storage/innobase/trx/trx0trx.cc | 13 +++++++++---- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/storage/innobase/include/read0types.h b/storage/innobase/include/read0types.h index 1428170e704..de3ff927520 100644 --- a/storage/innobase/include/read0types.h +++ b/storage/innobase/include/read0types.h @@ -279,17 +279,20 @@ public: /** - Unpoison the memory for innodb_monitor_set_option; - It is operating also on the freed transaction objects. - Declare the contents as initialized for Valgrind; - We checked that it was initialized in trx_pools->mem_free(trx). + Make the memory accessible by innodb_monitor_set_option; + It is operating also on freed transaction objects. */ void mem_valid() const { + /* Cancel the effect of MEM_NOACCESS(). */ #ifdef __SANITIZE_ADDRESS__ - MEM_UNDEFINED(&m_mutex, sizeof m_mutex); + MEM_MAKE_ADDRESSABLE(&m_mutex, sizeof m_mutex); #endif -#ifdef HAVE_valgrind +#if defined HAVE_valgrind && !__has_feature(memory_sanitizer) + /* In Valgrind, we cannot cancel MEM_NOACCESS() without changing + the state of the V bits (indicating which bits are initialized). + We will declare the contents as initialized. + We did invoke MEM_CHECK_DEFINED() in trx_pools->mem_free(). */ MEM_MAKE_DEFINED(&m_mutex, sizeof m_mutex); #endif } diff --git a/storage/innobase/include/ut0pool.h b/storage/innobase/include/ut0pool.h index 4a5f58f6fae..c31633e7f4c 100644 --- a/storage/innobase/include/ut0pool.h +++ b/storage/innobase/include/ut0pool.h @@ -92,9 +92,14 @@ struct Pool { MEM_MAKE_ADDRESSABLE(&elem->m_type, sizeof elem->m_type); #endif - /* Declare the contents initialized; - we checked this in mem_free(). */ +#if defined HAVE_valgrind && !__has_feature(memory_sanitizer) + /* In Valgrind, we cannot cancel MEM_NOACCESS() without + changing the state of the V bits (which indicate + which bits are initialized). + We will declare the contents as initialized. + We did invoke MEM_CHECK_DEFINED() in mem_free(). */ MEM_MAKE_DEFINED(&elem->m_type, sizeof elem->m_type); +#endif Factory::destroy(&elem->m_type); } @@ -136,11 +141,14 @@ struct Pool { MEM_MAKE_ADDRESSABLE(&elem->m_type, sizeof elem->m_type); # endif - /* Declare the memory initialized. - The trx_t that are released to the pool are - actually initialized; we checked that by - MEM_CHECK_DEFINED() in mem_free() below. */ +# if defined HAVE_valgrind && !__has_feature(memory_sanitizer) + /* In Valgrind, we cannot cancel MEM_NOACCESS() without + changing the state of the V bits (which indicate + which bits are initialized). + We will declare the contents as initialized. + We did invoke MEM_CHECK_DEFINED() in mem_free(). */ MEM_MAKE_DEFINED(&elem->m_type, sizeof elem->m_type); +# endif } #endif diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc index 2a4891f2716..7afac5c7d54 100644 --- a/storage/innobase/log/log0recv.cc +++ b/storage/innobase/log/log0recv.cc @@ -1050,6 +1050,7 @@ inline void recv_sys_t::clear() buf_block_t *prev_block= UT_LIST_GET_PREV(unzip_LRU, block); ut_ad(block->page.state() == BUF_BLOCK_MEMORY); UT_LIST_REMOVE(blocks, block); + MEM_MAKE_ADDRESSABLE(block->frame, srv_page_size); buf_block_free(block); block= prev_block; } @@ -1093,7 +1094,7 @@ create_block: ut_calc_align(static_cast(len), ALIGNMENT); static_assert(ut_is_2pow(ALIGNMENT), "ALIGNMENT must be a power of 2"); UT_LIST_ADD_FIRST(blocks, block); - MEM_UNDEFINED(block->frame, len); + MEM_MAKE_ADDRESSABLE(block->frame, len); MEM_NOACCESS(block->frame + len, srv_page_size - len); return my_assume_aligned(block->frame); } @@ -1113,7 +1114,7 @@ create_block: block->page.access_time= ((block->page.access_time >> 16) + 1) << 16 | ut_calc_align(static_cast(free_offset), ALIGNMENT); - MEM_UNDEFINED(block->frame + free_offset - len, len); + MEM_MAKE_ADDRESSABLE(block->frame + free_offset - len, len); return my_assume_aligned(block->frame + free_offset - len); } @@ -1148,6 +1149,7 @@ inline void recv_sys_t::free(const void *data) if (!((block->page.access_time -= 1U << 16) >> 16)) { UT_LIST_REMOVE(blocks, block); + MEM_MAKE_ADDRESSABLE(block->frame, srv_page_size); buf_block_free(block); } return; @@ -1758,7 +1760,7 @@ inline void recv_sys_t::add(const page_id_t page_id, { /* Use already allocated 'padding' bytes */ append: - MEM_UNDEFINED(end + 1, len); + MEM_MAKE_ADDRESSABLE(end + 1, len); /* Append to the preceding record for the page */ tail->append(l, len); return; diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index a8d3c2ed9b9..03e60d80d5b 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -455,14 +455,18 @@ void trx_free(trx_t*& trx) /* Unpoison the memory for innodb_monitor_set_option; it is operating also on the freed transaction objects. */ MEM_MAKE_ADDRESSABLE(&trx->mutex, sizeof trx->mutex); +# ifdef WITH_WSREP + MEM_MAKE_ADDRESSABLE(&trx->wsrep, sizeof trx->wsrep); +# endif /* For innobase_kill_connection() */ MEM_MAKE_ADDRESSABLE(&trx->state, sizeof trx->state); MEM_MAKE_ADDRESSABLE(&trx->mysql_thd, sizeof trx->mysql_thd); #endif - /* Unpoison the memory for innodb_monitor_set_option; - it is operating also on the freed transaction objects. - We checked that these were initialized in - trx_pools->mem_free(trx). */ +#if defined HAVE_valgrind && !__has_feature(memory_sanitizer) + /* In Valgrind, we cannot cancel the effect of MEM_NOACCESS() + without changing the state of the V bits (indicating which + bits are initialized). We did invoke MEM_CHECK_DEFINED() in + trx_pools->mem_free(). */ MEM_MAKE_DEFINED(&trx->mutex, sizeof trx->mutex); /* For innobase_kill_connection() */ # ifdef WITH_WSREP @@ -470,6 +474,7 @@ void trx_free(trx_t*& trx) # endif MEM_MAKE_DEFINED(&trx->state, sizeof trx->state); MEM_MAKE_DEFINED(&trx->mysql_thd, sizeof trx->mysql_thd); +#endif trx = NULL; } From a85f81af035f3c9420ccd536d249bd1e8ee429be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Sat, 4 Jul 2020 14:01:55 +0300 Subject: [PATCH 26/26] MDEV-22535 fixup: Define a single-caller function inline Let us avoid any overhead in release builds, for an empty function. --- sql/table.cc | 4 ++-- sql/table.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sql/table.cc b/sql/table.cc index 06178f8e789..32d0ee6f538 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -9922,7 +9922,7 @@ bool TABLE::export_structure(THD *thd, Row_definition_list *defs) return false; } -/* +/** @brief Initialize all the opt_range structures that are used to stored the estimates when the range optimizer is run. @@ -9931,7 +9931,7 @@ bool TABLE::export_structure(THD *thd, Row_definition_list *defs) to be able to find wrong usage of data with valgrind or MSAN. */ -void TABLE::initialize_opt_range_structures() +inline void TABLE::initialize_opt_range_structures() { TRASH_ALLOC((void*)&opt_range_keys, sizeof(opt_range_keys)); TRASH_ALLOC(opt_range, s->keys * sizeof(*opt_range)); diff --git a/sql/table.h b/sql/table.h index 94115c778bf..f719a0d03a2 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1637,7 +1637,8 @@ public: bool is_filled_at_execution(); bool update_const_key_parts(COND *conds); - void initialize_opt_range_structures(); + + inline void initialize_opt_range_structures(); my_ptrdiff_t default_values_offset() const { return (my_ptrdiff_t) (s->default_values - record[0]); }