Re: bk commit into 5.0 tree (mattiasj:1.2578) BUG#24159

[Mattias Jonsson]

Hi Ingo,

Thanks for the comments. See my replies below.

Regards
Mattias

Ingo Strüwing wrote:
> Hi Mattias,
> 
> mattiasj@stripped, 29.02.2008 11:22:
> ...
>> ChangeSet@stripped, 2008-02-29 11:21:57+01:00,
> mattiasj@stripped +4 -0
>>   Bug#24159: Altered AUTO_INCREMENT not effective when inserting in MERGE
>>   
>>   Problem was that it used the default handler::get_auto_increment,
>>   which gets next higher value, instead of using the auto_increment directly
>>   from myisam.
>>   
>>   Solved by adding ha_myisammrg::get_auto_increment and using the
>>   share->state.auto_increment variable (that is what myisam really does,
>>   and should be safe since there should be a table lock taken.)
> ...
>> +ulonglong ha_myisammrg::get_auto_increment()
>> +{
>> +  DBUG_ENTER("ha_myisammrg::get_auto_increment");
>> +
>> +  /* use default if secondary part of multi column index */
>> +  if (table->s->next_number_key_offset)
>> +    DBUG_RETURN(handler::get_auto_increment());
> 
> Have you run gcov tests? I wonder in which case this branch is taken? Or
> is the below not used? I ask because in the tests I see only MERGE
> tables with their own autoincrement value. So I suspect that either only
> the above branch is taken or only the below one.
> 

This happens for a table (create table t1 (a int not null, b int not
null auto_increment, primary key (a,b)) where the auto_increment column
is a secondary part of an index, and cannot be preallocated.

It is run through by row 226 "insert into t5 values (1,NULL),(5,NULL);"
in merge.test

(But you are right that I have not done a gcov test).

>> +
>> +  if (file->merge_insert_method == MERGE_INSERT_TO_FIRST)
>> +    file->current_table= file->open_tables;
>> +  else if (file->merge_insert_method == MERGE_INSERT_TO_LAST)
>> +    file->current_table= file->end_table-1;
>> +  else /* unsupported insertion method, will fail in myrg_write */
>> +    DBUG_RETURN(~(ulonglong) 0);
> 
> Again: gcov tested? I suspect that a MERGE table without INSERT_METHOD
> won't come here at all. IMHO a DBUG_ASSERT should be sufficient.
> 
OK, I'll change to:
DBUG_ASSERT(file->merge_insert_method == MERGE_INSERT_TO_LAST ||
file->merge_insert_method == MERGE_INSERT_TO_FIRST);


>> + 
> DBUG_RETURN(file->current_table->table->s->state.auto_increment+1);
>> +}
> ...
> 
> Regards
> Ingo

-- 
Mattias Jonsson, Software Engineer
MySQL AB, www.mysql.com

Are you MySQL certified?  www.mysql.com/certification