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.
I wonder if there's any central repository of DBUG_EXECUTE_IF()
macros (so they don't overlap).
Maybe they should be documented on Internals ?
>
> 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 ?
>
> 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).
Best Regards,
Joro
--
Georgi Kodinov, Senior Software Engineer
MySQL AB, Plovdiv, Bulgaria, www.mysql.com
Office: +359 32 634 397 Mobile: +359 887 700 566 Skype: georgekodinov
Are you MySQL certified? www.mysql.com/certification
