Hi, Sachin! I agree with the patch, I think it's correct. Good tests too.
I wasn't able to understand the bug from the commit comment though, see below. Ok to push (preferrably with the improved commit message). On Jan 19, Sachin Setiya wrote: > Hi serg , please review the latest patch > > commit d9641478bb8f6045039a3d942f2925ada334907e > Author: Sachin Setiya <[email protected]> > Date: Fri Jan 19 19:33:45 2018 +0530 > > MDEV-14586 Assertion `0' failed in retrieve_auto_increment ... > > Problem:- > If we create table using myisam/aria then this crashes the server. > CREATE TABLE t1(a bit(1), b int auto_increment , index(a,b)); > insert into t1 values(1,1); > Or this query > CREATE TABLE t1 (b BIT(1), pk INTEGER AUTO_INCREMENT PRIMARY KEY); > ALTER TABLE t1 ADD INDEX(b,pk); > INSERT INTO t1 VALUES (1,b'1'); > ALTER TABLE t1 DROP PRIMARY KEY; > > Reason:- > The reason for this is > 1st- Since bit field is stored in record null bits itself , so > find_ref_key in open_binary_frm will wrongly set > share->next_number_key_offset > and share->next_number_keypart (both set to 0). I would say that find_ref_key() finds what key an auto_increment field belongs to by comparing key_part->offset and field->ptr. But BIT fields might have zero length in the record, so a key might have many key parts with the same offset. That is, comparing offsets cannot uniquely identify the correct key part. > 2nd- Since next_number_key_offset is zero it myisam/aria will think that > auto_increment is in first part of key. > 3nd- myisam/aria will call retrieve_auto_key which will see first > key_part > field as a bit field and call assert(0) > > Solution:- Many key parts might have the same offset, but BIT fields do not support auto_increment. So, we can skip all key parts over BIT fields, and then comparing offsets will be unambiguous. > Although the bit field is stored in record null_byte but in key > file(.MYI) > this does not hold true. Bit(1-7) will take one byte of memory. So the > correct > approach will be to never call retrieve_auto_increment in the case when > bit field is in the starting of index. So I have changed the find_ref_key > function to take account that if first N fields is Bit field then simply > go > to next field which is not Bit field. > Regards, Sergei Chief Architect MariaDB and [email protected] _______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
