Re: bzr commit into mysql-5.0 branch (kgeorge:2672) Bug#32124

[Sergei Golubchik]

Hi!

On Sep 12, Georgi Kodinov wrote:
> #At file:///home/kgeorge/mysql/bzr/B32124-5.0-bugteam/
> 
>  2672 Georgi Kodinov	2008-09-12
>       Bug #32124: crash if prepared statements refer to variables in the where
> clause
>                   
>       The code to get read the value of a system variable was extracting its value 
>       on PREPARE stage and was substituting the value (as a constant) into the parse
> tree.
>       Note that this must be a reversible transformation, i.e. it must be reversed
> before
>       each re-execution.
>       Unfortunately this cannot be reliably done using the current code, because
> there are
>       other non-reversible source tree transformations that can interfere with this
>       reversible transformation.
>       Fixed by not resolving the value at PREPARE, but at EXECUTE (as the rest of the
> 
>       functions operate). Added a cache of the value (so that it's constant
> throughout
>       the execution of the query). Note that the cache also caches NULL values.
>       Updated an obsolete related test suite (variables-big) and the code to test the
> 
>       result type of system variables (as per bug 74).

Thanks!
ok to push!

....
> +longlong Item_func_get_system_var::val_int()
> +{
> +  THD *thd= current_thd;
> +
> +  if (cache_present && thd->query_id == used_query_id)
> +  {
> +    null_value= cached_null_value;
> +    return cached_llval;
> +  }
> +
> +  switch (var->show_type())
> +  {
> +    case SHOW_INT:      get_sys_var_safe (uint);
> +    case SHOW_LONG:     get_sys_var_safe (ulong);
> +    case SHOW_LONGLONG: get_sys_var_safe (longlong);
> +    case SHOW_HA_ROWS:  get_sys_var_safe (ha_rows);
> +    case SHOW_MY_BOOL:  get_sys_var_safe (my_bool);
> +    case SHOW_CHAR:
> +      {
> +        longlong val;
> +        char *cptr;
> +
> +        pthread_mutex_lock(&LOCK_global_system_variables);
> +        cptr= (char*) var->value_ptr(thd, var_type, &component);
> +        if (cptr)
> +          val= longlong_from_string_with_check (system_charset_info, 
> +                                                cptr, cptr + strlen (cptr));
> +        else
> +        {
> +          null_value= TRUE;
> +          val= 0;
> +        }
> +        pthread_mutex_unlock(&LOCK_global_system_variables);
> +        cache_present= true;
> +        used_query_id= thd->query_id;
> +        cached_null_value= null_value;
> +        cached_llval= val;
> +        return val;
> +      }
> +
> +    default:            
> +      my_error(ER_VAR_CANT_BE_READ, MYF(0), name); 
> +      return 0;                               // keep the compiler happy
> +  }
> +}

Okay, you apparently don't want to cache NULL in case of an error, as
you try hard to avoid it :)

For me either behaviour is okay (both caching and not caching), let it
be your way.

Regards / Mit vielen Grüßen,
Sergei

-- 
   __  ___     ___ ____  __
  /  |/  /_ __/ __/ __ \/ /   Sergei Golubchik <serg@stripped>
 / /|_/ / // /\ \/ /_/ / /__  Principal Software Engineer/Server Architect
/_/  /_/\_, /___/\___\_\___/  Sun Microsystems GmbH, HRB München 161028
       <___/                  Sonnenallee 1, 85551 Kirchheim-Heimstetten
Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Dr. Roland Boemer
Vorsitzender des Aufsichtsrates: Martin Häring