Hi, Jan! On Apr 26, Jan Lindström wrote:

revision-id: 607e22e1426ef3d7e11e3a2e9d7f884d36cf3573 (mariadb-10.0.24-43-g607e22e) parent(s): ce38adddfa91949b30956abd0e4cab201effcdef committer: Jan Lindström timestamp: 2016-04-26 13:47:30 +0300 message:

MDEV-8633: information_schema.index_statistics doesn't delete item when drop table indexes or drop table;

Problem was that table and index statistics is removed from persistent tables but not from memory cache. Added functions to remove table and index statistics from memory cache.

That's of course, ok. Just minor comments, see below:

diff --git a/sql/sql_show.cc b/sql/sql_show.cc index b8926b9..ac4503d 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -3442,6 +3442,100 @@ int fill_schema_table_stats(THD *thd, TABLE_LIST *tables, COND *cond) DBUG_RETURN(0); }

+/* Remove all indexes from global index statistics */

"all indexes for a given table"

+static +int del_global_index_stats(THD *thd, uchar* cache_key, uint cache_key_length) +{ + int res = 0; + DBUG_ENTER("del_global_index_stats"); + + mysql_mutex_lock(&LOCK_global_index_stats); + + for (uint i= 0; i < global_index_stats.records && !res;) + { + INDEX_STATS *index_stats = + (INDEX_STATS*) my_hash_element(&global_index_stats, i); + + /* We search correct db\0table_name\0 string */ + if (index_stats && + index_stats->index_name_length >= cache_key_length && + !memcmp(index_stats->index, cache_key, cache_key_length)) + { + res= my_hash_delete(&global_index_stats, (uchar*)index_stats); + /* + In our HASH implementation on deletion one elements + is moved into a place where a deleted element was, + and the last element is moved into the empty space. + Thus we need to re-examine the current element, but + we don't have to restart the search from the beginning. + */ + } + else + i++; + } + + mysql_mutex_unlock(&LOCK_global_index_stats); + DBUG_RETURN(res); +} + +/* Remove a table from global table statistics */ + +int del_global_table_stat(THD *thd, LEX_STRING *db, LEX_STRING *table) +{ + TABLE_STATS *table_stats; + int res = 0; + uchar *cache_key; + uint cache_key_length; + DBUG_ENTER("del_global_TABLE_stat");

correct letter case: "del_global_table_stat".

+ + cache_key_length= db->length + 1 + table->length + 1; + + if(!(cache_key= (uchar *)my_malloc(cache_key_length, + MYF(MY_WME | MY_ZEROFILL)))) + { + /* Out of memory error already given */ + res = 1; + goto end; + } + + memcpy(cache_key, db->str, db->length); + memcpy(cache_key + db->length + 1, table->str, table->length); + + res= del_global_index_stats(thd, cache_key, cache_key_length); + + mysql_mutex_lock(&LOCK_global_table_stats); + + if(!res && (table_stats= (TABLE_STATS*) my_hash_search(&global_table_stats,

here and below you, probably, don't want if (!res) - if the table is dropped, you should delete as many its entries as possible, even if something fails for an unknown reason.

+ cache_key, + cache_key_length))) + res= my_hash_delete(&global_table_stats, (uchar*)table_stats); + + my_free(cache_key); + mysql_mutex_unlock(&LOCK_global_table_stats); + +end: + DBUG_RETURN(res); +} + +/* Remove a index from global index statistics */ + +int del_global_index_stat(THD *thd, TABLE* table, KEY* key_info)

The difference is del_global_index_stat and del_global_index_stats is rather subtle. Could you rename them to make it clearer? For example, you can rename the static one to del_global_index_stats_for_table.

+{ + INDEX_STATS *index_stats; + uint key_length= table->s->table_cache_key.length + key_info->name_length + 1; + int res = 0; + DBUG_ENTER("del_global_index_stat"); + mysql_mutex_lock(&LOCK_global_index_stats); + + if((index_stats= (INDEX_STATS*) my_hash_search(&global_index_stats, + key_info->cache_name, + key_length))) + res= my_hash_delete(&global_index_stats, (uchar*)index_stats); + + mysql_mutex_unlock(&LOCK_global_index_stats); + DBUG_RETURN(res); +}

/* Fill information schema table with index statistics */

diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index e86c840..644ecf2 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -29,6 +29,7 @@ #include "sql_statistics.h" #include "opt_range.h" #include "my_atomic.h" +#include "sql_show.h"

/* The system variable 'use_stat_tables' can take one of the @@ -3193,6 +3194,9 @@ int delete_statistics_for_table(THD *thd, LEX_STRING *db, LEX_STRING *tab) rc= 1; }

+ if (!rc)

again here and below, better to do it without if (!rc)

+ rc= del_global_table_stat(thd, db, tab); + thd->restore_stmt_binlog_format(save_binlog_format);

close_system_tables(thd, &open_tables_backup); @@ -3339,6 +3343,9 @@ int delete_statistics_for_index(THD *thd, TABLE *tab, KEY *key_info, } }

+ if (!rc) + rc= del_global_index_stat(thd, tab, key_info); + thd->restore_stmt_binlog_format(save_binlog_format);

Regards, Sergei Chief Architect MariaDB and security@mariadb.org