List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:December 21 2010 10:48am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)
Bug#58918
View as plain text  
Hi Martin,

Thanks for following up on my suggestions.  Patch looks good.

With respect to the asserts in the clone method, I think I understand 
the pattern, but I wonder why you think it is necessary to check both 
type() and real_type() for Field_newdate and Field_varstring, but not 
Field_enum and Field_set.  I also think you should have an assert on 
type()==MYSQL_TYPE_BIT for Field_bit_as_char.

With respect to the following change:

 >
 > === modified file 'sql/table.cc'
 > --- a/sql/table.cc	2010-11-23 22:37:59 +0000
 > +++ b/sql/table.cc	2010-12-21 09:17:44 +0000
 > @@ -1876,8 +1876,13 @@ int open_table_from_share(THD *thd, TABL
 >     /* Setup copy of fields from share, but use the right alias and 
record */
 >     for (i=0 ; i<  share->fields; i++, field_ptr++)
 >     {
 > -    if (!((*field_ptr)= share->field[i]->clone(&outparam->mem_root, 
outparam)))
 > +    Field *new_field= share->field[i]->clone(&outparam->mem_root);
 > +    *field_ptr= new_field;
 > +    if (new_field == NULL)
 >         goto err;
 > +    new_field->init(outparam);
 > +    new_field->move_field_offset((my_ptrdiff_t) (outparam->record[0] -
 > + 
outparam->s->default_values));
 >     }
 >     (*field_ptr)= 0;                              // End marker
 >

I wonder why you think the new_field variable is needed, but that is no 
big deal.

Approved,

--
Øystein

PS!
Suggestion for a later refactoring: Drop the new_field methods, and move 
their initialization code to new reinit() methods.  Current uses of 
new_field can be replaced by clone() followed by reinit().
Thread
bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260) Bug#58918Martin Hansson21 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen21 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Roy Lyseng21 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen21 Dec