MySQL Bugs: #26642: create index corrupts table definition in .frm

Bug #26642	create index corrupts table definition in .frm
Submitted:	26 Feb 2007 17:55	Modified:	3 Jul 2007 18:40
Reporter:	Martin Friebe (Gold Quality Contributor) (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server	Severity:	S3 (Non-critical)
Version:	5.0.38 4.1.23	OS:	Any (*)
Assigned to:	Georgi Kodinov	CPU Architecture:	Any
Tags:	corrupt table, INDEX, qc

Description:
Creating indexes with the maximum name length (64) and the maximum amount of key parts (16), leads to corruption of the table.

mysql will report an invalid column name '' (empty).

This is due to the amount of memory needed not beeing correctly calculated in sql/table.cc
(see patch)

In addition myisam reports an error if you try to add the maximum allowed indexes (64) and all of them have the maximum amount of keyparts.
(It will allow 64 indexes, if they dont have as many key-parts)
This seems due to mi_open.c comparing:
if (key_part_in_table >= MAX_KEY * MAX_KEY)SEG)
  error;

However, I think if they are equal they should not yet give an error. I have added this to the patch, too.
 

How to repeat:
see attaced tests

Suggested fix:
see patch

test case

Attachment: index_create.test (application/octet-stream, text), 12.74 KiB.

expected test result

Attachment: index_create.result (application/octet-stream, text), 34.48 KiB.

patch to fix the issue

Attachment: index_len_in_frm.patch (text/x-patch), 2.14 KiB.

Sorry, uploaded the test result twice.
(there are some issues with the bug system, I got an error from your website, about not connecting to the mysql server)

Verified as described on MySQL 5.0.38

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/22851

ChangeSet@1.2624, 2007-03-23 18:53:07-06:00, tsmith@siva.hindu.god +4 -0
  Bug #26642: create index corrupts table definition in .frm
  
  Problems solved:
  
  A table with maximum number of key segments and maximum length key name
  would have a corrupted .frm file, due to an incorrect calculation of the
  complete key length.  Now the key length is computed correctly (I hope) :-)
  
  MyISAM would reject a table with the maximum number of keys and the maximum
  number of key segments in all keys.  It would allow one less than this total
  maximum.  Now MyISAM accepts a table defined with the maximum.  (This is a
  very minor issue.)

Ooops, I forgot to say thanks to Martin in the changeset comment; I've since rectified that in my local repository.

Thanks, Martin!

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/23205

ChangeSet@1.2625, 2007-03-28 14:36:28-06:00, tsmith@siva.hindu.god +4 -0
  Bug #26642: create index corrupts table definition in .frm
  
  A table with maximum number of key segments and maximum length key name
  would have a corrupted .frm file, due to an incorrect calculation of the
  complete key length.  Now the key length is computed correctly (I hope) :-)
  
  MyISAM would reject a table with the maximum number of keys and the maximum
  number of key segments in all keys.  It would allow one less than this total
  maximum.  Now MyISAM accepts a table defined with the maximum.  (This is a
  very minor issue.)

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/23210

ChangeSet@1.2625, 2007-03-28 15:25:13-06:00, tsmith@siva.hindu.god +4 -0
  Bug #26642: create index corrupts table definition in .frm
  
  Thanks to Martin Friebe for finding and submitting a fix for this bug!
  
  A table with maximum number of key segments and maximum length key name
  would have a corrupted .frm file, due to an incorrect calculation of the
  complete key length.  Now the key length is computed correctly (I hope) :-)
  
  MyISAM would reject a table with the maximum number of keys and the maximum
  number of key segments in all keys.  It would allow one less than this total
  maximum.  Now MyISAM accepts a table defined with the maximum.  (This is a
  very minor issue.)Bug #26642: create index corrupts table definition in .frm
  
  Thanks to Martin Friebe for finding and submitting a fix for this bug!
  
  A table with maximum number of key segments and maximum length key name
  would have a corrupted .frm file, due to an incorrect calculation of the
  complete key length.  Now the key length is computed correctly (I hope) :-)
  
  MyISAM would reject a table with the maximum number of keys and the maximum
  number of key segments in all keys.  It would allow one less than this total
  maximum.  Now MyISAM accepts a table defined with the maximum.  (This is a
  very minor issue.)

