Thank you very much!
Sven Sandberg wrote:
> 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;
> >
> >
> >
> > ------------------------------------------------------------------------
> >
> >
>
