List:Commits« Previous MessageNext Message »
From:Ingo Strüwing Date:July 10 2007 7:15am
Subject:Re: bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158
View as plain text  
Hi Sergei,

thanks for the review. Answers/comments below.

Sergei Golubchik wrote:
> Hi!
> 
> On Jul 04, ingo@stripped wrote:
>> ChangeSet@stripped, 2007-07-04 15:32:31+02:00, istruewing@stripped +4 -0
>>   Bug#28158 - table->read_set is set incorrectly,
>>               causing wrong error message in Falcon

...
>> +  /* Copy the newly read columns into the new record. */
>> +  for (field_p= table->field; (field= *field_p); field_p++)
>> +    if (bitmap_is_set(&unique_map, field->field_index))
>> +      field->copy_from_tmp(table->s->rec_buff_length);
> 
> heh, good!
> I expected it to be more complex than that :)

Me too. But when digging through class Field for something to help with
field copying, I stumbled over this one. It looks quite appropriate for
this purpose. :-)

...
>> @@ -1752,8 +1838,11 @@ int multi_update::do_updates(bool from_s
>>  err:
>>    if (!from_send_error)
>>    {
>> +    /* purecov: begin inspected */
>>      thd->fatal_error();
>> +    prepare_record_for_error_message(local_error, table);
>>      table->file->print_error(local_error,MYF(0));
>> +    /* purecov: end */
> 
> Okay, I understood the rest, but why this code has purecov comment ?

I guessed that this branch is for exceptional cases which are unlikely
to be covered by the test suite. DGcov seconded that. Admittedly I
didn't spend much time to find a test which triggers this branch though.
I'll try again for some time, but don't want to burn too much time on it.

> 
>>    }
>> --- 1.10/mysql-test/t/ndb_update.test	2007-07-04 15:33:10 +02:00
>> +++ 1.11/mysql-test/t/ndb_update.test	2007-07-04 15:33:10 +02:00
> 
> Hmm, I didn't see a multi-update test here. But you fixed
> multi_update::send_data and it didn't have purecov comment, so I believe
> you tested that branch somehow. Why not to put the test in the test
> suite then ?

The test is in the test suite. I just don't know where. DGcov showed
that the branch was taken when running the whole suite. If you like,
I'll spend some time to find the "guilty" test and copy it to the other
test for this bug, though I'm not fully convinced that we need it twice
in the suite.

Regards
Ingo
-- 
Ingo Strüwing, Senior Software Developer
MySQL GmbH, Radlkoferstr. 2, D-81373 München
Geschäftsführer: Kaj Arnö - HRB München 162140
Thread
bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158ingo4 Jul
  • Re: bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158Sergei Golubchik9 Jul
    • Re: bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158Ingo Strüwing10 Jul
      • Re: bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158Sergei Golubchik10 Jul
        • Re: bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158Ingo Strüwing10 Jul
        • Re: bk commit into 6.0-falcon tree (istruewing:1.2567) BUG#28158Ingo Strüwing10 Jul