Hi Kristofer,
On 3/19/09 8:20 PM, Kristofer Pettersson wrote:
> #At file:///home/thek/Development/cpp/mysqlbzr/51-bug40127/ based on
> revid:gni@stripped
>
> 2842 Kristofer Pettersson 2009-03-20
> Bug#40127 Multiple table DELETE IGNORE hangs on foreign key constraint
> violation
> on 5.0
>
> The server crashes on an assert in net_end_statement indicating that the
> Diagnostics area wasn't set properly during execution.
> This happened on a multi table DELETE operation using the IGNORE keyword.
> The keyword is suppose to allow for execution to continue on a best effort
> despite some non-fatal errors. Instead execution stopped and no client
> response was sent which would have led to a protocol error if it hadn't been
> for the assert.
>
> This patch corrects this issue by checking for the existance of an IGNORE
existance -> existence
> option before setting an error state during row-by-row delete iteration.
> @ mysql-test/r/innodb_mysql.result
> * Added test case for bug40127
> @ mysql-test/t/innodb_mysql.test
> * Added test case for bug40127
> @ sql/sql_delete.cc
> * IGNORE option wasn't implemented in multi_delete::send_data
> and multi_delete::do_deletes
>
> modified:
> mysql-test/r/innodb_mysql.result
> mysql-test/t/innodb_mysql.test
> sql/sql_delete.cc
[..]
> === modified file 'mysql-test/t/innodb_mysql.test'
> --- a/mysql-test/t/innodb_mysql.test 2009-02-20 09:50:50 +0000
> +++ b/mysql-test/t/innodb_mysql.test 2009-03-19 23:20:25 +0000
> @@ -185,4 +185,53 @@ TRUNCATE TABLE t2;
> DROP TABLE t2;
> DROP TABLE t1;
>
> +--echo #
> +--echo # Bug#40127 Multiple table DELETE IGNORE hangs on foreign key constraint
> violation on 5.0
> +--echo #
> +CREATE TABLE t1 (
> + id INT UNSIGNED NOT NULL AUTO_INCREMENT,
> + PRIMARY KEY (id)
> +) ENGINE=InnoDB;
> +
> +CREATE TABLE t2 (
> + id INT UNSIGNED NOT NULL AUTO_INCREMENT,
> + aid INT UNSIGNED NOT NULL,
> + PRIMARY KEY (id),
> + FOREIGN KEY (aid) REFERENCES t1 (id)
> +) ENGINE=InnoDB;
> +
> +CREATE TABLE t3 (
> + bid INT UNSIGNED NOT NULL,
> + FOREIGN KEY (bid) REFERENCES t2 (id)
> +) ENGINE=InnoDB;
> +
> +CREATE TABLE t4 (
> + a INT
> +) ENGINE=InnoDB;
> +
> +CREATE TABLE t5 (
> + a INT
> +) ENGINE=InnoDB;
> +
> +INSERT INTO t1 (id) VALUES (1);
> +INSERT INTO t2 (id, aid) VALUES (1, 1),(2,1),(3,1),(4,1);
> +INSERT INTO t3 (bid) VALUES (1);
> +
> +INSERT INTO t4 VALUES (1),(2),(3),(4),(5);
> +INSERT INTO t5 VALUES (1);
> +
> +DELETE t5 FROM t4 LEFT JOIN t5 ON t4.a= t5.a;
> +
> +--error 1451
1451 -> ER_ROW_IS_REFERENCED_2
> +DELETE t2, t1 FROM t2 INNER JOIN t1 ON (t2.aid = t1.id) WHERE t2.id = 1;
> +--error 1451
> +DELETE t2, t1 FROM t2 INNER JOIN t1 ON (t2.aid = t1.id) WHERE t2.id = 1;
> +
> +DELETE IGNORE t2, t1 FROM t2 INNER JOIN t1 ON (t2.aid = t1.id) WHERE t2.id = 1;
> +
> +DROP TABLE t3;
> +DROP TABLE t2;
> +DROP TABLE t1;
> +DROP TABLES t4,t5;
> +
> --echo End of 5.1 tests
>
> === modified file 'sql/sql_delete.cc'
> --- a/sql/sql_delete.cc 2009-02-09 22:51:59 +0000
> +++ b/sql/sql_delete.cc 2009-03-19 23:20:25 +0000
> @@ -709,6 +709,8 @@ bool multi_delete::send_data(List<Item>
> TABLE_LIST *del_table;
> DBUG_ENTER("multi_delete::send_data");
>
> + bool no_error= thd->lex->current_select->no_error;
May I suggest to rename it to ignore? It's the name used elsewhere.
What is the difference between Lex::current_select::no_error and
Lex::ignore? do you test it?
> +
> for (del_table= delete_tables;
> del_table;
> del_table= del_table->next_local, secure_counter++)
> @@ -729,9 +731,28 @@ bool multi_delete::send_data(List<Item>
> if (table->triggers&&
> table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
> TRG_ACTION_BEFORE, FALSE))
> - DBUG_RETURN(1);
Do we ignore errors during trigger processing for the other forms
(INSERT, UPDATE, etc) of IGNORE?
> + {
> + /*
> + If the IGNORE option is used the errors will be translated into
> + warnings and we should not interrupt the loop.
> + */
> + if (!no_error)
> + DBUG_RETURN(1);
> + }
> table->status|= STATUS_DELETED;
> - if (!(error=table->file->ha_delete_row(table->record[0])))
> + error=table->file->ha_delete_row(table->record[0]);
> +
> + /*
> + If an error occurred and there is no IGNORE flag;
> + Signal an error and exit.
> + */
No need to rework here, just make the else check the flag:
if (!(error=table->file->ha_delete_row(table->record[0])))
..
else if (!ignore)
print_error
> + if (!no_error && error)
> + {
> + table->file->print_error(error,MYF(0));
> + DBUG_RETURN(1);
> + }
> +
> + if (!error)
> {
> deleted++;
> if (!table->file->has_transactions())
> @@ -739,12 +760,14 @@ bool multi_delete::send_data(List<Item>
> if (table->triggers&&
> table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
> TRG_ACTION_AFTER, FALSE))
> - DBUG_RETURN(1);
> - }
> - else
> - {
> - table->file->print_error(error,MYF(0));
> - DBUG_RETURN(1);
> + {
> + /*
> + If the IGNORE option is used the errors will be translated into
> + warning messages and we should not stop.
> + */
> + if (!no_error)
> + DBUG_RETURN(1);
> + }
> }
> }
> else
> @@ -834,6 +857,11 @@ int multi_delete::do_deletes()
> {
> int local_error= 0, counter= 0, tmp_error;
> bool will_batch;
> + /*
> + If the IGNORE option is used all non fatal errors will be translated
> + to warnings and we should not break the row-by-row iteration.
> + */
> + bool no_errors= thd->lex->current_select->no_error;
> DBUG_ENTER("do_deletes");
> DBUG_ASSERT(do_delete);
>
> @@ -869,21 +897,30 @@ int multi_delete::do_deletes()
> table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
> TRG_ACTION_BEFORE, FALSE))
> {
> - local_error= 1;
> - break;
> + if (!no_errors)
> + {
> + local_error= 1;
> + break;
> + }
Isn't this a fatal error? It seems we shouldn't ignore fatal errors.
> }
> if ((local_error=table->file->ha_delete_row(table->record[0])))
> {
> - table->file->print_error(local_error,MYF(0));
> - break;
> + if (!no_errors)
> + {
> + table->file->print_error(local_error,MYF(0));
> + break;
> + }
Please take the opportunity and remove any tabs.
> }
> deleted++;
> if (table->triggers&&
> table->triggers->process_triggers(thd, TRG_EVENT_DELETE,
> TRG_ACTION_AFTER, FALSE))
> {
> - local_error= 1;
> - break;
> + if (!no_errors)
> + {
> + local_error= 1;
> + break;
> + }
Same question about triggers.
Regards,
-- Davi Arnaut
