Sergei Golubchik wrote:
> Hi, Kristofer!
>
> On May 11, Kristofer Pettersson wrote:
>> Sergei Golubchik wrote:
>>>> I'm removing the comment + the two null assignments for now.
>>> please move them to construct_help_options and use strcmp("MyISAM" or
>>> "HEAP") as before.
>>> Then bug#44691 will only need to fix hard-coded "mandatory" list,
>>> which is what this bug is about. And your bugfix won't break old
>>> behavior too.
>> 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.
>> We can add a hard coded test as you suggest but it wouldn't change
>> anything and I still need to change it in the patch for 44691, right?
>
> It will preserve the old behavior (no --myisam and --memory) options.
> And no, for 44691 this zero-out code won't need to be changed, only
> hard-coded test will (I mean, only the test, but not the code executed
> when the test succeeds).
>
> 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).
:-)
