From: Jorgen Loland
Date: September 6 2010 7:04am
Subject: Re: bzr commit into mysql-5.1-bugteam branch (martin.hansson:3498)
 Bug#54543
List-Archive: http://lists.mysql.com/commits/117564
Message-Id: <4C849299.7090700@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Martin,

The fix looks correct, but since I was curious why the query was well behaving 
in next-mr I spent a few hours to realize that do_select() was restructured a 
bit by Kostja's patch:

revno: 2630.13.6
committer: Konstantin Osipov <konstantin@stripped>
branch nick: mysql-6.0-3288
timestamp: Fri 2008-07-11 20:22:44 +0400
message:
   WL#3288, step 1: ensure that the SQL layer always closes an open
   cursor (rnd or index read) before closing a handler.

In next-mr, the "if(table)" block is moved before the "if (error == 
NESTED_LOOP_OK)" block, and in the else part of if(table), you find

join->join_free();

As I see it, Kostja's fix of do_select is exactly what you are looking for to 
fix this bug. If you backport that (at least the parts concerning do_select) 
instead of the patch you just committed, you'll also help align the 5.1 and 
next-mr codebases.

Just my two cents.

Jørgen

On 09/01/2010 09:18 AM, Martin Hansson wrote:
> #At file:///data0/martin/bzr/bug54543/5.1bt-commit/ based on revid:davi.arnaut@stripped
>
>   3498 Martin Hansson	2010-09-01
>        Bug#54543: update ignore with incorrect subquery leads to assertion failure:
>        inited==INDEX
>
>        When an error occurs while sending the data in a temporary table there was no
>        cleanup performed. This caused a failed assertion in the case when different
>        access methods were used for populating the table vs. retrieving the data from
>        the table if IGNORE was specified and sql_safe_updates = 0. In this case
>        execution continues, but the handler expects to continue with the access
>        method used for row retrieval.
>
>        Fixed by doing the cleanup even if errors occur.
>
>      modified:
>        mysql-test/r/multi_update.result
>        mysql-test/t/multi_update.test
>        sql/sql_select.cc
> === modified file 'mysql-test/r/multi_update.result'
> --- a/mysql-test/r/multi_update.result	2010-05-24 13:54:08 +0000
> +++ b/mysql-test/r/multi_update.result	2010-09-01 07:18:20 +0000
> @@ -639,4 +639,24 @@ SET SESSION sql_safe_updates = 1;
>   UPDATE IGNORE t1, t1 t1a SET t1.a = 1 WHERE t1a.a = 1;
>   ERROR HY000: You are using safe update mode and you tried to update a table without a WHERE that uses a KEY column
>   DROP TABLE t1;
> +#
> +# Bug#54543: update ignore with incorrect subquery leads to assertion
> +# failure: inited==INDEX
> +#
> +SET SESSION sql_safe_updates = 0;
> +CREATE TABLE t1 ( a INT );
> +INSERT INTO t1 VALUES (1), (2);
> +CREATE TABLE t2 ( a INT );
> +INSERT INTO t2 VALUES (1), (2);
> +CREATE TABLE t3 ( a INT );
> +INSERT INTO t3 VALUES (1), (2);
> +# Should not crash
> +UPDATE IGNORE
> +( SELECT ( SELECT COUNT(*) FROM t1 GROUP BY a, @v ) a FROM t2 ) x, t3
> +SET t3.a = 0;
> +Warnings:
> +Error	1242	Subquery returns more than 1 row
> +Error	1242	Subquery returns more than 1 row
> +DROP TABLE t1, t2, t3;
> +SET SESSION sql_safe_updates = DEFAULT;
>   end of tests
>
> === modified file 'mysql-test/t/multi_update.test'
> --- a/mysql-test/t/multi_update.test	2010-05-24 13:54:08 +0000
> +++ b/mysql-test/t/multi_update.test	2010-09-01 07:18:20 +0000
> @@ -651,5 +651,26 @@ SET SESSION sql_safe_updates = 1;
>   UPDATE IGNORE t1, t1 t1a SET t1.a = 1 WHERE t1a.a = 1;
>   DROP TABLE t1;
>
> +--echo #
> +--echo # Bug#54543: update ignore with incorrect subquery leads to assertion
> +--echo # failure: inited==INDEX
> +--echo #
> +SET SESSION sql_safe_updates = 0;
> +CREATE TABLE t1 ( a INT );
> +INSERT INTO t1 VALUES (1), (2);
> +
> +CREATE TABLE t2 ( a INT );
> +INSERT INTO t2 VALUES (1), (2);
> +
> +CREATE TABLE t3 ( a INT );
> +INSERT INTO t3 VALUES (1), (2);
> +
> +--echo # Should not crash
> +UPDATE IGNORE
> +  ( SELECT ( SELECT COUNT(*) FROM t1 GROUP BY a, @v ) a FROM t2 ) x, t3
> +SET t3.a = 0;
> +
> +DROP TABLE t1, t2, t3;
> +SET SESSION sql_safe_updates = DEFAULT;
>
>   --echo end of tests
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2010-08-26 09:31:04 +0000
> +++ b/sql/sql_select.cc	2010-09-01 07:18:20 +0000
> @@ -11157,22 +11157,20 @@ do_select(JOIN *join,List<Item>  *fields,
>     if (error == NESTED_LOOP_NO_MORE_ROWS)
>       error= NESTED_LOOP_OK;
>
> +  if (table == NULL)					// If sending data to client
> +    /*
> +      The following will unlock all cursors if the command wasn't an
> +      update command
> +    */
> +    join->join_free();			// Unlock all cursors
>     if (error == NESTED_LOOP_OK)
>     {
>       /*
>         Sic: this branch works even if rc != 0, e.g. when
>         send_data above returns an error.
>       */
> -    if (!table)					// If sending data to client
> -    {
> -      /*
> -	The following will unlock all cursors if the command wasn't an
> -	update command
> -      */
> -      join->join_free();			// Unlock all cursors
> -      if (join->result->send_eof())
> -	rc= 1;                                  // Don't send error
> -    }
> +    if (table == NULL&&  join->result->send_eof()) // If sending data to client
> +      rc= 1;                                  // Don't send error
>       DBUG_PRINT("info",("%ld records output", (long) join->send_records));
>     }
>     else
>
>
>
>
>

-- 
Jørgen Løland | Senior Software Engineer | +47 73842138
Oracle MySQL
Trondheim, Norway