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
