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

[Ingo Strüwing]

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.

> +
> +  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.

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

Regards
Ingo
-- 
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Dachauer Str. 37, D-80335 München
Geschäftsführer: Kaj Arnö - HRB München 162140