List:Commits« Previous MessageNext Message »
From:Sven Sandberg Date:July 13 2010 7:02pm
Subject:Re: bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415
View as plain text  
Hi Zhenxing,

There is also a memory leak in is_order_deterministic(). It should call 
bitmap_free() on order_set and key_set.

/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