MySQL Bugs: #45800: crash when replacing into a merge table and there is a duplicate

Bug #45800	crash when replacing into a merge table and there is a duplicate
Submitted:	27 Jun 2009 11:25	Modified:	22 Sep 2009 10:48
Reporter:	Shane Bester (Platinum Quality Contributor)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Merge storage engine	Severity:	S1 (Critical)
Version:	5.0.84, 5.1.30, 5.1.37	OS:	Any
Assigned to:		CPU Architecture:	Any
Tags:	crash, merge, REPLACE

Description:
Looks like the fix for bug #41305 was not complete.
Server crashes when replacing a duplicate value:

Stack trace from 5.1.37:

mysqld.exe!key_copy()[key.cc:115]
mysqld.exe!write_record()[sql_insert.cc:1398]
mysqld.exe!mysql_insert()[sql_insert.cc:812]
mysqld.exe!mysql_execute_command()[sql_parse.cc:3141]
mysqld.exe!mysql_parse()[sql_parse.cc:5933]
mysqld.exe!dispatch_command()[sql_parse.cc:1213]
mysqld.exe!do_command()[sql_parse.cc:854]
mysqld.exe!handle_one_connection()[sql_connect.cc:1127]
mysqld.exe!pthread_start()[my_winthread.c:85]
mysqld.exe!_callthreadstart()[thread.c:293]
mysqld.exe!_threadstart()[thread.c:277]
kernel32.dll!FlsSetValue()

5.0.84 is not affected

How to repeat:
flush tables;
drop table if exists `m1`,`t1`;
create table `t1`(`a` int primary key) engine=myisam;
create table `m1`(`a` int not null)engine=mrg_myisam 
insert_method=last union=(`t1`);
insert into `m1`  values (666);
replace into `m1` values (666);

same problem with insert..on duplicate key update..

flush tables;
drop table if exists `m1`,`t1`;
create table `t1`(`a` int primary key) engine=myisam;
create table `m1`(`a` int not null)engine=mrg_myisam 
insert_method=last union=(`t1`);
insert into `m1`  values (666);
insert into `m1` values (666) on duplicate key update a=a+1;

PROBLEM ANALYSIS
----------------

A REPLACE on a MERGE table works the following way,

Please note that the cause of the crash is step 2, I will
however explain the whole gamut.

1) Insert into the table (FIRST or LAST depending on CREATE syntax)

  =================================================================
  In the code
  -----------

  sql/sql_insert.cc:write_record(THD *thd, TABLE *table,COPY_INFO *info)

  while ((error=table->file->ha_write_row(table->record[0])))
  ==================================================================
    ha_write_row calls mi_write on the child myisam table

2) If the Insert fails because a duplicate value exists in the primary key column

  ==================================================================
  NOTE
  ----
  The MERGE table *has no* PRIMARY KEY. The PRIMARY KEY is present in the
  underlying child myisam table. Please remember that table->file->ha_write_row
  actually writes into a MYISAM table that has a PRIMARY KEY which throws
  a duplicate error.
  ===================================================================

  we try copying the key information from the current row we tried to insert.
  This is the key that would have caused the earlier duplicate error.

  key_copy((uchar*) key,table->record[0],table->key_info+key_nr,0);

  ====================================================================
  NOTE
  ----
  The table is a merge table that has no key information. Please remember
  there is no PRIMARY KEY on the merge table. There is a PRIMARY KEY on the
  underlying MYISAM table only. Hence, table->key_info is null.
    so when we try to do anything with table->key_info we go kaput!

  which is why the current bug is crashing at key_copy
  =====================================================================

3) We use the key information obtained from key_copy to try to position the
  index cursor and read the row at this place. This would be the row in the
  table that conflicted with our insert.

  if ((error=(table->file->index_read_idx_map(table->record[1],key_nr,
                                                   (uchar*) key, HA_WHOLE_KEY,
                                                   HA_READ_KEY_EXACT))))
         goto err;

  so table->record[1] would represent the row of interest to us.

