Re: bk commit into 5.0 tree (gshchepa:1.2504) BUG#30126

[Gleb Shchepa]

Hello Georgi,

Georgi Kodinov wrote:
> Hi Gleb,
> 
> The patch looks ok to push. Few minor questions/suggestions below.
> 
> On 25.08.2007, at 20:26, <gshchepa@stripped> <gshchepa@stripped>
> wrote:
> 
>> ChangeSet@stripped, 2007-08-25 22:26:08+05:00, gshchepa@stripped +5 -0
>>   Fixed bug #30126.
> 
> <cut>
> 
>>   mysql-test/t/mysqldump-old.opt@stripped, 2007-08-25 22:23:27+05:00, 
>> gshchepa@stripped +1 -0
>>     Added testcase for bug #30126.
>>     The 4x_server_emul option is to emulate a parse error as an answer
>>     to the "SHOW CREATE DATABASE" query.
>>     The mysqldump client uses that negative answer to distinguish
>>     old server and to produce the "CREATE DATABASE" statement
>>     from scratch - required for testing of #30126.
> 
> Adding this option is OK (and would test the code) on a debug server
> (where DBUG_EXECUTE_IF would work). It will not go through that code if 
> the server is non-debugged.
> This at least must be stated in the commit comment IMHO. I would also 
> add a comment why the DBUG_EXECUTE_IF is there in sql_parse.cc.


Ok, will add an additional commentary.


> I wonder if there's any central repository of DBUG_EXECUTE_IF() macros 
> (so they don't overlap).


No AFAIK.


> Maybe they should be documented on Internals ?


May be, but I need assistance of more experienced person to do that.


> 
>>
>>   mysql-test/t/mysqldump-old.opt@stripped, 2007-08-25 22:23:27+05:00, 
>> gshchepa@stripped +0 -0
>>
>>   mysql-test/t/mysqldump-old.test@stripped, 2007-08-25 22:23:27+05:00, 
>> gshchepa@stripped +13 -0
>>     Added testcase for bug #30126.
>>
>>   mysql-test/t/mysqldump-old.test@stripped, 2007-08-25 22:23:27+05:00, 
>> gshchepa@stripped +0 -0
> 
> I don't think -old is a good name : what about -compat ?


Ok, will change.


> 
>>
>>   sql/sql_parse.cc@stripped, 2007-08-25 22:23:33+05:00, gshchepa@stripped 
>> +2 -0
>>     Fixed bug #30126.
>>     The mysqldump client uses the "SHOW CREATE DATABASE" query to
>>     obtain the "CREATE DATABASE" statement from that database.
>>     The 4.x server doesn't recognise that query, and mysqldump
>>     forms the "CREATE DATABASE" statement from scratch.
>>     That statement was formed incorrectly.
>>
>>     To enforce the mysqldump client to create that statement from
>>     scratch, debugging code has been added to the mysql_execute_command
>>     function: in tcase of the --loose-debug=d,4x_server_emul option,
> 
> Why loose-debug ? I couldn't find any mention of --loose-debug anywhere 
> in the docs.
> Can you see that this gets documented please ? (such a handy debug 
> feature and I haven't noticed it until now because of lack of docs).


======================================================================
4.3.1. Using Options on the Command Line

If an option is prefixed by `--loose', a program does not exit with an
error if it does not recognize the option, but instead issues only a
warning:

      shell> mysql --loose-no-such-option
      mysql: WARNING: unknown option '--no-such-option'

The `--loose' prefix can be useful when you run programs from multiple
installations of MySQL on the same machine and list options in an
option file, An option that may not be recognized by all versions of a
program can be given using the `--loose' prefix (or `loose' in an
option file). Versions of the program that recognize the option process
it normally, and versions that do not recognize it issue a warning and
ignore it.
======================================================================

As I see there is a common practice to use --loose prefix instead of
simple --debug option.

Thank you,
Gleb.