Re: bk commit into 5.1 tree (mattiasj:1.2568) BUG#30695

[Ingo Strüwing]

Hi Mattias,

mattiasj@stripped, 05.10.2007 10:10:
...
> ChangeSet@stripped, 2007-10-05 10:10:18+02:00, mattiasj@mattiasj-laptop.(none) +3 -0
>   Bug #30695: An apostrophe ' in the comment of the ADD PARTITION causes the Server
> to crash.

Please try to break long lines. For email review it would be most
comfortable to stick <= 72. I select my 'citool' window width so that I
don't get too long lines when breaking near the right border of the
comment window.

>   
>   I crashed because of the comment in the partition clause was saved in the frm file
> without escaping,

You crashed? ;-)

>   causing the server to crash when it was read/parsed again.
>   
>   Fixed by escaping the comment when writing it to the frm-file

Good changeset comment. The required comments are present and the level
of "technicality" is good (IMHO).

>   mysql-test/r/partition.result@stripped, 2007-10-05 10:10:15+02:00,
> mattiasj@mattiasj-laptop.(none) +17 -0
>     Added testresult for bug #30695: An apostrophe ' in the comment of the ADD
> PARTITION causes the Server to crash.
> 
>   mysql-test/t/partition.test@stripped, 2007-10-05 10:10:15+02:00,
> mattiasj@mattiasj-laptop.(none) +24 -0
>     Added test for bug #30695: An apostrophe ' in the comment of the ADD PARTITION
> causes the Server to crash.
> 
>   sql/sql_partition.cc@stripped, 2007-10-05 10:10:15+02:00,
> mattiasj@mattiasj-laptop.(none) +10 -1
>     Bug #30695: An apostrophe ' in the comment of the ADD PARTITION causes the Server
> to crash.
>       crashes when there is an non escaped apostrophe in the partition comment

IMHO it is not necessary to repeat in a file comment, what the problem
was, but only what you changed. Sometimes it happens that one has to
resolve merge conflicts from other developers patches. Then it is very
helpful to read what the intended changes of each patch are. The pure
code changes do not always tell the story. You may already know all this
as you have the relevant information included here:

>     fixed by escaping the comments before writing them to the frm-file

...
> +CREATE TABLE t1 (
...
> +COMMENT 'START \' åÀö
> ��� \\ 
> +\' END'

I am curious what a SHOW CREATE TABLE prints for this table. If you
don't mind, please add this to the test.

...

Otherwise OK to push from me.

Regards
Ingo
-- 
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Dachauer Str. 37, D-80335 München
Geschäftsführer: Kaj Arnö - HRB München 162140