4) Now with this row that we retrieved from the table we attempt an update.
    if ((error=table->file->ha_update_row(table->record[1],
                                               table->record[0])) &&
             error != HA_ERR_RECORD_IS_THE_SAME)

  Replace table->record[1] with table->record[0]

SUMMARY
-------

Although the TABLE structure contains the information for a MERGE table,
(the meta data pertaining to the underlying MERGE table) (the same reason
for which it does not contain the key information) the actual insert is
being done on a MyISAM table that has a PRIMARY KEY.

We expect the underlying engine to report a duplicate error to us,
(table->file->ha_write_row(table->record[0])) throws the actual duplicate
error. Please note that table->file represents a handler interface.

Our problem is the fact that table->file->ha_write_row writes to a
MyISAM table having a PRIMARY KEY and reports a duplicate error and
what we have in our hands is the meta data for the MERGE table that does
not have a PRIMARY KEY.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/79250

3039 V Narayanan	2009-07-24
      Bug#45800 crash when replacing into a merge table and there is a duplicate
      
      A REPLACE in the MERGE engine is actually a REPLACE
      into one (FIRST or LAST) of the underlying MyISAM
      tables. So in effect we have the meta data of the
      MERGE tables but we are inserting into a MyISAM table.
      
      When a REPLACE into a MERGE table (and the REPLACE
      conflicts with a duplicate in a child table) is done,
      we try to access the duplicate key information for
      the MERGE table. This information actually does not
      exist since it is the child tables that have keys
      and not the MERGE table. Hence this results in a crash.
      
      The problem can be resolved by modifying the MERGE
      engine to provide us the duplicate key information
      directly, instead of replying on the metadata on the
      server.
      
      The current patch modifies the MERGE engine to provide
      the duplicate key information to the server.
     @ include/myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Add a member to the st_mymerge_info structure that will
        store the duplicate key offset in the MERGE table. This
        offset will be the sum of the offset of the MyISAM table
        and the offset into the MERGE table.
     @ mysql-test/r/merge.result
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Result file for the test case.
     @ mysql-test/t/merge.test
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Added test case for both REPLACE and INSERT...ON DUPLICATE UPDATE.
     @ sql/handler.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        With the current change the MERGE engine will be able to
        return the duplicate key offset and will not use MAX_KEY
        as a error indicator. This adversely affects the case
        when we do a INSERT into a MERGE table that has a duplicate.
        
        An INSERT into a MERGE table having a duplicate causes the
        write into the storage engine to fail. When we print an error
        using print_keydup_error we access the key information from
        the server, causing a crash.
        
        This problem can be resolved by checking for the case
        when table->key_info is NULL.
     @ storage/myisammrg/ha_myisammrg.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        The info method now will process the HA_STATUS_ERRKEY flag
        and will return the index and the offset of the duplicate
        key.
        
        Since the storage engine returns the duplicate key information
        even if the key is on the child tables and not on the MERGE
        tables, we remove the code that sets the errkey to MAX_KEY.
     @ storage/myisammrg/ha_myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Set the HA_DUPLICATE_POS flag to indicate that the duplicate
        key information is now available in the MERGE storage engine.
     @ storage/myisammrg/myrg_info.c
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        We modify the myrg_status function to return the position of the
        duplicate key. The duplicate key position in the MERGE table will
        be the MyISAM file_offset and the offset within the MyISAM table
        where the key is located.

Good patch, but asking for more tests. Please see email.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/79321

