List:Commits« Previous MessageNext Message »
From:Guilhem Bichot Date:November 3 2010 1:54pm
Subject:bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330) Bug#57928
View as plain text  
#At file:///home/mysql_src/bzrrepos_new/mysql-next-mr-bugfixing2/ based on revid:guilhem@stripped

 3330 Guilhem Bichot	2010-11-03
      Fix for BUG#57928 "More Valgrind warnings in print_keyuse() with --debug":
      for a fulltext index, the KEYUSE object had some uninitialized members.
     @ include/my_sys.h
        insert_dynamic() and set_dynamic() treat "element" as bytes and uses it as
        the source of memcpy(). As memcpy() accepts void* it sounds natural to
        use void* as type for "element". It then avoids casts in calling code.
        Before the change, we had to write:
        insert_dynamic(array, (uchar*)keyuse);
        because conversion from KEYUSE* to uchar* requires an explicit cast.
        After the change we can write
        insert_dynamic(array, keyuse);
        because conversion from KEYUSE* to void* is always allowed.
        If reviewers like this change, I will commit it separately from the bugfix
        and also remove all now-unneeded casts in calls to insert_dynamic()
        and set_dynamic().
     @ mysys/array.c
        void* is more natural.
     @ sql/sql_select.cc
        use constructor which sets all members. The second change is the bugfix: we forgot
        to set ref_table_rows, null_rejecting and cond_guard. Now there is the question
        of: what value to pick for them, for a fulltext index?
        * ref_table_rows: we now set it to ~(ha_rows)0 which seems to be a default
        value (see optimize_keyuse() and add_key_part()).
        * null_rejecting: it's not used for a fulltext index, and it serves
        only for the so-called early-NULL-filtering optimization, so we play
        safe and set it to "false"; setting it to "true" would require
        intelligent code determining when the optimization is allowed.
        * cond_guard: it's not used for a fulltext index (see create_ref_for_key()),
        so we set it to NULL.
     @ sql/sql_select.h
        constructor, comments. With that,
        "KEYUSE keyuse;" will not compile anymore.

    modified:
      include/my_sys.h
      mysys/array.c
      sql/sql_select.cc
      sql/sql_select.h
