List:Commits« Previous MessageNext Message »
From:kroki Date:January 30 2007 1:20pm
Subject:bk commit into 5.0 tree (kroki:1.2387) BUG#21483
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of tomash. When tomash does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-01-30 16:20:17+03:00, kroki@stripped +6 -0
  BUG#21483: Server abort or deadlock on INSERT DELAYED with another
             implicit insert
  
  If the statement operates on several tables via functions or triggers,
  it is vital to execute all operations at one time, none of the actions
  can be delayed, because all tables should be locked simultaneously.
  
  The solution is to downgrade INSERT DELAYED to normal INSERT if the
  statement uses functions that access tables or triggers, or is called
  from a function or a trigger.  This also fixes bug#20497 (Trigger with
  INSERT DELAYED causes Error 1165) and bug#21714 (Wrong NEW.value and
  server abort on INSERT DELAYED to a table with a trigger).
  
  REPLACE DELAYED is handled by the same code.

  mysql-test/r/insert.result@stripped, 2007-01-30 16:20:14+03:00, kroki@stripped +116 -0
    Add result for bug#21483: Server abort or deadlock on INSERT DELAYED
    with another implicit insert.

  mysql-test/t/insert.test@stripped, 2007-01-30 16:20:14+03:00, kroki@stripped +146 -0
    Add test case for bug#21483: Server abort or deadlock on INSERT DELAYED
    with another implicit insert.

  sql/mysql_priv.h@stripped, 2007-01-30 16:20:14+03:00, kroki@stripped +4 -1
    Change prototype of open_and_lock_tables() to take
    open_and_lock_tables_func and additional argument for it.

  sql/sp_head.cc@stripped, 2007-01-30 16:20:15+03:00, kroki@stripped +7 -0
    Table list from sp_head::merge_table_list() is used only in prelocked
    mode, so we downgrade any TL_WRITE_DELAYED to TL_WRITE.

  sql/sql_base.cc@stripped, 2007-01-30 16:20:15+03:00, kroki@stripped +15 -5
    Change open_and_lock_tables() to accept a callback function
    open_and_lock_tables_func, which is called between open and lock
    phases.  This function may later the list of tables to be locked.

  sql/sql_insert.cc@stripped, 2007-01-30 16:20:15+03:00, kroki@stripped +98 -24
    If the statement is (or will be) executed in prelocked mode, we
    downgrade TL_WRITE_DELAYED lock type to TL_WRITE.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	kroki
# Host:	moonlight.home
# Root:	/home/tomash/src/mysql_ab/mysql-5.0-bug21483

