List:Commits« Previous MessageNext Message »
From:He Zhenxing Date:July 11 2010 1:30pm
Subject:bzr commit into mysql-5.1-bugteam branch (zhenxing.he:3418) Bug#42415
View as plain text  
#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: [text/bzr-bundle] bzr/zhenxing.he@sun.com-20100711133046-5g7y5ziyg2qcf0hu.bundle
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