=== modified file 'include/my_sys.h'
--- a/include/my_sys.h	2010-08-06 08:54:01 +0000
+++ b/include/my_sys.h	2010-11-03 13:54:05 +0000
@@ -778,10 +778,11 @@ extern my_bool init_dynamic_array2(DYNAM
 /* init_dynamic_array() function is deprecated */
 extern my_bool init_dynamic_array(DYNAMIC_ARRAY *array, uint element_size,
                                   uint init_alloc, uint alloc_increment);
-extern my_bool insert_dynamic(DYNAMIC_ARRAY *array,uchar * element);
+extern my_bool insert_dynamic(DYNAMIC_ARRAY *array, const void *element);
 extern uchar *alloc_dynamic(DYNAMIC_ARRAY *array);
 extern uchar *pop_dynamic(DYNAMIC_ARRAY*);
-extern my_bool set_dynamic(DYNAMIC_ARRAY *array,uchar * element,uint array_index);
+extern my_bool set_dynamic(DYNAMIC_ARRAY *array, const void *element,
+                           uint array_index);
 extern my_bool allocate_dynamic(DYNAMIC_ARRAY *array, uint max_elements);
 extern void get_dynamic(DYNAMIC_ARRAY *array,uchar * element,uint array_index);
 extern void delete_dynamic(DYNAMIC_ARRAY *array);

=== modified file 'mysys/array.c'
--- a/mysys/array.c	2010-07-08 21:20:08 +0000
+++ b/mysys/array.c	2010-11-03 13:54:05 +0000
@@ -92,7 +92,7 @@ my_bool init_dynamic_array(DYNAMIC_ARRAY
     FALSE	Ok
 */
 
-my_bool insert_dynamic(DYNAMIC_ARRAY *array, uchar* element)
+my_bool insert_dynamic(DYNAMIC_ARRAY *array, const void *element)
 {
   uchar* buffer;
   if (array->elements == array->max_element)
@@ -196,7 +196,7 @@ uchar *pop_dynamic(DYNAMIC_ARRAY *array)
     FALSE	Ok
 */
 
-my_bool set_dynamic(DYNAMIC_ARRAY *array, uchar* element, uint idx)
+my_bool set_dynamic(DYNAMIC_ARRAY *array, const void *element, uint idx)
 {
   if (idx >= array->elements)
   {

=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc	2010-10-21 13:30:19 +0000
+++ b/sql/sql_select.cc	2010-11-03 13:54:05 +0000
@@ -5861,20 +5861,15 @@ add_key_part(DYNAMIC_ARRAY *keyuse_array
       {
 	if (field->eq(form->key_info[key].key_part[part].field))
 	{
-          KEYUSE keyuse;
-          keyuse.table=          field->table;
-          keyuse.val=            key_field->val;
-          keyuse.used_tables=    key_field->val->used_tables();
-          keyuse.key=            key;
-          keyuse.keypart=        part;
-          keyuse.optimize=       key_field->optimize & KEY_OPTIMIZE_REF_OR_NULL;
-          keyuse.keypart_map=    (key_part_map) 1 << part;
-          /* This will be set accordingly in optimize_keyuse */
-          keyuse.ref_table_rows= ~(ha_rows) 0;
-          keyuse.null_rejecting= key_field->null_rejecting;
-          keyuse.cond_guard=     key_field->cond_guard;
-          keyuse.sj_pred_no=     key_field->sj_pred_no;
-          if (insert_dynamic(keyuse_array, (uchar*) &keyuse))
+          const KEYUSE keyuse(field->table, key_field->val,
+                              key_field->val->used_tables(), key, part,
+                              key_field->optimize & KEY_OPTIMIZE_REF_OR_NULL,
+                              (key_part_map) 1 << part,
+                              ~(ha_rows) 0, // will be set in optimize_keyuse
+                              key_field->null_rejecting,
+                              key_field->cond_guard,
+                              key_field->sj_pred_no);
+          if (insert_dynamic(keyuse_array, &keyuse))
             return TRUE;
 	}
       }
@@ -5938,16 +5933,11 @@ add_ft_keys(DYNAMIC_ARRAY *keyuse_array,
       !(usable_tables & cond_func->table->map))
     return FALSE;
 
-  KEYUSE keyuse;
-  keyuse.table= cond_func->table;
-  keyuse.val =  cond_func;
-  keyuse.key =  cond_func->key;
-  keyuse.keypart= FT_KEYPART;
-  keyuse.used_tables=cond_func->key_item()->used_tables();
-  keyuse.optimize= 0;
-  keyuse.keypart_map= 0;
-  keyuse.sj_pred_no= UINT_MAX;
-  return insert_dynamic(keyuse_array,(uchar*) &keyuse);
+  const KEYUSE keyuse(cond_func->table, cond_func,
+                      cond_func->key_item()->used_tables(), cond_func->key,
+                      FT_KEYPART, 0, 0, ~(ha_rows) 0, false, NULL,
+                      UINT_MAX);
+  return insert_dynamic(keyuse_array, &keyuse);
 }
 
 
@@ -6184,17 +6174,17 @@ update_ref_and_keys(THD *thd, DYNAMIC_AR
   */
   if (keyuse->elements)
   {
-    KEYUSE key_end,*prev,*save_pos,*use;
+    KEYUSE *save_pos,*use;
 
     my_qsort(keyuse->buffer,keyuse->elements,sizeof(KEYUSE),
 	  (qsort_cmp) sort_keyuse);
 
-    bzero((char*) &key_end,sizeof(key_end));    /* Add for easy testing */
-    if (insert_dynamic(keyuse,(uchar*) &key_end))
+    const KEYUSE key_end(NULL, NULL, 0, 0, 0, 0, 0, 0, false, NULL, 0);
+    if (insert_dynamic(keyuse, &key_end)) // added for easy testing
       return TRUE;
 
     use=save_pos=dynamic_element(keyuse,0,KEYUSE*);
-    prev= &key_end;
+    const KEYUSE *prev= &key_end;
     found_eq_constant=0;
     for (i=0 ; i < keyuse->elements-1 ; i++,use++)
     {

=== modified file 'sql/sql_select.h'
--- a/sql/sql_select.h	2010-09-23 12:16:36 +0000
+++ b/sql/sql_select.h	2010-11-03 13:54:05 +0000
@@ -39,17 +39,34 @@
 #define KEY_OPTIMIZE_EXISTS		1
 #define KEY_OPTIMIZE_REF_OR_NULL	2
 
-typedef struct keyuse_t {
+/**
+  @note such objects are stored in DYNAMIC_ARRAY which uses sizeof(), so keep
+  this class as POD as possible.
+*/
+class KEYUSE {
+public:
+  KEYUSE(TABLE *table_arg, Item *val_arg, table_map used_tables_arg,
+         uint key_arg, uint keypart_arg, uint optimize_arg,
+         key_part_map keypart_map_arg, ha_rows ref_table_rows_arg,
+         bool null_rejecting_arg, bool *cond_guard_arg,
+         uint sj_pred_no_arg) :
+  table(table_arg), val(val_arg), used_tables(used_tables_arg), key(key_arg),
+  keypart(keypart_arg), optimize(optimize_arg), keypart_map(keypart_map_arg),
+  ref_table_rows(ref_table_rows_arg), null_rejecting(null_rejecting_arg),
+  cond_guard(cond_guard_arg), sj_pred_no(sj_pred_no_arg) {}
   TABLE *table;
   Item	*val;				/**< or value if no field */
   table_map used_tables;
   uint	key, keypart;
   uint optimize; // 0, or KEY_OPTIMIZE_*
   key_part_map keypart_map;
+  /** Estimate of how many rows for a key value */
   ha_rows      ref_table_rows;
   /**
     If true, the comparison this value was created from will not be
     satisfied if val has NULL 'value'.
+    Not used if the index is fulltext (such index cannot be used for
+    equalities).
   */
   bool null_rejecting;
   /*
@@ -61,14 +78,20 @@ typedef struct keyuse_t {
               *cond_guard == FALSE <=> equality condition is off
 
     NULL  - Otherwise (the source equality can't be turned off)
+
+    Not used if the index is fulltext (such index cannot be used for
+    equalities).
   */
   bool *cond_guard;
   /*
      0..64    <=> This was created from semi-join IN-equality # sj_pred_no.
      MAX_UINT  Otherwise
+
+     Not used if the index is fulltext (such index cannot be used for
+     semijoin).
   */
   uint         sj_pred_no;
-} KEYUSE;
+};
 
 class store_key;
 


Attachment: [text/bzr-bundle] bzr/guilhem@mysql.com-20101103135405-bopuovkpsfb2heyu.bundle
Thread
bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330) Bug#57928Guilhem Bichot3 Nov
Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330) Bug#57928Tor Didriksen4 Nov
  • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330) Bug#57928Tor Didriksen4 Nov
  • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330)Bug#57928Guilhem Bichot4 Nov
    • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330) Bug#57928Tor Didriksen5 Nov
      • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330)Bug#57928Guilhem Bichot5 Nov
Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330)Bug#57928Jorgen Loland4 Nov
  • Re: bzr commit into mysql-next-mr-bugfixing branch (guilhem:3330)Bug#57928Guilhem Bichot4 Nov