Re: bk commit into 5.1 tree (gluh:1.2578) BUG#29263

[Sergei Golubchik]

Hi!

On Sep 06, gluh@stripped wrote:
> ChangeSet@stripped, 2007-09-06 17:02:37+05:00, gluh@stripped +4 -0
>   Bug#29263 disabled storage engines omitted in SHOW ENGINES
>   added filling of 'engines' table with disabled builtin plugins
> 
> diff -Nrup a/sql/sql_show.cc b/sql/sql_show.cc
> --- a/sql/sql_show.cc	2007-08-25 13:43:15 +05:00
> +++ b/sql/sql_show.cc	2007-09-06 17:02:34 +05:00
> @@ -3830,10 +3830,13 @@ static my_bool iter_schema_engines(THD *
>                               strlen(plugin_decl(plugin)->descr), scs);
>        tmp= &yesno[test(hton->commit)];
>        table->field[3]->store(tmp->str, tmp->length, scs);
> +      table->field[3]->set_notnull();
>        tmp= &yesno[test(hton->prepare)];
>        table->field[4]->store(tmp->str, tmp->length, scs);
> +      table->field[4]->set_notnull();
>        tmp= &yesno[test(hton->savepoint_set)];
>        table->field[5]->store(tmp->str, tmp->length, scs);
> +      table->field[5]->set_notnull();

Mention in the changeset comment that disabled storage engines will have
NULL in the corresponding property fields in SHOW ENGINES.
 
>        if (schema_table_store_record(thd, table))
>          DBUG_RETURN(1);
> @@ -3845,8 +3848,24 @@ static my_bool iter_schema_engines(THD *
>  
>  int fill_schema_engines(THD *thd, TABLE_LIST *tables, COND *cond)
>  {
> -  return plugin_foreach(thd, iter_schema_engines,
> -                        MYSQL_STORAGE_ENGINE_PLUGIN, tables->table);
> +  CHARSET_INFO *scs= system_charset_info;
> +  TABLE *table= tables->table;
> +  struct st_mysql_plugin *plugin, **builtins;
> +  if (plugin_foreach(thd, iter_schema_engines,
> +                     MYSQL_STORAGE_ENGINE_PLUGIN, table))
> +    return 1;
> +
> +  for (builtins= mysqld_builtins_unused; *builtins; builtins++)
> +  {
> +    plugin= *builtins;
> +    restore_record(table, s->default_values);
> +    table->field[0]->store(plugin->name, strlen(plugin->name), scs);
> +    table->field[1]->store(C_STRING_WITH_LEN("NO"), scs);
> +    table->field[2]->store(plugin->descr, strlen(plugin->descr), scs);
> +    if (schema_table_store_record(thd, table))
> +      return 1;

Here you confuse "plugin" with a "storage engine". There could be other
plugins too - not storage engines. In this case your patch will print
too much in SHOW ENGINES and too little in SHOW PLUGINS.

Another problem - you only support disabled built-in plugins. What about
disabled dynamically loaded plugins ?

Perhaps a better solution would be to create st_plugin_int structure
also for disabled plugins and store them in the global plugin array and
hashes together with normal active plugins, but with a state
PLUGIN_IS_DISABLED ?

to conclude: the fix should cover both built-in and dynamic plugins,
and not be limited to storage engine plugins only.

> +  }
> +  return 0;
>  }

Can you create a test case for the bugfix ?
  
Regards / Mit vielen Grüssen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Developer
/_/  /_/\_, /___/\___\_\___/  MySQL GmbH, Dachauer Str. 37, D-80335 München
       <___/                  Geschäftsführer: Kaj Arnö - HRB
München 162140