#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
