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().
