Hi again,
Btw, I used the attached test case to test your patch. It covers a lot
of cases. Maybe it can be useful to include in the patch.
OK, sorry for sending so many emails. I will shut up now :-)
/Sven
He Zhenxing wrote:
> #At file:///media/sdb2/hezx/work/mysql/bzr/b42415/5.1-bugteam/ based on
> revid:ramil@stripped
>
> 3418 He Zhenxing 2010-07-11
> Bug #42415 UPDATE/DELETE with LIMIT clause unsafe for SBL even with ORDER BY PK
> clause
>
> Before the patch, when using STATEMENT mode, unsafe warnings were
> issued for all DML statements (INSERT...SELECT, UPDATE, DELETE)
> with LIMIT clause due to the possobility of non-deterministic result
> order, even when they also had a ORDER BY primary_key clause. In
> which case the order would be deterministic.
>
> This patch fixed the problem by checking the ORDER BY clause of the
> statement, and do not issue the warning if the result is ordered by
> the primary key (thus deterministic).
>
> modified:
> mysql-test/suite/binlog/r/binlog_unsafe.result
> mysql-test/suite/binlog/t/binlog_unsafe.test
> sql/sql_delete.cc
> sql/sql_insert.cc
> sql/sql_select.cc
> sql/sql_select.h
> sql/sql_update.cc
> === modified file 'mysql-test/suite/binlog/r/binlog_unsafe.result'
> --- a/mysql-test/suite/binlog/r/binlog_unsafe.result 2010-01-13 09:00:03 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_unsafe.result 2010-07-11 13:30:46 +0000
> @@ -412,4 +412,53 @@ a
> 13:46:40
> 1970-01-12 13:46:40
> DROP TABLE t1;
> +DROP TABLE IF EXISTS t1,t2,t3,t4;
> +Warnings:
> +Note 1051 Unknown table 't1'
> +Note 1051 Unknown table 't2'
> +Note 1051 Unknown table 't3'
> +Note 1051 Unknown table 't4'
> +CREATE TABLE t1 (a INT PRIMARY KEY);
> +CREATE TABLE t2 (a INT);
> +CREATE TABLE t3 (a INT);
> +CREATE TABLE t4 (a INT);
> +INSERT INTO t1 VALUES (1),(2),(3);
> +INSERT INTO t2 VALUES (1),(2),(3);
> +#
> +# Unsafe statements
> +#
> +UPDATE t1 SET a = a+10 LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +UPDATE t2 SET a = a+10 ORDER BY a LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +INSERT INTO t3 SELECT * FROM t1 LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +INSERT INTO t4 SELECT * FROM t2 ORDER BY a LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +INSERT INTO t4 SELECT t1.a FROM t2,t1 ORDER BY t1.a LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +INSERT INTO t4 SELECT t2.a FROM t1,t2 ORDER BY t2.a LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +DELETE FROM t1 LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +DELETE FROM t2 ORDER BY a LIMIT 1;
> +Warnings:
> +Note 1592 Statement may not be safe to log in statement format.
> +#
> +# Safe statements
> +#
> +UPDATE t1 SET a = a+10 ORDER BY a LIMIT 1;
> +INSERT INTO t3 SELECT * FROM t1 ORDER BY a LIMIT 1;
> +DELETE FROM t1 ORDER BY a LIMIT 1;
> +UPDATE t2 SET a = a+10 LIMIT 0;
> +INSERT INTO t4 SELECT * FROM t2 LIMIT 0;
> +DELETE FROM t2 LIMIT 0;
> +DROP TABLE t1, t2, t3, t4;
> "End of tests"
>
> === modified file 'mysql-test/suite/binlog/t/binlog_unsafe.test'
> --- a/mysql-test/suite/binlog/t/binlog_unsafe.test 2010-01-13 09:00:03 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_unsafe.test 2010-07-11 13:30:46 +0000
> @@ -49,6 +49,7 @@
> # BUG#42640: mysqld crashes when unsafe statements are executed (STRICT_TRANS_TABLES
> mode)
> # BUG#47995: Mark user functions as unsafe
> # BUG#49222: Mare RAND() unsafe
> +# BUG#42415: UPDATE/DELETE with LIMIT clause unsafe for SBL even with ORDER BY PK
> clause
> #
> # ==== Related test cases ====
> #
> @@ -444,4 +445,41 @@ SELECT * FROM t1;
>
> DROP TABLE t1;
>
> +#
> +# Test case for BUG#42415
> +# UPDATE/DELETE with LIMIT clause unsafe for SBL even with ORDER BY PK clause
> +#
> +DROP TABLE IF EXISTS t1,t2,t3,t4;
> +CREATE TABLE t1 (a INT PRIMARY KEY);
> +CREATE TABLE t2 (a INT);
> +CREATE TABLE t3 (a INT);
> +CREATE TABLE t4 (a INT);
> +
> +INSERT INTO t1 VALUES (1),(2),(3);
> +INSERT INTO t2 VALUES (1),(2),(3);
> +
> +-- echo #
> +-- echo # Unsafe statements
> +-- echo #
> +UPDATE t1 SET a = a+10 LIMIT 1;
> +UPDATE t2 SET a = a+10 ORDER BY a LIMIT 1;
> +INSERT INTO t3 SELECT * FROM t1 LIMIT 1;
> +INSERT INTO t4 SELECT * FROM t2 ORDER BY a LIMIT 1;
> +INSERT INTO t4 SELECT t1.a FROM t2,t1 ORDER BY t1.a LIMIT 1;
> +INSERT INTO t4 SELECT t2.a FROM t1,t2 ORDER BY t2.a LIMIT 1;
> +DELETE FROM t1 LIMIT 1;
> +DELETE FROM t2 ORDER BY a LIMIT 1;
> +
> +-- echo #
> +-- echo # Safe statements
> +-- echo #
> +UPDATE t1 SET a = a+10 ORDER BY a LIMIT 1;
> +INSERT INTO t3 SELECT * FROM t1 ORDER BY a LIMIT 1;
> +DELETE FROM t1 ORDER BY a LIMIT 1;
> +UPDATE t2 SET a = a+10 LIMIT 0;
> +INSERT INTO t4 SELECT * FROM t2 LIMIT 0;
> +DELETE FROM t2 LIMIT 0;
> +
> +DROP TABLE t1, t2, t3, t4;
> +
> --echo "End of tests"
>
> === modified file 'sql/sql_delete.cc'
> --- a/sql/sql_delete.cc 2010-05-14 11:36:27 +0000
> +++ b/sql/sql_delete.cc 2010-07-11 13:30:46 +0000
> @@ -92,6 +92,19 @@ bool mysql_delete(THD *thd, TABLE_LIST *
> }
> }
>
> + /*
> + Statement-based replication of DELETE ... LIMIT is not safe as
> + order of rows is not defined unless ORDER BY primary_key, so in
> + mixed mode we go to row-based.
> + */
> + if (thd->lex->current_select->select_limit &&
> + select_lex->select_limit->val_int() &&
> + !is_order_deterministic(table_list->table, (ORDER*)order->first))
> + {
> + thd->lex->set_stmt_unsafe();
> + thd->set_current_stmt_binlog_row_based_if_mixed();
> + }
> +
> const_cond= (!conds || conds->const_item());
> safe_update=test(thd->options & OPTION_SAFE_UPDATES);
> if (safe_update && const_cond)
> @@ -475,19 +488,6 @@ int mysql_prepare_delete(THD *thd, TABLE
> DBUG_ENTER("mysql_prepare_delete");
> List<Item> all_fields;
>
> - /*
> - Statement-based replication of DELETE ... LIMIT is not safe as order of
> - rows is not defined, so in mixed mode we go to row-based.
> -
> - Note that we may consider a statement as safe if ORDER BY primary_key
> - is present. However it may confuse users to see very similiar statements
> - replicated differently.
> - */
> - if (thd->lex->current_select->select_limit)
> - {
> - thd->lex->set_stmt_unsafe();
> - thd->set_current_stmt_binlog_row_based_if_mixed();
> - }
> thd->lex->allow_sum_func= 0;
> if (setup_tables_and_check_access(thd, &thd->lex->select_lex.context,
> &thd->lex->select_lex.top_join_list,
>
> === modified file 'sql/sql_insert.cc'
> --- a/sql/sql_insert.cc 2010-03-29 02:32:30 +0000
> +++ b/sql/sql_insert.cc 2010-07-11 13:30:46 +0000
> @@ -2881,19 +2881,6 @@ bool mysql_insert_select_prepare(THD *th
> DBUG_ENTER("mysql_insert_select_prepare");
>
> /*
> - Statement-based replication of INSERT ... SELECT ... LIMIT is not safe
> - as order of rows is not defined, so in mixed mode we go to row-based.
> -
> - Note that we may consider a statement as safe if ORDER BY primary_key
> - is present or we SELECT a constant. However it may confuse users to
> - see very similiar statements replicated differently.
> - */
> - if (lex->current_select->select_limit)
> - {
> - lex->set_stmt_unsafe();
> - thd->set_current_stmt_binlog_row_based_if_mixed();
> - }
> - /*
> SELECT_LEX do not belong to INSERT statement, so we can't add WHERE
> clause if table is VIEW
> */
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-05-27 15:13:53 +0000
> +++ b/sql/sql_select.cc 2010-07-11 13:30:46 +0000
> @@ -2492,6 +2492,30 @@ mysql_select(THD *thd, Item ***rref_poin
> }
> }
>
> + if (thd->lex->sql_command == SQLCOM_INSERT_SELECT ||
> + thd->lex->sql_command == SQLCOM_REPLACE_SELECT)
> + {
> + /*
> + Statement-based replication of INSERT ... SELECT ... LIMIT is
> + not safe as order of rows is not defined unless ORDER BY
> + primary_key, so in mixed mode we go to row-based.
> +
> + NOTE: When more than one tables are joined in the SELECT part,
> + it will very hard to figure out whether the result order will be
> + deterministic or not, so they are always considered unsafe to
> + simplify the logic.
> + */
> + if (!thd->lex->is_stmt_unsafe() && select_lex->select_limit
> &&
> + select_lex->select_limit->val_int() &&
> + (((TABLE_LIST*)select_lex->table_list.first)->next_local ||
> + !is_order_deterministic(((TABLE_LIST
> *)select_lex->table_list.first)->table,
> + (ORDER *)(select_lex->order_list.first))))
> + {
> + thd->lex->set_stmt_unsafe();
> + thd->set_current_stmt_binlog_row_based_if_mixed();
> + }
> + }
> +
> if ((err= join->optimize()))
> {
> goto err; // 1
>
> === modified file 'sql/sql_select.h'
> --- a/sql/sql_select.h 2010-02-26 13:16:46 +0000
> +++ b/sql/sql_select.h 2010-07-11 13:30:46 +0000
> @@ -794,3 +794,4 @@ inline bool optimizer_flag(THD *thd, uin
> return (thd->variables.optimizer_switch & flag);
> }
>
> +bool is_order_deterministic(TABLE *table, ORDER *order);
>
> === modified file 'sql/sql_update.cc'
> --- a/sql/sql_update.cc 2010-05-27 20:07:40 +0000
> +++ b/sql/sql_update.cc 2010-07-11 13:30:46 +0000
> @@ -850,6 +850,35 @@ err:
> }
>
> /*
> + Test if the result order is deterministic.
> +
> + @retval FALSE not deterministic
> + @retval TRUE deterministic
> + */
> +bool is_order_deterministic(TABLE *table, ORDER *order)
> +{
> + MY_BITMAP order_set;
> + MY_BITMAP key_set;
> + uint key= table->s->primary_key;
> +
> + if (order == NULL)
> + return FALSE;
> + if (key == MAX_KEY)
> + return FALSE;
> +
> + bitmap_init(&order_set, NULL, table->s->fields, FALSE);
> + for (; order; order=order->next)
> + {
> + Field *field=((Item_field*) (*order->item)->real_item())->field;
> + bitmap_set_bit(&order_set, field->field_index);
> + }
> +
> + bitmap_init(&key_set, NULL, table->s->fields, FALSE);
> + table->mark_columns_used_by_index_no_reset(key, &key_set);
> + return bitmap_is_subset(&key_set, &order_set);
> +}
> +
> +/*
> Prepare items in UPDATE statement
>
> SYNOPSIS
> @@ -875,19 +904,6 @@ bool mysql_prepare_update(THD *thd, TABL
> SELECT_LEX *select_lex= &thd->lex->select_lex;
> DBUG_ENTER("mysql_prepare_update");
>
> - /*
> - Statement-based replication of UPDATE ... LIMIT is not safe as order of
> - rows is not defined, so in mixed mode we go to row-based.
> -
> - Note that we may consider a statement as safe if ORDER BY primary_key
> - is present. However it may confuse users to see very similiar statements
> - replicated differently.
> - */
> - if (thd->lex->current_select->select_limit)
> - {
> - thd->lex->set_stmt_unsafe();
> - thd->set_current_stmt_binlog_row_based_if_mixed();
> - }
> #ifndef NO_EMBEDDED_ACCESS_CHECKS
> table_list->grant.want_privilege= table->grant.want_privilege=
> (SELECT_ACL & ~table->grant.privilege);
> @@ -908,6 +924,19 @@ bool mysql_prepare_update(THD *thd, TABL
> setup_ftfuncs(select_lex))
> DBUG_RETURN(TRUE);
>
> + /*
> + Statement-based replication of UPDATE ... LIMIT is not safe as
> + order of rows is not defined unless ORDER BY primary_key, so in
> + mixed mode we go to row-based.
> + */
> + if (thd->lex->current_select->select_limit &&
> + select_lex->select_limit->val_int() &&
> + !is_order_deterministic(table_list->table, order))
> + {
> + thd->lex->set_stmt_unsafe();
> + thd->set_current_stmt_binlog_row_based_if_mixed();
> + }
> +
> /* Check that we are not using table that we are updating in a sub select */
> {
> TABLE_LIST *duplicate;
>
>
>
> ------------------------------------------------------------------------
>
>
Attachment: [application/zip] binlog_unsafe_limit_test.zip