3039 V Narayanan	2009-07-27
      Bug#45800 crash when replacing into a merge table and there is a duplicate
      
      A REPLACE in the MERGE engine is actually a REPLACE
      into one (FIRST or LAST) of the underlying MyISAM
      tables. So in effect the server works on the meta
      data of the MERGE table, while the real insert happens
      in the MyISAM table.
      
      The MERGE table has no index, while MyISAM has a
      unique index. When a REPLACE into a MERGE table (
      and the REPLACE conflicts with a duplicate in a
      child table) is done, we try to access the duplicate
      key information for the MERGE table. This information
      actually does not exist, hence this results in a crash.
      
      The problem can be resolved by modifying the MERGE
      engine to provide us the duplicate key information
      directly, instead of just returning the MyISAM index
      number as the error key. Then the SQL layer (or "the
      server") does not try to access the key_info of the
      MERGE table, which does not exist.
      
      The current patch modifies the MERGE engine to provide
      the duplicate key information to the server.
     @ include/myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Add a member to the st_mymerge_info structure that will
        store the duplicate key offset in the MERGE table. This
        offset will be the sum of the offset of the MyISAM table
        and the offset into the MERGE table.
     @ mysql-test/r/merge.result
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Result file for the test case.
     @ mysql-test/t/merge.test
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Added test case for both REPLACE and INSERT...ON DUPLICATE UPDATE.
     @ storage/myisammrg/ha_myisammrg.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        The info method now will process the HA_STATUS_ERRKEY flag
        and will return the index and the offset of the duplicate
        key.
     @ storage/myisammrg/ha_myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Set the HA_DUPLICATE_POS flag to indicate that the duplicate.
     @ storage/myisammrg/myrg_info.c
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        We modify the myrg_status function to return the position of the
        duplicate key. The duplicate key position in the MERGE table will
        be the MyISAM file_offset and the offset within the MyISAM table
        of the start position of the records.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/79382

3042 V Narayanan	2009-07-28
      Bug#45800 crash when replacing into a merge table and there is a duplicate
      
      A REPLACE in the MERGE engine is actually a REPLACE
      into one (FIRST or LAST) of the underlying MyISAM
      tables. So in effect the server works on the meta
      data of the MERGE table, while the real insert happens
      in the MyISAM table.
      
      The MERGE table has no index, while MyISAM has a
      unique index. When a REPLACE into a MERGE table (
      and the REPLACE conflicts with a duplicate in a
      child table) is done, we try to access the duplicate
      key information for the MERGE table. This information
      actually does not exist, hence this results in a crash.
      
      The problem can be resolved by modifying the MERGE
      engine to provide us the duplicate key information
      directly, instead of just returning the MyISAM index
      number as the error key. Then the SQL layer (or "the
      server") does not try to access the key_info of the
      MERGE table, which does not exist.
      
      The current patch modifies the MERGE engine to provide
      the duplicate key information to the server.
     @ include/myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Add a member to the st_mymerge_info structure that will
        store the duplicate key offset in the MERGE table. This
        offset will be the sum of the record offset of the MyISAM
        table within the MERGE table and the offset of the record
        within the MyISAM table.
     @ mysql-test/r/merge.result
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Result file for the test case.
     @ mysql-test/t/merge.test
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Added test case for both REPLACE and INSERT...ON DUPLICATE UPDATE.
     @ storage/myisammrg/ha_myisammrg.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        The info method now will process the HA_STATUS_ERRKEY flag
        and will return the index and the offset of the duplicate
        key.
     @ storage/myisammrg/ha_myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Set the HA_DUPLICATE_POS flag to indicate that the duplicate
        key information is now available in the MERGE storage engine.
     @ storage/myisammrg/myrg_info.c
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        We modify the myrg_status function to return the position of the
        duplicate key. The duplicate key position in the MERGE table will
        be the MyISAM file_offset and the offset within the MyISAM table
        of the start position of the records.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/79616

3048 V Narayanan	2009-07-30
      Bug#45800 crash when replacing into a merge table and there is a duplicate
            
      A REPLACE in the MERGE engine is actually a REPLACE
      into one (FIRST or LAST) of the underlying MyISAM
      tables. So in effect the server works on the meta
      data of the MERGE table, while the real insert happens
      in the MyISAM table.
      
      The MERGE table has no index, while MyISAM has a
      unique index. When a REPLACE into a MERGE table (
      and the REPLACE conflicts with a duplicate in a
      child table) is done, we try to access the duplicate
      key information for the MERGE table. This information
      actually does not exist, hence this results in a crash.
      
      The problem can be resolved by modifying the MERGE
      engine to provide us the duplicate key information
      directly, instead of just returning the MyISAM index
      number as the error key. Then the SQL layer (or "the
      server") does not try to access the key_info of the
      MERGE table, which does not exist.
      
      The current patch modifies the MERGE engine to provide
      the position for a record where a unique key violation
      occurs.
     @ include/myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Add a member to the st_mymerge_info structure that will
        store the duplicate key offset in the MERGE table. This
        offset will be the sum of the record offset of the MyISAM
        table within the MERGE table and the offset of the record
        within the MyISAM table.
     @ mysql-test/r/merge.result
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Result file for the test case.
     @ mysql-test/t/merge.test
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Added test case for both REPLACE and INSERT...ON DUPLICATE UPDATE.
     @ storage/myisammrg/ha_myisammrg.cc
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        The info method now will process the HA_STATUS_ERRKEY flag
        and will return the index and the offset of the duplicate
        key.
     @ storage/myisammrg/ha_myisammrg.h
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        Set the HA_DUPLICATE_POS flag to indicate that the duplicate
        key information is now available in the MERGE storage engine.
     @ storage/myisammrg/myrg_info.c
        Bug#45800 crash when replacing into a merge table and there is a duplicate
        
        We modify the myrg_status function to return the position of the
        duplicate key. The duplicate key position in the MERGE table will
        be the MyISAM file_offset and the offset within the MyISAM table
        of the start position of the records.

NOTE:

The fix for this bug introduced a minor functionality change,

The documentation here http://dev.mysql.com/doc/refman/5.1/en/merge-table-problems.html
says the following:

"If REPLACE detects unique key violation, it will only change the
corresponding row in the first underlying table in which the row
is present, whereas  a row with the same unique key value may be
present in all underlying tables."

With the present fix:

If REPLACE detects unique key violation, it will only change the
corresponding row in the underlying table it is writing to i.e.
the FIRST or LAST table.

Pushed into 5.4.4-alpha (revid:alik@sun.com-20090804194615-h40sa098mx4z49qg) (version source revid:kristofer.pettersson@sun.com-20090731070848-zd38c7u5xaofh8hl) (merge vers: 5.4.4-alpha) (pib:11)

Pushed into 5.1.38 (revid:davi.arnaut@sun.com-20090804204317-ggodqkik7de6nfpz) (version source revid:davi.arnaut@sun.com-20090804204317-ggodqkik7de6nfpz) (merge vers: 5.1.38) (pib:11)

The reference manual for 5.1 and 5.4 was updated.

An entry was added to the 5.1.38 and 5.4.4 changelogs:

The MySQL Server crashed when performing a REPLACE into a MERGE table if there was a duplicate.

Pushed into 5.1.39-ndb-6.3.28 (revid:jonas@mysql.com-20091001055605-ap2kiaarr7p40mmv) (version source revid:jonas@mysql.com-20091001055605-ap2kiaarr7p40mmv) (merge vers: 5.1.39-ndb-6.3.28) (pib:11)

Pushed into 5.1.39-ndb-7.0.9 (revid:jonas@mysql.com-20091001072547-kv17uu06hfjhgjay) (version source revid:jonas@mysql.com-20091001071652-irejtnumzbpsbgk2) (merge vers: 5.1.39-ndb-7.0.9) (pib:11)

Pushed into 5.1.39-ndb-7.1.0 (revid:jonas@mysql.com-20091001123013-g9ob2tsyctpw6zs0) (version source revid:jonas@mysql.com-20091001123013-g9ob2tsyctpw6zs0) (merge vers: 5.1.39-ndb-7.1.0) (pib:11)

Pushed into 5.1.39-ndb-6.2.19 (revid:jonas@mysql.com-20091005103850-dwij2dojwpvf5hi6) (version source revid:jonas@mysql.com-20090930185117-bhud4ek1y0hsj1nv) (merge vers: 5.1.39-ndb-6.2.19) (pib:11)

The 5.4 fix has been pushed to 5.4.2.