Hi Jørgen,
Thanks for fixing this bug and for a very well commented commit message.
The solution looks correct. I have only two minor comments. Feel free to
ignore them if you do not agree.
OK to push.
Olav
On 02/18/2011 03:57 PM, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-5.5-59299/ based on
> revid:jorgen.loland@stripped
>
> 3257 Jorgen Loland 2011-02-18
> BUG#11766234: ASSERT (TABLE_REF->TABLE || TABLE_REF->VIEW)
> FAILS IN SET_FIELD_ITERATOR
>
> (Former 59299)
>
> When a PROCEDURE does a natural join, resolving of which columns are
> used in the join is done only once; consecutive CALLs to the procedure
> will reuse this information:
>
> CREATE PROCEDURE proc() SELECT * FROM t1 NATURAL JOIN v1;
> CALL proc();<- natural join columns resolved here
> CALL proc();<- reuse resolved NJ columns from first CALL
>
> The second CALL knows that it can reuse the resolved NJ columns because
> the first CALL sets st_select_lex::first_natural_join_processing=false.
> The problem in this bug was that the table the view v1 depends on
> changed between CREATE PROCEDURE and the first CALL:
>
> CREATE PROCEDURE...
> ALTER TABLE t2 CHANGE COLUMN a b CHAR;
> CALL proc();<- error when resolving natural join columns
> CALL proc();<- tries to reuse from first CALL => crash
>
> The fix for this bug is to set first_natural_join_processing= FALSE iff
> the natural join columns resolving was successful.
> @ mysql-test/r/sp.result
> Add test for bug 11766234
> @ mysql-test/t/sp.test
> Add test for bug 11766234
> @ sql/sql_base.cc
> Set first_natural_join_processing= FALSE iff the natural join columns
> resolving was successful.
>
> modified:
> mysql-test/r/sp.result
> mysql-test/t/sp.test
> sql/sql_base.cc
> === modified file 'mysql-test/r/sp.result'
> --- a/mysql-test/r/sp.result 2010-07-30 15:28:36 +0000
> +++ b/mysql-test/r/sp.result 2011-02-18 14:57:48 +0000
> @@ -7161,6 +7161,26 @@ init_connect
> SET @@GLOBAL.init_connect= @old_init_connect;
> DROP PROCEDURE p2;
> DROP PROCEDURE p5;
> +#
> +# BUG#11766234: 59299: ASSERT (TABLE_REF->TABLE || TABLE_REF->VIEW)
> +# FAILS IN SET_FIELD_ITERATOR
> +#
> +CREATE TABLE t1 (i INT, a INT);
> +CREATE TABLE t2 (j INT, a INT);
> +INSERT INTO t1 VALUES (1,1), (2,2);
> +INSERT INTO t2 VALUES (2,2), (3,3);
> +CREATE VIEW v1 AS SELECT a FROM t2;
> +CREATE PROCEDURE proc() SELECT * FROM t1 NATURAL JOIN v1;
> +ALTER TABLE t2 CHANGE COLUMN a b CHAR;
> +
> +CALL proc();
> +ERROR HY000: View 'test.v1' references invalid table(s) or column(s) or function(s)
> or definer/invoker of view lack rights to use them
> +CALL proc();
> +ERROR HY000: View 'test.v1' references invalid table(s) or column(s) or function(s)
> or definer/invoker of view lack rights to use them
> +
> +DROP TABLE t1,t2;
> +DROP VIEW v1;
> +DROP PROCEDURE proc;
> # ------------------------------------------------------------------
> # -- End of 5.1 tests
> # ------------------------------------------------------------------
>
> === modified file 'mysql-test/t/sp.test'
> --- a/mysql-test/t/sp.test 2010-07-30 15:28:36 +0000
> +++ b/mysql-test/t/sp.test 2011-02-18 14:57:48 +0000
> @@ -8376,6 +8376,30 @@ SET @@GLOBAL.init_connect= @old_init_con
> DROP PROCEDURE p2;
> DROP PROCEDURE p5;
>
> +--echo #
> +--echo # BUG#11766234: 59299: ASSERT (TABLE_REF->TABLE || TABLE_REF->VIEW)
> +--echo # FAILS IN SET_FIELD_ITERATOR
> +--echo #
> +
> +CREATE TABLE t1 (i INT, a INT);
> +CREATE TABLE t2 (j INT, a INT);
> +INSERT INTO t1 VALUES (1,1), (2,2);
> +INSERT INTO t2 VALUES (2,2), (3,3);
> +CREATE VIEW v1 AS SELECT a FROM t2;
> +CREATE PROCEDURE proc() SELECT * FROM t1 NATURAL JOIN v1;
> +ALTER TABLE t2 CHANGE COLUMN a b CHAR;
> +
> +--echo
> +--error ER_VIEW_INVALID
> +CALL proc();
> +--error ER_VIEW_INVALID
> +CALL proc();
> +
> +--echo
> +DROP TABLE t1,t2;
> +DROP VIEW v1;
> +DROP PROCEDURE proc;
> +
> --echo # ------------------------------------------------------------------
> --echo # -- End of 5.1 tests
> --echo # ------------------------------------------------------------------
>
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc 2011-01-11 11:33:28 +0000
> +++ b/sql/sql_base.cc 2011-02-18 14:57:48 +0000
> @@ -7569,9 +7569,10 @@ static bool setup_natural_join_row_types
> List<TABLE_LIST> *from_clause,
> Name_resolution_context *context)
> {
> + DBUG_ENTER("setup_natural_join_row_types");
> thd->where= "from clause";
> if (from_clause->elements == 0)
> - return FALSE; /* We come here in the case of UNIONs. */
> + DBUG_RETURN(false); /* We come here in the case of UNIONs. */
>
> List_iterator_fast<TABLE_LIST> table_ref_it(*from_clause);
> TABLE_LIST *table_ref; /* Current table reference. */
> @@ -7582,8 +7583,6 @@ static bool setup_natural_join_row_types
> bool save_first_natural_join_processing=
> context->select_lex->first_natural_join_processing;
Consider: remove the local variable "save_first_natural_join_processing"
and use "context->select_lex->first_natural_join_processing" directly in
the only place in function where this now is in use after your fix.
>
> - context->select_lex->first_natural_join_processing= FALSE;
> -
> /* Note that tables in the list are in reversed order */
> for (left_neighbor= table_ref_it++; left_neighbor ; )
> {
> @@ -7596,10 +7595,9 @@ static bool setup_natural_join_row_types
> */
> if (save_first_natural_join_processing)
> {
> - context->select_lex->first_natural_join_processing= FALSE;
> if (store_top_level_join_columns(thd, table_ref,
> left_neighbor, right_neighbor))
> - return TRUE;
> + DBUG_RETURN(true);
> if (left_neighbor)
> {
> TABLE_LIST *first_leaf_on_the_right;
> @@ -7619,8 +7617,9 @@ static bool setup_natural_join_row_types
> DBUG_ASSERT(right_neighbor);
> context->first_name_resolution_table=
> right_neighbor->first_leaf_for_name_resolution();
> + context->select_lex->first_natural_join_processing= FALSE;
Consider "false" instead of "FALSE".
>
> - return FALSE;
> + DBUG_RETURN (false);
> }
>
>
>
>
>
>
