From: Roy Lyseng
Date: December 21 2010 11:16am
Subject: Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)
 Bug#58918
List-Archive: http://lists.mysql.com/commits/127390
Message-Id: <4D108C90.1070908@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Hi,

about this bug: I think that you should consider making it a Worklog instead. 
Triage has a problem handling them as it is a) not a bug that we are fixing, and 
b) not a feature that is introduced.

A Woklog may seem like overkill, but we can use only the parts of the WL system 
that matters for this case.

Thanks,
Roy

On 21.12.10 11.48, Øystein Grøvlen wrote:
> 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().
>