List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:July 15 2010 7:16am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418)
Bug#42415
View as plain text  
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;
> > 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > 
> 


Thread
bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415He Zhenxing11 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418)Bug#42415He Zhenxing14 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
  • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415Sven Sandberg13 Jul
    • Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418)Bug#42415He Zhenxing15 Jul