Hi, Kristofer!
On May 11, Kristofer Pettersson wrote:
>>> I would agree on not breaking old behaviour, but what _is_ the old
>>> behaviour? I don't see any --myisam or --memory/--heap options if I
>>> do "mysqld --verbose --help", do you?
>> No, I don't. But I'm afraid that after your patch I will see them,
>> unless you will zero out comment field in construct_help_options().
>
> No, you wont. As you recall we had this conversation on irc a few days
> ago, and we concluded that although the myisam and memory plugins are
> being iterated, their corresponding options are not visible.
>
> If I run my patched version "mysqld --verbose --help | grep -i
> disable" there won't be any references to either the memory or myisam
> storage engine.
Right. It's a separate bug. In my_print_help_inc_plugins() we have
for (uint idx= 0; idx < plugin_array.elements; idx++)
{
p= *dynamic_element(&plugin_array, idx, struct st_plugin_int **);
if (!p->plugin->system_vars ||
!(opt= construct_help_options(&mem_root, p)))
continue;
that is if plugin XXX provides no system_vars of its own there will be
no standard --XXX and --plugin-XXX options either.
Okay, until it's fixed this comment=0 thing is a non-issue.
>> Another comment to your patch: you've changed the option comment from
>> (innodb, for example)
>> --innodb Enable InnoDB plugin. Disable with --skip-innodb
>> (will save memory)
>> to
>> --innodb Enable or disable InnoDB plugin. Disable with --skip-innodb
>> (will save memory)
>> I think the old version is more clear. Indeed, --innodb will "enable"
>> the plugin, not "enable or disable" it.
>
> The new help text is actually:
> --innodb[=#] Enable or disable InnoDB plugin. Disable with --skip-innodb
> (will save memory)
>
> I still think it is more accurate since you can disable innodb by setting
> --innodb=OFF. As you can see, the symbol for optional parameters ([=#]) is
> shown too.
>
> Actually I've taken the liberty to even go a step further so the current
> patch makes it look like this:
> --innodb[=#] Enable or disable InnoDB plugin. Disable with
> --skip-innodb or --innodb=OFF (will save memory).
Makes sense. I've decided not to overload my creativity and asked on
#docs about the help text :)
Paul said he prefers something like
--innodb[=#] Enable or disable InnoDB plugin. Possible values
are ON, OFF, FORCE (don't start if plugin fails to load).
Another thing, I've got compilation failure after I applied your patch:
sql_plugin.cc:3264: error: cast from 'uchar*' to 'uint' loses precision
The fix:
- plugin_load_policy= (plugin_load_policy_t)(uint)*opts[0].value;
+ plugin_load_policy= (plugin_load_policy_t)*(int*)opts[0].value;
Regards / Mit vielen GrЭъen,
Sergei
--
__ ___ ___ ____ __
/ |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@stripped>
/ /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect
/_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB MЭnchen 161028
<___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten
GeschДftsfЭhrer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel
Vorsitzender des Aufsichtsrates: Martin HДring