I don't think the patch for this bug is upgrade safe. Remember that the mysqqld might read a file with too short keylength in it's frm and we should compensate for that when readnig the keylength back.

I looked through the code again. 

The trouble in table.cc "openfrm" is caused by teh fact that the last index-name is overwritten with 3 0xff chars. They go into fix_type_pointers and cause additional entries added to the array.

Because "names" (the column names) calculate there position in the same array by the amount of expected keys (and not the amount of unexpected key-names) the first name points to an empty name.

I will look into this, and a possible additional patch and come back on it.

enables the server to read a "broken" frm file

Attachment: read_invalid_frm.patch (text/x-patch), 2.70 KiB.

patch attached. There is no extra test. It can be tested with the supplied test case *before* applying the other patch. The test will work, since the broken frm file can now be read.

Another way to test it is to create a broken frm file, with a mysql server that has none of those 2 patches, and then open the table with a server that ispatched. It will open the table.

The patch only looks at index names. The function is also used by column names (there are overall 3 other calls). There is no known issue, that any of them can be broken deliberatly. Yet data can always get corrupted. Maybe it is desirable to extend it to the other calls (as long as the expected number of names is known).

Also it may be desirable to add a warning to the server logfile?

also the comment in the original patch (not resubmitting, since it is queued, but maybe the comment can (if wanted) still be amended?

+      For all keys:
+        6 bytes for the header
+        1 byte for the NAMES_SEP_CHAR (after the last name)
+        9 extra bytes (padding for safety? alignment?)

sould be
+      For all keys:
+        6 bytes for the header
+        1 byte for the NAMES_SEP_CHAR (after the last name)
         1 byte for the terminating 00 byte
+        8 extra bytes (padding for safety? alignment?)

downport to 4.1, includes all patches. 4.1 needed additional fixes in unireq.cc. Test can be taken from previous post

Attachment: index_len_41.patch (text/x-patch), 6.14 KiB.

Pushed into 4.1.23

Pushed into 5.0.40

Pushed into 5.1.18-beta

The fix for this was NOT pushed; the automated bugs system made a mistake with this bug.  It is still in progress.

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/29694

ChangeSet@1.2506, 2007-06-27 14:35:49+03:00, gkodinov@magare.gmz +4 -0
  Bug #26642: create index corrupts table definition in .frm
    
    Thanks to Martin Friebe for finding and submitting a fix for this bug!
    
    A table with maximum number of key segments and maximum length key name
    would have a corrupted .frm file, due to an incorrect calculation of the
    complete key length.  Now the key length is computed correctly (I hope) :-)
    
    MyISAM would reject a table with the maximum number of keys and the maximum
    number of key segments in all keys.  It would allow one less than this total
    maximum.  Now MyISAM accepts a table defined with the maximum.  (This is a
    very minor issue.)

Good, no upgrade problem apparently exists since unpatched mysqld would crash.

Approved the same some time ago already. ;-)

Pushed into 5.1.21-beta

Pushed into 5.0.46

Noted in 5.0.46, 5.1.21 changelogs.

Index creation could corrupt the table definition in the .frm file: 
1) A table with the maximum number of key segments and maximum length 
key name would have a corrupted .frm file, due to incorrect
calculation of the total key length. 2) MyISAM would reject a table
with the maximum number of keys and the maximum number of key
segments in all keys. (It would allow one less than this total
maximum.) Now MyISAM accepts a table defined with the maximum.

bug #29618 was marked as a duplicate of this one