List:Commits« Previous MessageNext Message »
From:Olav Sandstaa Date:February 20 2011 7:14pm
Subject:Re: bzr commit into mysql-5.5 branch (jorgen.loland:3257) Bug#11766234
View as plain text  
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);
>   }
>
>
>
>
>
>


Thread
bzr commit into mysql-5.5 branch (jorgen.loland:3257) Bug#11766234Jorgen Loland18 Feb
  • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3257) Bug#11766234Olav Sandstaa20 Feb
    • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3257) Bug#11766234Jon Olav Hauglid21 Feb
      • Re: bzr commit into mysql-5.5 branch (jorgen.loland:3257) Bug#11766234Jorgen Loland21 Feb