Hi Anurag,
On Thu, Feb 26, 2009 at 10:10:10AM +0000, Anurag Shekhar wrote:
> #At file:///home/anurag/mysqlsrc/mysql-5.1-41305/ based on
> revid:bernt.johnsen@stripped
>
> 2824 Anurag Shekhar 2009-02-26
> Bug#41305 server crashes when inserting duplicate row into a merge table
>
> This problem comes while inserting a duplicate row in merge table
> without key but the child table has a primary key.
> While forming the error message handler tries to locate the key field
> which is creating this problem but as there is no key on the merge
> table there is a segmentation fault.
> modified:
> mysql-test/r/merge.result
> mysql-test/t/merge.test
> storage/myisammrg/ha_myisammrg.cc
>
> per-file messages:
> mysql-test/r/merge.result
> Updated results with new test for this bug.
> mysql-test/t/merge.test
> Added new test to test error generated from a key on child table where merge
> table doesn't have any key.
> storage/myisammrg/ha_myisammrg.cc
> Added a new check to see if the value of error key is higher than the number of
> key in merge table and if it is the error key set to MAX_KEY. The error message generation
> routine treats MAX_KEY as unknown key and doesn't tries to
> access this in key_info.
Please keep comment lines shorter.
> === modified file 'mysql-test/t/merge.test'
> --- a/mysql-test/t/merge.test 2009-02-12 10:25:12 +0000
> +++ b/mysql-test/t/merge.test 2009-02-26 10:10:00 +0000
> @@ -1496,4 +1496,27 @@ UNLOCK TABLES;
> --echo # drop the created tables
> DROP TABLE t1, t2, t3;
>
> +#
> +# Bug #41305 server crashes when inserting duplicate row into a merge table
> +#
> +--echo # insert duplicate value in child table while merge table doesn't have key
> +create table t1 (
> + col1 int(10),
> + primary key (col1)
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1;
> +
> +create table t2 (
> + col1 int(10),
> + primary key (col1)
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1;
Why do you need t2?
> +CREATE TABLE m1 (
> + col1 int(10) NOT NULL
> +) ENGINE=MRG_MyISAM DEFAULT CHARSET=latin1 INSERT_METHOD=LAST UNION=(t1,t2);
> +
> +insert into m1 (col1) values (1);
> +--error ER_DUP_ENTRY
> +insert into m1 (col1) values (1);
> +
> +drop table m1, t1, t2;
> --echo End of 5.1 tests
>
> === modified file 'storage/myisammrg/ha_myisammrg.cc'
> --- a/storage/myisammrg/ha_myisammrg.cc 2009-02-12 11:12:07 +0000
> +++ b/storage/myisammrg/ha_myisammrg.cc 2009-02-26 10:10:00 +0000
> @@ -872,6 +872,15 @@ int ha_myisammrg::info(uint flag)
> table->s->crashed= 1;
> #endif
> stats.data_file_length= mrg_info.data_file_length;
> + if (mrg_info.errkey >= table_share->keys) {
> + /*
> + If value of errkey is higher than the number of keys
> + on the table set errkey to MAX_KEY. This will be
> + treated as unknown key case and error message generator
> + won't try to locate key causing segmentation fault.
> + */
> + mrg_info.errkey= MAX_KEY;
> + }
Ok, but a few coding style concerns:
1) the identation is 2 characters, not 4
2) The identation inside the comment is also 2 characters, not 1
3) we put opening brace on the next line after if ():
if ()
{
...
}
Otherwise the patch looks pretty good.
Regards,
Sergey
--
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