--- 1.429/sql/mysql_priv.h	2007-01-30 16:20:26 +03:00
+++ 1.430/sql/mysql_priv.h	2007-01-30 16:20:26 +03:00
@@ -1035,7 +1035,10 @@ int init_ftfuncs(THD *thd, SELECT_LEX* s
 void wait_for_refresh(THD *thd);
 int open_tables(THD *thd, TABLE_LIST **tables, uint *counter, uint flags);
 int simple_open_n_lock_tables(THD *thd,TABLE_LIST *tables);
-bool open_and_lock_tables(THD *thd,TABLE_LIST *tables);
+typedef bool (*open_and_lock_tables_func)(void *arg, TABLE_LIST **tables,
+                                          uint *counter);
+bool open_and_lock_tables(THD *thd,TABLE_LIST *tables,
+                          open_and_lock_tables_func func = 0, void *arg = 0);
 bool open_normal_and_derived_tables(THD *thd, TABLE_LIST *tables, uint flags);
 int lock_tables(THD *thd, TABLE_LIST *tables, uint counter, bool *need_reopen);
 TABLE *open_temporary_table(THD *thd, const char *path, const char *db,

--- 1.363/sql/sql_base.cc	2007-01-30 16:20:26 +03:00
+++ 1.364/sql/sql_base.cc	2007-01-30 16:20:26 +03:00
@@ -2441,6 +2441,9 @@ int simple_open_n_lock_tables(THD *thd, 
     open_and_lock_tables()
     thd		- thread handler
     tables	- list of tables for open&locking
+    func        - function returning bool, to be called after open_tables()
+                  but before lock_tables()
+    arg         - argument to func
 
   RETURN
     FALSE - ok
@@ -2450,7 +2453,8 @@ int simple_open_n_lock_tables(THD *thd, 
     The lock will automaticaly be freed by close_thread_tables()
 */
 
-bool open_and_lock_tables(THD *thd, TABLE_LIST *tables)
+bool open_and_lock_tables(THD *thd, TABLE_LIST *tables,
+                          open_and_lock_tables_func func, void *arg)
 {
   uint counter;
   bool need_reopen;
@@ -2459,18 +2463,24 @@ bool open_and_lock_tables(THD *thd, TABL
   for ( ; ; ) 
   {
     if (open_tables(thd, &tables, &counter, 0))
-      DBUG_RETURN(-1);
-    if (!lock_tables(thd, tables, counter, &need_reopen))
+      DBUG_RETURN(TRUE);
+
+    TABLE_LIST *tables_to_lock = tables;
+    /* The call below may update tables_to_lock and counter. */
+    if (func && (*func)(arg, &tables_to_lock, &counter))
+      DBUG_RETURN(TRUE);
+
+    if (!lock_tables(thd, tables_to_lock, counter, &need_reopen))
       break;
     if (!need_reopen)
-      DBUG_RETURN(-1);
+      DBUG_RETURN(TRUE);
     close_tables_for_reopen(thd, &tables);
   }
   if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) ||
       (thd->fill_derived_tables() &&
        mysql_handle_derived(thd->lex, &mysql_derived_filling)))
     DBUG_RETURN(TRUE); /* purecov: inspected */
-  DBUG_RETURN(0);
+  DBUG_RETURN(FALSE);
 }
 
 

--- 1.213/sql/sql_insert.cc	2007-01-30 16:20:26 +03:00
+++ 1.214/sql/sql_insert.cc	2007-01-30 16:20:26 +03:00
@@ -305,6 +305,91 @@ void mark_fields_used_by_triggers_for_in
 }
 
 
+#ifndef EMBEDDED_LIBRARY
+/*
+  The structure lock_for_insert_delayed_state and function
+  lock_for_insert_delayed() below are the arguments passed to
+  open_and_lock_tables() in mysql_insert() below.  The reason for this
+  is the following: we can't use INSERT DELAYED if the statement
+  modifies several tables (via triggers or stored functions).
+  However, we might learn if the statement requires prelocking as late
+  as after open_tables() call.
+
+  So we pass a callback to open_and_lock_tables(), which is called
+  between open and lock phases, and that will check if the prelocking
+  will be used.  If so, then the requested lock is downgraded from
+  TL_WRITE_DELAYED to normal TL_WRITE.  If prelocking won't be used,
+  then we try to start an insert_delayed thread (we do that only once,
+  even if there are several attempts to lock the tables).  If the
+  thread has started successfully, then on return from
+  delayed_get_table() the first table (the one we INSERT DELAYED into)
+  is locked with TL_WRITE_DELAYED lock, so we exclude the first table
+  from the list of tables that open_and_lock_tables() should lock
+  then.
+*/
+
+struct lock_for_insert_delayed_state
+{
+  THD *thd;
+  TABLE *table;
+  thr_lock_type lock_type;
+  bool first_time;
+
+  lock_for_insert_delayed_state(THD *t)
+    : thd(t), table(NULL), lock_type(TL_WRITE_DELAYED), first_time(true)
+  {}
+};
+
+static
+bool lock_for_insert_delayed(void *arg, TABLE_LIST **tables, uint *counter)
+{
+  DBUG_ENTER("lock_for_insert_delayed");
+
+  lock_for_insert_delayed_state *state = (lock_for_insert_delayed_state*)arg;
+
+  if (state->first_time)
+  {
+    state->first_time = false;
+
+    DBUG_ASSERT(state->table == NULL);
+    DBUG_ASSERT(state->thd->net.last_errno != ER_WRONG_OBJECT);
+
+    if (!state->thd->lex->requires_prelocking())
+    {
+      /*
+        If not in prelocked mode, try to start or reinitialize INSERT
+        DELAYED thread.
+      */
+      state->table= delayed_get_table(state->thd, *tables);
+
+      if (state->table == NULL || state->thd->is_fatal_error)
+      {
+        if (state->thd->net.last_errno != ER_WRONG_OBJECT)
+          state->table= NULL;                /* Force normal insert */
+        else
+          DBUG_RETURN(TRUE);
+      }
+    }
+  }
+
+  if (state->table && !state->thd->lex->requires_prelocking())
+  {
+    DBUG_ASSERT(*counter > 0);
+
+    (*tables)->lock_type= state->lock_type= TL_WRITE_DELAYED;
+    *tables= (*tables)->next_global;
+    (*counter)--;
+  }
+  else
+  {
+    (*tables)->lock_type= state->lock_type= TL_WRITE;
+  }
+
+  DBUG_RETURN(FALSE);
+}
+#endif /* ! EMBEDDED_LIBRARY */
+
+
 bool mysql_insert(THD *thd,TABLE_LIST *table_list,
                   List<Item> &fields,
                   List<List_item> &values_list,
@@ -350,7 +435,8 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
 #else
   if ((lock_type == TL_WRITE_DELAYED &&
        ((specialflag & (SPECIAL_NO_NEW_FUNC | SPECIAL_SAFE_MODE)) ||
-	thd->slave_thread || !thd->variables.max_insert_delayed_threads)) ||
+        thd->slave_thread || !thd->variables.max_insert_delayed_threads ||
+        thd->prelocked_mode || thd->lex->requires_prelocking())) ||
       (lock_type == TL_WRITE_CONCURRENT_INSERT && duplic == DUP_REPLACE) ||
       (duplic == DUP_UPDATE))
     lock_type=TL_WRITE;
@@ -370,32 +456,20 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
 	DBUG_RETURN(TRUE);
       }
     }
