Thanks for the fix, Tor. I appreciate the use of new and the
introduction of a constructor. Makes things safer.
I think you could remove some ad-hoc initialization that is no longer
needed. There is some initialization in setup_sj_materialization that
should be unecessary now. Also, create_ref_for_key() contains
initialization that could probably be removed now.
I also think that the default for key_err should be TRUE. Ref.
JOIN::reinit().
--
Øystein
On 09/23/10 08:39 AM, Tor Didriksen wrote:
> #At file:///export/home/didrik/repo/next-mr-opt-backporting-bug56936/ based on
> revid:tor.didriksen@stripped
>
> 3250 Tor Didriksen 2010-09-23
> Bug #56936 valgrind warnings, TABLE_REF objects are not properly initialized
>
> Use placement new, and constructor, rather than thd->alloc() and ad-hoc
> initialization.
> @ sql/sql_select.cc
> Use placement new.
> Fix some missing DBUG_RETURN(TRUE)
> @ sql/sql_select.h
> st_table_ref: inherit from Sql_alloc, so we can do placement new.
> Add constructor.
>
> modified:
> sql/sql_select.cc
> sql/sql_select.h
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-09-22 11:37:38 +0000
> +++ b/sql/sql_select.cc 2010-09-23 06:39:52 +0000
> @@ -10680,7 +10680,7 @@ bool setup_sj_materialization(JOIN_TAB *
> temptable.
> */
> TABLE_REF *tab_ref;
> - if (!(tab_ref= (TABLE_REF*) thd->alloc(sizeof(TABLE_REF))))
> + if (!(tab_ref= new (thd->mem_root) TABLE_REF))
> DBUG_RETURN(TRUE); /* purecov: inspected */
> tab_ref->key= 0; /* The only temp table index. */
> tab_ref->key_length= tmp_key->key_length;
> @@ -10887,7 +10887,7 @@ make_join_readinfo(JOIN *join, ulonglong
> {
> if (!(tab->loosescan_buf= (uchar*)join->thd->alloc(tab->
> loosescan_key_len)))
> - return TRUE; /* purecov: inspected */
> + DBUG_RETURN(TRUE); /* purecov: inspected */
> }
> if (sj_is_materialize_strategy(join->best_positions[i].sj_strategy))
> {
> @@ -10900,7 +10900,7 @@ make_join_readinfo(JOIN *join, ulonglong
> tab[-1].next_select= sub_select_sjm;
>
> if (setup_sj_materialization(tab))
> - return TRUE;
> + DBUG_RETURN(TRUE);
> }
> switch (tab->type) {
> case JT_EQ_REF:
>
> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h 2010-09-22 11:37:38 +0000
> +++ b/sql/sql_select.h 2010-09-23 06:39:52 +0000
> @@ -72,7 +72,7 @@ typedef struct keyuse_t {
>
> class store_key;
>
> -typedef struct st_table_ref
> +typedef struct st_table_ref : public Sql_alloc
> {
> bool key_err;
> /** True if something was read into buffer in join_read_key. */
> @@ -116,6 +116,25 @@ typedef struct st_table_ref
> */
> bool disable_cache;
>
> + st_table_ref()
> + : key_err(FALSE),
> + has_record(FALSE),
> + key_parts(0),
> + key_length(0),
> + key(0),
> + key_buff(NULL),
> + key_buff2(NULL),
> + key_copy(NULL),
> + items(NULL),
> + cond_guards(NULL),
> + null_rejecting(0),
> + depend_map(0),
> + null_ref_key(NULL),
> + use_count(0),
> + disable_cache(FALSE)
> + {
> + }
> +
> /**
> @returns whether the reference contains NULL values which could never give
> a match.
>
>
>
>
>
--
Øystein Grøvlen, Senior Staff Engineer
Sun Microsystems, Database Group
Trondheim, Norway
