Hi!
On Aug 14, Gleb Shchepa wrote:
> #At file:///work/bzr/5.1-bugteam-26020/
>
> 2688 Gleb Shchepa 2008-08-14
> Bug#26020: User-Defined Variables are not consistent with
> columns data types
>
> The "SELECT @lastId, @lastId := Id FROM t" query returns
> different result sets depending on the type of the Id column
> (INT or BIGINT).
>
> === modified file 'mysql-test/r/user_var.result'
> --- a/mysql-test/r/user_var.result 2007-12-13 11:56:04 +0000
> +++ b/mysql-test/r/user_var.result 2008-08-13 20:00:41 +0000
> @@ -121,8 +121,8 @@ select @a:=0;
> select @a+0, @a:=@a+0+count(*), count(*), @a+0 from t1 group by i;
> @a+0 @a:=@a+0+count(*) count(*) @a+0
> 0 1 1 0
> -1 3 2 0
> -3 6 3 0
> +0 2 2 0
> +0 3 3 0
Why is new result better than the old ?
> === modified file 'sql/item_func.cc'
> --- a/sql/item_func.cc 2008-07-31 09:50:24 +0000
> +++ b/sql/item_func.cc 2008-08-13 20:00:41 +0000
> @@ -4667,9 +4661,36 @@ void Item_func_get_user_var::fix_length_
> }
>
>
> -bool Item_func_get_user_var::const_item() const
> +bool Item_func_get_user_var::fix_fields(THD *thd, Item **ref)
> {
> - return (!var_entry || current_thd->query_id != var_entry->update_query_id);
> + DBUG_ENTER("Item_func_get_user_var::fix_fields");
> + DBUG_ASSERT(fixed == 0);
> + /* fix_fields will call Item_func_set_user_var::fix_length_and_dec */
> + if (Item_func::fix_fields(thd, ref))
> + DBUG_RETURN(TRUE);
> +
> + const_item_cache= TRUE;
> +
> + if (!var_entry)
> + DBUG_RETURN(FALSE);
> +
> + if (thd->lex->sql_command == SQLCOM_LOAD)
> + {
> + const_item_cache= current_thd->query_id != var_entry->update_query_id;
Why do you want to treat LOAD DATA specially ? I'd prefer to handle all
statements uniformly.
Actually, you can keep var_entry->update_query_id for all statements.
Consider
SELECT @a, @a:=col, @a FROM ...
In this case in the first @a item you'll do a search in the
thd->lex->set_var_list, and notice that @a is not constant. You can
set var_entry->update_query_id right away then, and in the second @a
item you won't need to search the list, you can know that the @a is not
constant immediately.
That is, you only need to search the list if
current_thd->query_id == var_entry->update_query_id
Even better would be to have a three-state status field here, "var_entry
is const", "var_entry is not const", "unknown, need to search the list".
Note that typically, if a variable is used and modified in the same
query, it's used in a pretty complex expressions, appearing many times
in a query. With the optimization as above, you will only need to search
the list once.
A different approach would be to mark variables as "modifiable" before
fix_fields. That is,
* move entry= get_variable(&thd->user_vars, name, 1) to a separate
method, Item_func_set_user_var::set_entry().
* call ::set_entry() for all items from the thd->lex->set_var_list
from setup_fields() before the first call of ::fix_fields().
* when calling ::set_entry() update var_entry->update_query_id to the
current_thd->query_id.
This way you have no memcmp and no list searches at all.
> + DBUG_RETURN(FALSE);
> + }
> + List_iterator<Item_func_set_user_var> li(thd->lex->set_var_list);
> + Item_func_set_user_var *var;
> + while ((var= li++))
> + {
> + if (name.length == var->name.length &&
> + !memcmp(name.str, var->name.str, name.length))
memcmp() is probably wrong, it should be charset-aware
comparison.
> + {
> + const_item_cache= FALSE;
> + break;
> + }
> + }
> + DBUG_RETURN(FALSE);
> }
Regards / Mit vielen Grüssen,
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