-    if ((table= delayed_get_table(thd,table_list)) && !thd->is_fatal_error)
-    {
-      /*
-        Open tables used for sub-selects or in stored functions, will also
-        cache these functions.
-      */
-      res= open_and_lock_tables(thd, table_list->next_global);
-      /*
-	First is not processed by open_and_lock_tables() => we need set
-	updateability flags "by hands".
-      */
-      if (!table_list->derived && !table_list->view)
-        table_list->updatable= 1;  // usual table
-    }
-    else if (thd->net.last_errno != ER_WRONG_OBJECT)
-    {
-      /* Too many delayed insert threads;  Use a normal insert */
-      table_list->lock_type= lock_type= TL_WRITE;
-      res= open_and_lock_tables(thd, table_list);
-    }
+
+    lock_for_insert_delayed_state state(thd);
+    if (open_and_lock_tables(thd, table_list,
+                             lock_for_insert_delayed, &state))
+      DBUG_RETURN(TRUE);
+    table = state.table;
+    lock_type = state.lock_type;
   }
   else
 #endif /* EMBEDDED_LIBRARY */
-    res= open_and_lock_tables(thd, table_list);
-  if (res || thd->is_fatal_error)
-    DBUG_RETURN(TRUE);
+  {
+    if (open_and_lock_tables(thd, table_list))
+      DBUG_RETURN(TRUE);
+  }
 
   thd->proc_info="init";
   thd->used_tables=0;

--- 1.26/mysql-test/r/insert.result	2007-01-30 16:20:26 +03:00
+++ 1.27/mysql-test/r/insert.result	2007-01-30 16:20:26 +03:00
@@ -325,3 +325,119 @@ select row_count();
 row_count()
 1
 drop table t1;
