bk commit into 5.0 tree (dlenev:1.2082) BUG#17764

[dlenev]

Below is the list of changes that have just been committed into a local
5.0 repository of dlenev. When dlenev 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
  1.2082 06/03/18 21:39:34 dlenev@stripped +6 -0
  Proposed fix for bug #17764 "Trigger crashes MyISAM table"
  
  Table was reported as crashed when it was subject table of trigger invoked by
  insert statement which was executed with enabled bulk insert mode (which is
  actually set of optimizations enabled by handler::start_bulk_insert()) and
  this trigger also explicitly referenced it.
  The same problem arose when table to which bulk insert was done was also
  referenced in function called by insert statement.
  
  This fix solves this problem in the following way - now when we open table
  in prelocked mode we also check that there is no bulk insert operation in
  progress for it (started by one of the calling statements), if we find such
  table instance we end it.
  
  QQ: Will this fix work fine for NDB the only handler except MyISAM which
      really supports bulk insert? 

  sql/table.h
    1.126 06/03/18 21:39:30 dlenev@stripped +6 -0
    Added TABLE::bulk_insert member to be able easily identify instances of
    tables for which bulk insert operation is in progress.

  sql/sql_load.cc
    1.92 06/03/18 21:39:30 dlenev@stripped +3 -1
    Mark TABLE objects for which we start bulk insert operation and be ready
    that it will be ended somewhen in the middle of insert by function or
    trigger which access to the same table.

  sql/sql_insert.cc
    1.184 06/03/18 21:39:30 dlenev@stripped +17 -3
    Mark TABLE objects for which we start bulk insert operation and be ready
    that it will be ended somewhen in the middle of insert by function or
    trigger which access to the same table.

  sql/sql_base.cc
    1.331 06/03/18 21:39:30 dlenev@stripped +34 -17
    open_table():
      Now when we open table in prelocked mode we also check that there is
      no bulk insert operation in progress for it (started by one of the
      calling statements), if we find such table instance we end it.
      This allows us to access tables to which we insert data in triggers
      invoked by this operation or in functions used by it.

  mysql-test/t/trigger.test
    1.38 06/03/18 21:39:29 dlenev@stripped +34 -0
    Added test bug #17764 "Trigger crashes MyISAM table".

  mysql-test/r/trigger.result
    1.33 06/03/18 21:39:29 dlenev@stripped +24 -0
    Added test bug #17764 "Trigger crashes MyISAM table".

# 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:	dlenev
# Host:	jabberwock.site
# Root:	/home/dlenev/mysql-5.0-bg17764

