On 12/21/10 12:16 PM, Roy Lyseng wrote:
> 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.
Why does triage need to be able to handle it? Will it stop us from
pushing it, if it is not triaged?
--
Øystein
>
> 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().
>>
>
>
--
Øystein Grøvlen, Principal Software Engineer
MySQL Group, Oracle
Trondheim, Norway