+DROP TABLE IF EXISTS t1;
+DROP FUNCTION IF EXISTS f1;
+DROP FUNCTION IF EXISTS f2;
+CREATE TABLE t1 (i INT);
+CREATE FUNCTION f1() RETURNS INT
+BEGIN
+INSERT INTO t1 VALUES (1);
+RETURN 1;
+END |
+CREATE FUNCTION f2() RETURNS INT
+BEGIN
+INSERT DELAYED INTO t1 VALUES (2);
+RETURN 1;
+END |
+SELECT f1();
+f1()
+1
+SELECT f2();
+f2()
+1
+INSERT INTO t1 VALUES (3);
+INSERT DELAYED INTO t1 VALUES (4);
+INSERT INTO t1 VALUES (f1());
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+INSERT DELAYED INTO t1 VALUES (f1());
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+INSERT INTO t1 VALUES (f2());
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+INSERT DELAYED INTO t1 VALUES (f2());
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+INSERT INTO t1 VALUES (NEW.i);
+INSERT INTO t1 VALUES (1);
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+INSERT DELAYED INTO t1 VALUES (1);
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+SELECT * FROM t1;
+i
+1
+2
+3
+4
+DROP FUNCTION f2;
+DROP FUNCTION f1;
+DROP TABLE t1;
+DROP TABLE IF EXISTS t1, t2;
+CREATE TABLE t1 (i INT);
+CREATE TABLE t2 (i INT);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+INSERT DELAYED INTO t2 VALUES (NEW.i);
+CREATE TRIGGER t1_bu BEFORE UPDATE ON t1 FOR EACH ROW
+INSERT DELAYED INTO t2 VALUES (NEW.i);
+CREATE TRIGGER t1_bd BEFORE DELETE ON t1 FOR EACH ROW
+INSERT DELAYED INTO t2 VALUES (OLD.i);
+INSERT INTO t1 VALUES (1);
+INSERT DELAYED INTO t1 VALUES (2);
+SELECT * FROM t1;
+i
+1
+2
+UPDATE t1 SET i = 3 WHERE i = 1;
+SELECT * FROM t1;
+i
+3
+2
+DELETE FROM t1 WHERE i = 3;
+SELECT * FROM t1;
+i
+2
+SELECT * FROM t2;
+i
+1
+2
+3
+3
+DROP TABLE t1, t2;
+DROP TABLE IF EXISTS t1, t2;
+CREATE TABLE t1 (i INT);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+SET @a= NEW.i;
+SET @a= 0;
+INSERT DELAYED INTO t1 VALUES (1);
+SELECT @a;
+@a
+1
+INSERT DELAYED INTO t1 VALUES (2);
+SELECT @a;
+@a
+2
+DROP TABLE t1;
+CREATE TABLE t1 (i INT);
+CREATE TABLE t2 (i INT);
+CREATE TRIGGER t1_ai AFTER INSERT ON t1 FOR EACH ROW
+INSERT INTO t2 VALUES (NEW.i);
+CREATE TRIGGER t1_au AFTER UPDATE ON t1 FOR EACH ROW
+INSERT DELAYED INTO t2 VALUES (NEW.i);
+CREATE TRIGGER t1_ad AFTER DELETE ON t1 FOR EACH ROW
+INSERT DELAYED INTO t2 VALUES (OLD.i);
+INSERT DELAYED INTO t1 VALUES (1);
+SELECT * FROM t1;
+i
+1
+UPDATE t1 SET i = 2 WHERE i = 1;
+SELECT * FROM t1;
+i
+2
+DELETE FROM t1 WHERE i = 2;
+SELECT * FROM t1;
+i
+SELECT * FROM t2;
+i
+1
+2
+2
+DROP TABLE t1, t2;
+End of 5.0 tests.

--- 1.24/mysql-test/t/insert.test	2007-01-30 16:20:26 +03:00
+++ 1.25/mysql-test/t/insert.test	2007-01-30 16:20:26 +03:00
@@ -198,3 +198,149 @@ select row_count();
 insert into t1 values (5, 5) on duplicate key update data= data + 10;
 select row_count();
 drop table t1;