--- 1.330/sql/sql_base.cc	2006-03-06 12:41:16 +03:00
+++ 1.331/sql/sql_base.cc	2006-03-18 21:39:30 +03:00
@@ -1139,26 +1139,41 @@
   {						// Using table locks
     TABLE *best_table= 0;
     int best_distance= INT_MIN;
-    bool check_if_used= thd->prelocked_mode &&
-                        ((int) table_list->lock_type >=
-                         (int) TL_WRITE_ALLOW_WRITE);
     for (table=thd->open_tables; table ; table=table->next)
     {
       if (table->s->key_length == key_length &&
           !memcmp(table->s->table_cache_key, key, key_length))
       {
-        if (check_if_used && table->query_id &&
+        if (thd->prelocked_mode && table->query_id &&
             table->query_id != thd->query_id)
         {
-          /*
-            If we are in stored function or trigger we should ensure that
-            we won't change table that is already used by calling statement.
-            So if we are opening table for writing, we should check that it
-            is not already open by some calling stamement.
-          */
-          my_error(ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG, MYF(0),
-                   table->s->table_name);
-          DBUG_RETURN(0);
+          if ((int) table_list->lock_type >= (int) TL_WRITE_ALLOW_WRITE)
+          {
+            /*
+              If we are in stored function or trigger we should ensure that
+              we won't change table that is already used by calling statement.
+              So if we are opening table for writing, we should check that it
+              is not already open by some calling stamement.
+            */
+            my_error(ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG, MYF(0),
+                     table->s->table_name);
+            DBUG_RETURN(0);
+          }
+          if (table->bulk_insert)
+          {
+            /*
+              Also if we are going to read from table and one of calling
+              statements have started bulk inserts into it we should end
+              this bulk insert.
+
+              QQ: Is not it akward to emit error here ?
+                  Also code in ha_ndbcluster::end_bulk_insert() looks
+                  a bit scary...
+            */
+            if (table->file->end_bulk_insert())
+              table->file->print_error(my_errno, MYF(0));
+            table->bulk_insert= FALSE;
+          }
         }
         if (!my_strcasecmp(system_charset_info, table->alias, alias) &&
             table->query_id != thd->query_id && /* skip tables already used
*/
@@ -1183,12 +1198,14 @@
           {
             best_distance= distance;
             best_table= table;
-            if (best_distance == 0 && !check_if_used)
+            if (best_distance == 0 && !thd->prelocked_mode)
             {
               /*
-                If we have found perfect match and we don't need to check that
-                table is not used by one of calling statements (assuming that
-                we are inside of function or trigger) we can finish iterating
+                If we have found perfect match and we don't need to iterate
+                through all instances of this table in order to check that
+                table is not used by one of calling statements or to stop
+                bulk inserts started by one of them (assuming that we are
+                inside of function or trigger) we can finish iterating
                 through open tables list.
               */
               break;

--- 1.183/sql/sql_insert.cc	2006-01-06 19:36:10 +03:00
+++ 1.184/sql/sql_insert.cc	2006-03-18 21:39:30 +03:00
@@ -409,7 +409,10 @@
     the code to make the call of end_bulk_insert() below safe.
   */
   if (lock_type != TL_WRITE_DELAYED)
+  {
     table->file->start_bulk_insert(values_list.elements);
+    table->bulk_insert= TRUE;
+  }
 
   thd->no_trans_update= 0;
   thd->abort_on_warning= (!ignore &&
@@ -534,11 +537,12 @@
   else
 #endif
   {
-    if (table->file->end_bulk_insert() && !error)
+    if (table->bulk_insert && table->file->end_bulk_insert() &&
!error)
     {
       table->file->print_error(my_errno,MYF(0));
       error=1;
     }
+    table->bulk_insert= FALSE;
     if (id && values_list.elements != 1)
       thd->insert_id(id);			// For update log
     else if (table->next_number_field && info.copied)
@@ -2191,6 +2195,7 @@
       in select_insert::prepare2().
     */
     table->file->start_bulk_insert((ha_rows) 0);
+    table->bulk_insert= TRUE;
   }
   restore_record(table,s->default_values);		// Get empty record
   table->next_number_field=table->found_next_number_field;
@@ -2230,7 +2235,10 @@
 {
   DBUG_ENTER("select_insert::prepare2");
   if (thd->lex->current_select->options & OPTION_BUFFER_RESULT)
+  {
     table->file->start_bulk_insert((ha_rows) 0);
+    table->bulk_insert= TRUE;
+  }
   DBUG_RETURN(0);
 }
 
@@ -2332,7 +2340,11 @@
     */
     DBUG_VOID_RETURN;
   }
-  table->file->end_bulk_insert();
+  if (table->bulk_insert)
+  {
+    table->file->end_bulk_insert();
+    table->bulk_insert= FALSE;
+  }
   /*
     If at least one row has been inserted/modified and will stay in the table
     (the table doesn't have transactions) (example: we got a duplicate key
@@ -2367,7 +2379,8 @@
   int error,error2;
   DBUG_ENTER("select_insert::send_eof");
 
-  error=table->file->end_bulk_insert();
+  error= table->bulk_insert ? table->file->end_bulk_insert() : 0;
+  table->bulk_insert= FALSE;
   table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
 
   /*
@@ -2451,6 +2464,7 @@
   if (info.ignore || info.handle_duplicates != DUP_ERROR)
     table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
   table->file->start_bulk_insert((ha_rows) 0);
+  table->bulk_insert= TRUE;
   thd->no_trans_update= 0;
   thd->abort_on_warning= (!info.ignore &&
                           (thd->variables.sql_mode &

--- 1.91/sql/sql_load.cc	2006-02-07 02:06:16 +03:00
+++ 1.92/sql/sql_load.cc	2006-03-18 21:39:30 +03:00
@@ -357,6 +357,7 @@
 	handle_duplicates == DUP_REPLACE)
       table->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
     table->file->start_bulk_insert((ha_rows) 0);
+    table->bulk_insert= TRUE;
     table->copy_blobs=1;
 
     thd->no_trans_update= 0;
@@ -373,11 +374,12 @@
       error= read_sep_field(thd, info, table_list, fields_vars,
                             set_fields, set_values, read_info,
 			    *enclosed, skip_lines, ignore);
-    if (table->file->end_bulk_insert() && !error)
+    if (table->bulk_insert && table->file->end_bulk_insert() &&
!error)
     {
       table->file->print_error(my_errno, MYF(0));
       error= 1;
     }
+    table->bulk_insert= FALSE;
     table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
     table->next_number_field=0;
   }

--- 1.125/sql/table.h	2006-03-06 12:11:04 +03:00
+++ 1.126/sql/table.h	2006-03-18 21:39:30 +03:00
@@ -269,6 +269,12 @@
   my_bool auto_increment_field_not_null;
   my_bool insert_or_update;             /* Can be used by the handler */
   my_bool alias_name_used;		/* true if table_name is alias */
+  /*
+    TRUE when there is bulk insert in progress for this instance of table.
+    Note that under bulk insert here we mean set of optimizations enabled
+    by handler::start_bulk_insert().
+  */
+  my_bool bulk_insert;                  
 
   REGINFO reginfo;			/* field connections */
   MEM_ROOT mem_root;

--- 1.32/mysql-test/r/trigger.result	2006-03-04 16:54:59 +03:00
+++ 1.33/mysql-test/r/trigger.result	2006-03-18 21:39:29 +03:00
@@ -2,6 +2,7 @@
 drop view if exists v1;
 drop database if exists mysqltest;
 drop function if exists f1;
+drop function if exists f2;
 drop procedure if exists p1;
 create table t1 (i int);
 create trigger trg before insert on t1 for each row set @a:=1;
@@ -897,3 +898,26 @@
 ERROR 42000: RETURN is only allowed in a FUNCTION
 insert into t1 values (1);
 drop table t1;
+create table t1 (a varchar(64), b int);
+create table t2 like t1;
+create trigger t1_ai after insert on t1 for each row
+set @a:= (select max(a) from t1);
+insert into t1 (a) values
+("Twas"),("brillig"),("and"),("the"),("slithy"),("toves"),
+("Did"),("gyre"),("and"),("gimble"),("in"),("the"),("wabe");
+create trigger t2_ai after insert on t2 for each row
+set @a:= (select max(a) from t2);
+insert into t2 select * from t1;
+load data infile '../std_data_ln/words.dat' into table t1 (a);
+drop trigger t1_ai;
+drop trigger t2_ai;
+create function f1() returns int return (select max(b) from t1);
+insert into t1 values
+("All",f1()),("mimsy",f1()),("were",f1()),("the",f1()),("borogoves",f1()),
+("And",f1()),("the",f1()),("mome", f1()),("raths",f1()),("outgrabe",f1());
+create function f2() returns int return (select max(b) from t2);
+insert into t2 select a, f2() from t1;
+load data infile '../std_data_ln/words.dat' into table t1 (a) set b:= f1();
+drop table t1;
+drop function f1;
+drop function f2;

--- 1.37/mysql-test/t/trigger.test	2006-03-04 16:54:59 +03:00
+++ 1.38/mysql-test/t/trigger.test	2006-03-18 21:39:29 +03:00
@@ -7,6 +7,7 @@
 drop view if exists v1;
 drop database if exists mysqltest;
 drop function if exists f1;
+drop function if exists f2;
 drop procedure if exists p1;
 --enable_warnings
 
@@ -1057,3 +1058,36 @@
 create trigger t1_bi before insert on t1 for each row return 0;
 insert into t1 values (1);
 drop table t1;
+
+# Test for bug #17764 "Trigger crashes MyISAM table"
+#
+# Table was reported as crashed when it was subject table of trigger invoked
+# by insert statement which was executed with enabled bulk insert mode (which
+# is actually set of optimizations enabled by handler::start_bulk_insert())
+# and this trigger also explicitly referenced it.
+# The same problem arose when table to which bulk insert was done was also
+# referenced in function called by insert statement.
+create table t1 (a varchar(64), b int);
+create table t2 like t1;
+create trigger t1_ai after insert on t1 for each row
+  set @a:= (select max(a) from t1);
+insert into t1 (a) values
+  ("Twas"),("brillig"),("and"),("the"),("slithy"),("toves"),
+  ("Did"),("gyre"),("and"),("gimble"),("in"),("the"),("wabe");
+create trigger t2_ai after insert on t2 for each row
+  set @a:= (select max(a) from t2);
+insert into t2 select * from t1;
+load data infile '../std_data_ln/words.dat' into table t1 (a);
+drop trigger t1_ai;
+drop trigger t2_ai;
+# Test that the problem for functions is fixed as well
+create function f1() returns int return (select max(b) from t1);
+insert into t1 values
+  ("All",f1()),("mimsy",f1()),("were",f1()),("the",f1()),("borogoves",f1()),
+  ("And",f1()),("the",f1()),("mome", f1()),("raths",f1()),("outgrabe",f1());
+create function f2() returns int return (select max(b) from t2);
+insert into t2 select a, f2() from t1;
+load data infile '../std_data_ln/words.dat' into table t1 (a) set b:= f1();
+drop table t1;
+drop function f1;
+drop function f2;