List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:September 23 2010 9:36am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3250)
Bug#56936
View as plain text  
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
Thread
bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3250) Bug#56936Tor Didriksen23 Sep
  • Re: bzr commit into mysql-next-mr-bugfixing branch (tor.didriksen:3250)Bug#56936Øystein Grøvlen23 Sep
    • Re: bzr commit into mysql-next-mr-bugfixing branch(tor.didriksen:3250) Bug#56936Tor Didriksen23 Sep