+
+
+#
+# BUG#21483: Server abort or deadlock on INSERT DELAYED with another
+# implicit insert 
+#
+# The solution is to downgrade INSERT DELAYED to normal INSERT if the
+# statement uses functions and access tables or triggers, or is called
+# from a function or a trigger.
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+DROP FUNCTION IF EXISTS f1;
+DROP FUNCTION IF EXISTS f2;
+--enable_warnings
+
+CREATE TABLE t1 (i INT);
+delimiter |;
+CREATE FUNCTION f1() RETURNS INT
+BEGIN
+  INSERT INTO t1 VALUES (1);
+  RETURN 1;
+END |
+CREATE FUNCTION f2() RETURNS INT
+BEGIN
+  INSERT DELAYED INTO t1 VALUES (2);
+  RETURN 1;
+END |
+delimiter ;|
+
+SELECT f1();
+SELECT f2();
+INSERT INTO t1 VALUES (3);
+INSERT DELAYED INTO t1 VALUES (4);
+
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT INTO t1 VALUES (f1());
+
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT DELAYED INTO t1 VALUES (f1());
+
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT INTO t1 VALUES (f2());
+
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT DELAYED INTO t1 VALUES (f2());
+
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+  INSERT INTO t1 VALUES (NEW.i);
+
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT INTO t1 VALUES (1);
+
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+INSERT DELAYED INTO t1 VALUES (1);
+
+SELECT * FROM t1;
+
+DROP FUNCTION f2;
+DROP FUNCTION f1;
+DROP TABLE t1;
+
+
+#
+# BUG#20497: Trigger with INSERT DELAYED causes Error 1165
+#
+# The solution is to downgrade INSERT DELAYED to normal INSERT if the
+# statement uses functions and access tables or triggers, or is called
+# from a function or a trigger.
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+
+CREATE TABLE t1 (i INT);
+CREATE TABLE t2 (i INT);
+
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+  INSERT DELAYED INTO t2 VALUES (NEW.i);
+
+CREATE TRIGGER t1_bu BEFORE UPDATE ON t1 FOR EACH ROW
+  INSERT DELAYED INTO t2 VALUES (NEW.i);
+
+CREATE TRIGGER t1_bd BEFORE DELETE ON t1 FOR EACH ROW
+  INSERT DELAYED INTO t2 VALUES (OLD.i);
+
+INSERT INTO t1 VALUES (1);
+INSERT DELAYED INTO t1 VALUES (2);
+SELECT * FROM t1;
+UPDATE t1 SET i = 3 WHERE i = 1;
+SELECT * FROM t1;
+DELETE FROM t1 WHERE i = 3;
+SELECT * FROM t1;
+SELECT * FROM t2;
+
+DROP TABLE t1, t2;
+
+
+#
+# BUG#21714: Wrong NEW.value and server abort on INSERT DELAYED to a
+# table with a trigger 
+#
+# The solution is to downgrade INSERT DELAYED to normal INSERT if the
+# statement uses functions and access tables or triggers, or is called
+# from a function or a trigger.
+#
+--disable_warnings
+DROP TABLE IF EXISTS t1, t2;
+--enable_warnings
+
+CREATE TABLE t1 (i INT);
+CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW
+  SET @a= NEW.i;
+
+SET @a= 0;
+INSERT DELAYED INTO t1 VALUES (1);
+SELECT @a;
+INSERT DELAYED INTO t1 VALUES (2);
+SELECT @a;
+
+DROP TABLE t1;
+
+CREATE TABLE t1 (i INT);
+CREATE TABLE t2 (i INT);
+
+CREATE TRIGGER t1_ai AFTER INSERT ON t1 FOR EACH ROW
+  INSERT INTO t2 VALUES (NEW.i);
+
+CREATE TRIGGER t1_au AFTER UPDATE ON t1 FOR EACH ROW
+  INSERT DELAYED INTO t2 VALUES (NEW.i);
+
+CREATE TRIGGER t1_ad AFTER DELETE ON t1 FOR EACH ROW
+  INSERT DELAYED INTO t2 VALUES (OLD.i);
+
+INSERT DELAYED INTO t1 VALUES (1);
+SELECT * FROM t1;
+UPDATE t1 SET i = 2 WHERE i = 1;
+SELECT * FROM t1;
+DELETE FROM t1 WHERE i = 2;
+SELECT * FROM t1;
+SELECT * FROM t2;
+
+DROP TABLE t1, t2;
+
+
+--echo End of 5.0 tests.

--- 1.230/sql/sp_head.cc	2007-01-30 16:20:26 +03:00
+++ 1.231/sql/sp_head.cc	2007-01-30 16:20:26 +03:00
@@ -3443,6 +3443,13 @@ sp_head::merge_table_list(THD *thd, TABL
       tname[tlen]= '\0';
 
       /*
+        Downgrade the lock type because this table list will be used
+        only in prelocked mode.
+      */
+      if (table->lock_type == TL_WRITE_DELAYED)
+        table->lock_type= TL_WRITE;
+
+      /*
         We ignore alias when we check if table was already marked as temporary
         (and therefore should not be prelocked). Otherwise we will erroneously
         treat table with same name but with different alias as non-temporary.
Thread
bk commit into 5.0 tree (kroki:1.2387) BUG#21483kroki30 Jan