Joro,
please find my comments below.
Timour
kgeorge@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of gkodinov. When gkodinov does a push these changes
> will be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2008-04-09 15:44:27+03:00, gkodinov@stripped +4 -0
> Bug #33811: Call to stored procedure with SELECT * / RIGHT JOIN
> fails after the first time
>
> Two separate problems :
> 1. When flattening joins the linked list used for name resolution
> (next_name_resolution_table) was not updated.
> Fixed by updating the pointers when extending the table list
>
> 2. The items created by expanding a * (star) as a column reference
> were marked as fixed, but no cached table was assigned to them
> (unlike what Item_field::fix_fields does).
> Fixed by assigning a cached table (so the re-preparation is done
> faster).
It is not clear whether the fix would work without the second item.
Secondly, from the comment it is not clear where do all items get marked
as "fixed". The create_item() method has different implementations depending
on the type of column.
If your hypothesis is correct, then it should be possible to add a
DBUG_ASSERT(item->fixed) just before updating the cache (see below).
>
> Note that the fix for #2 hides the fix for #1 in most cases
> (except when a table reference cannot be cached).
>
> mysql-test/r/sp.result@stripped, 2008-04-09 15:44:24+03:00, gkodinov@stripped +18
> -0
> Bug #33811: test case
>
> mysql-test/t/sp.test@stripped, 2008-04-09 15:44:24+03:00, gkodinov@stripped +15 -0
> Bug #33811: test case
>
> sql/sql_base.cc@stripped, 2008-04-09 15:44:24+03:00, gkodinov@stripped +3 -0
> Bug #33811: cache the table for Item_fields created by expanding '*'
>
> sql/sql_select.cc@stripped, 2008-04-09 15:44:24+03:00, gkodinov@stripped +11 -1
> Bug #33811: maintain a correct name resolution chain when
> flattening joins.
>
> diff -Nrup a/mysql-test/r/sp.result b/mysql-test/r/sp.result
> --- a/mysql-test/r/sp.result 2008-02-17 13:37:38 +02:00
> +++ b/mysql-test/r/sp.result 2008-04-09 15:44:24 +03:00
> @@ -6646,6 +6646,24 @@ ttt
> 2
> drop function func30787;
> drop table t1;
> +CREATE TABLE t1 (id INT);
> +INSERT INTO t1 VALUES (1),(2),(3),(4);
> +CREATE PROCEDURE test_sp()
> +SELECT t1.* FROM t1 RIGHT JOIN t1 t2 ON t1.id=t2.id;
> +CALL test_sp();
> +id
> +1
> +2
> +3
> +4
> +CALL test_sp();
> +id
> +1
> +2
> +3
> +4
> +DROP PROCEDURE test_sp;
> +DROP TABLE t1;
> # ------------------------------------------------------------------
> # -- End of 5.0 tests
> # ------------------------------------------------------------------
> diff -Nrup a/mysql-test/t/sp.test b/mysql-test/t/sp.test
> --- a/mysql-test/t/sp.test 2008-02-17 13:37:38 +02:00
> +++ b/mysql-test/t/sp.test 2008-04-09 15:44:24 +03:00
> @@ -7793,6 +7793,21 @@ select (select func30787(f1)) as ttt fro
> drop function func30787;
> drop table t1;
>
> +#
> +# Bug #33811: Call to stored procedure with SELECT * / RIGHT JOIN fails
> +# after the first time
> +#
> +CREATE TABLE t1 (id INT);
> +INSERT INTO t1 VALUES (1),(2),(3),(4);
> +
> +CREATE PROCEDURE test_sp()
> + SELECT t1.* FROM t1 RIGHT JOIN t1 t2 ON t1.id=t2.id;
> +
> +CALL test_sp();
> +CALL test_sp();
> +
> +DROP PROCEDURE test_sp;
> +DROP TABLE t1;
>
> --echo # ------------------------------------------------------------------
> --echo # -- End of 5.0 tests
> diff -Nrup a/sql/sql_base.cc b/sql/sql_base.cc
> --- a/sql/sql_base.cc 2008-03-21 17:23:14 +02:00
> +++ b/sql/sql_base.cc 2008-04-09 15:44:24 +03:00
> @@ -5486,6 +5486,9 @@ insert_fields(THD *thd, Name_resolution_
>
> if (!(item= field_iterator.create_item(thd)))
> DBUG_RETURN(TRUE);
This assert should hold true at this point:
DBUG_ASSERT(item->fixed);
> + /* cache the table for the Item_fields inserted by expanding stars */
> + if (item->type() == Item::FIELD_ITEM &&
> tables->cacheable_table)
> + ((Item_field *)item)->cached_table= tables;
I think that it makes more sense to update cached_table where the item is
created, that is, either inside create_item, or in the methods that it
calls that create items. Since items are not always created (or found).
>
> if (!found)
> {
> diff -Nrup a/sql/sql_select.cc b/sql/sql_select.cc
> --- a/sql/sql_select.cc 2008-03-26 20:10:00 +02:00
> +++ b/sql/sql_select.cc 2008-04-09 15:44:24 +03:00
> @@ -8283,6 +8283,8 @@ simplify_joins(JOIN *join, List<TABLE_LI
> }
>
> /* Flatten nested joins that can be flattened. */
> + TABLE_LIST *right_neighbour= NULL;
Replace right_neighbour with right_neighbor - we use US spelling :).
> + bool fix_name_res= FALSE;
> li.rewind();
> while ((table= li++))
> {
> @@ -8295,9 +8297,17 @@ simplify_joins(JOIN *join, List<TABLE_LI
> {
> tbl->embedding= table->embedding;
> tbl->join_list= table->join_list;
> - }
> + }
> li.replace(nested_join->join_list);
> + /* Need to update the name resolution table chain when falttening joins */
Replace "falttening" with "flattening".
> + fix_name_res= TRUE;
> + table= *li.ref();
> }
> + if (fix_name_res)
> + table->next_name_resolution_table= right_neighbour ?
> + right_neighbour->first_leaf_for_name_resolution() :
> + NULL;
> + right_neighbour= table;
> }
> DBUG_RETURN(conds);
> }
>
