List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:December 20 2010 8:51am
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)
Bug#58918
View as plain text  
Øystein Grøvlen skrev 2010-12-17 13.28:
> Hi Martin,
>
> Thanks for a good initiative.  I have bitten by this memcpy stuff 
> myself. I wonder whether I should require or just suggest that you try 
> to add a unit test for your clone methods.  Unit tests for basic 
> classes like Field should be possible without too much work.  (Tor 
> told me it was pretty straight-forward for Item_int).
It could of course be interesting to get into the unit testing 
framework, I haven't had time for that in the past. But I am not sure 
what I should be testing. Any suggestions? The only thing that needs to 
be tested IMHO is that every clone method returns an object of precisely 
the class for which is it's called and not a descendant class. But to 
test that requires RTTI - which we don't have - doesn't it?

Should I just instantiate every Field subclass and call clone on it?
>
> I also think that we should drop the existing clone method:
>
> >     Field *clone(MEM_ROOT *mem_root, TABLE *new_table);
>
> This is used in only one place, and its implementation uses only 
> public Field methods, so I think the entire code should be moved to 
> the caller.
> (The Field interface is unecessarily large. No need to provide 
> unecessary helper functions.)
Sure, I'll do that. I didn't look into this at all. Attached please find 
a patch for only this. Please scrutinize it.

Best Regards

Martin

=== modified file 'sql/field.cc'
--- sql/field.cc	2010-12-16 13:49:35 +0000
+++ sql/field.cc	2010-12-20 08:38:33 +0000
@@ -1845,21 +1845,6 @@
 }
 
 
-/* This is used to generate a field in TABLE from TABLE_SHARE */
-
-Field *Field::clone(MEM_ROOT *root, TABLE *new_table)
-{
-  Field *tmp= clone(root);
-  if (tmp != NULL)
-  {
-    tmp->init(new_table);
-    tmp->move_field_offset((my_ptrdiff_t) (new_table->record[0] -
-                                           new_table->s->default_values));
-  }
-  return tmp;
-}
-
-
 /****************************************************************************
   Field_null, a field that always return NULL
 ****************************************************************************/

=== modified file 'sql/field.h'
--- sql/field.h	2010-12-16 13:49:35 +0000
+++ sql/field.h	2010-12-20 08:35:23 +0000
@@ -336,7 +336,6 @@
   virtual Field *new_key_field(MEM_ROOT *root, TABLE *new_table,
                                uchar *new_ptr, uchar *new_null_ptr,
                                uint new_null_bit);
-  Field *clone(MEM_ROOT *mem_root, TABLE *new_table);
 
   /**
      Makes a shallow copy of the Field object.

=== modified file 'sql/table.cc'
--- sql/table.cc	2010-11-23 22:37:59 +0000
+++ sql/table.cc	2010-12-20 08:42:30 +0000
@@ -1876,8 +1876,13 @@
   /* 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
 


Thread
bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260) Bug#58918Martin Hansson14 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Tor Didriksen15 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson15 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Tor Didriksen16 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen16 Dec
      • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson16 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Tor Didriksen16 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson16 Dec
Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen17 Dec
  • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Martin Hansson20 Dec
    • Re: bzr commit into mysql-trunk-bugfixing branch (martin.hansson:3260)Bug#58918Øystein Grøvlen20 Dec