bk commit into 5.0 tree (dlenev:1.2250) BUG#20670

[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@stripped, 2006-09-08 14:28:59+04:00, dlenev@stripped +7 -0
  Proposed fix for bug#20670 "UPDATE using key and invoking trigger that
  modifies this key does not stop" (version for 5.0 only).
  
  UPDATE statement which WHERE clause used key and which invoked trigger
  that modified field in this key worked indefinetely.
  
  This problem occured because in cases when UPDATE statement was
  executed in update-on-the-fly mode (in which row is updated right
  during evaluation of select for WHERE clause) the new version of
  the row became visible to select representing WHERE clause and was
  updated again and again.
  We already solve this problem for UPDATE statements which does not
  invoke triggers by detecting the fact that we are going to update
  field in key used for scanning and performing update in two steps,
  during the first step we gather information about the rows to be
  updated and then doing actual updates. We also do this for
  MULTI-UPDATE and in its case we even detect situation when such
  fields are updated in triggers (actually we simply assume that
  we always update fields used in key if we have before update
  trigger).
  
  The fix simply extends this check which is done in check_if_key_used()/
  QUICK_SELECT_I::check_if_keys_used() routine/method in such way that
  it also detects cases when field used in key is updated in trigger.
  As nice side-effect we have more precise and thus more optimal
  perfomance-wise check for the MULTI-UPDATE.
  
  Note that this check will be implemented in much more elegant way in 5.1 

  mysql-test/r/trigger.result@stripped, 2006-09-08 14:28:56+04:00, dlenev@stripped +12
-0
    Added test case for bug#20670 "UPDATE using key and invoking trigger that
    modifies this key does not stop".

  mysql-test/t/trigger.test@stripped, 2006-09-08 14:28:56+04:00, dlenev@stripped +19
-0
    Added test case for bug#20670 "UPDATE using key and invoking trigger that
    modifies this key does not stop".

  sql/key.cc@stripped, 2006-09-08 14:28:56+04:00, dlenev@stripped +18 -2
    Now check_if_key_used() also checks if key uses field which can be updated
    by before update trigger defined on the table. As result we avoid using
    update-on-the-fly method in cases when trigger updates part of key which
    is used by select which filters rows to be updated and thus avoid infinite
    updates. By doing such check here we cover both UPDATE and MULTI-UPDATE
    cases.

  sql/opt_range.h@stripped, 2006-09-08 14:28:56+04:00, dlenev@stripped +1 -0
    Updated comment describing QUICK_SELECT_I::check_if_keys_used() method
    to reflect its extended semantics (this change was caused by change in
    check_if_key_used() routine semantics).

  sql/sql_trigger.cc@stripped, 2006-09-08 14:28:57+04:00, dlenev@stripped +32 -0
    Introduced Table_triggers_list::check_if_updated_in_before_update_triggers()
    method which is needed for checking if field of subject table can be changed
    in before update trigger.

  sql/sql_trigger.h@stripped, 2006-09-08 14:28:57+04:00, dlenev@stripped +2 -5
    Table_triggers_list:
      Removed has_before_update_triggers() method which is not used any longer.
      Added declaration of check_if_updated_in_before_update_triggers() which
      is needed for checking if field of subject table can be changed by before
      update trigger.

  sql/sql_update.cc@stripped, 2006-09-08 14:28:57+04:00, dlenev@stripped +3 -9
    safe_update_on_fly():
      Now cases when trigger updates fields which are part of key used for
      filtering rows for update are caught directly in check_if_key_used().
      This also allows to cover both UPDATE and MULTI-UPDATE cases.

# 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:	mockturtle.local
# Root:	/home/dlenev/src/mysql-5.0-bg20670-2

--- 1.36/sql/key.cc	2006-09-08 14:29:04 +04:00
+++ 1.37/sql/key.cc	2006-09-08 14:29:04 +04:00
@@ -18,6 +18,7 @@
 /* Functions to handle keys and fields in forms */
 
 #include "mysql_priv.h"
+#include "sql_trigger.h"
 
 	/*
 	** Search after with key field is. If no key starts with field test
@@ -342,12 +343,24 @@ void key_unpack(String *to,TABLE *table,
 
 
 /*
-  Return 1 if any field in a list is part of key or the key uses a field
-  that is automaticly updated (like a timestamp)
+  Check if key uses field that is listed in passed field list or is
+  automatically updated (like a timestamp) or can be updated by before
+  update trigger defined on the table.
+
+  SYNOPSIS
+    check_if_key_used()
+      table   TABLE object with which keys and fields are associated.
+      idx     Key to be checked.
+      fields  List of fields to be checked.
+
+  RETURN VALUE
+    TRUE   Key uses field which meets one the above conditions
+    FALSE  Otherwise
 */
 
 bool check_if_key_used(TABLE *table, uint idx, List<Item> &fields)
 {
+  Table_triggers_list *triggers= table->triggers;
   List_iterator_fast<Item> f(fields);
   KEY_PART_INFO *key_part,*key_part_end;
   for (key_part=table->key_info[idx].key_part,key_part_end=key_part+
@@ -366,6 +379,9 @@ bool check_if_key_used(TABLE *table, uin
       if (key_part->field->eq(field->field))
 	return 1;
     }
+    if (triggers &&
+        triggers->check_if_updated_in_before_update_triggers(key_part->field))
+      return 1;
   }
 
   /*

--- 1.59/sql/opt_range.h	2006-09-08 14:29:04 +04:00
+++ 1.60/sql/opt_range.h	2006-09-08 14:29:04 +04:00
@@ -225,6 +225,7 @@ public:
     Return 1 if any index used by this quick select
      a) uses field that is listed in passed field list or
      b) is automatically updated (like a timestamp)
+     c) can be updated by one of before update triggers defined on table
   */
   virtual bool check_if_keys_used(List<Item> *fields);
 

--- 1.194/sql/sql_update.cc	2006-09-08 14:29:04 +04:00
+++ 1.195/sql/sql_update.cc	2006-09-08 14:29:04 +04:00
@@ -1188,21 +1188,15 @@ static bool safe_update_on_fly(JOIN_TAB 
     return TRUE;				// At most one matching row
   case JT_REF:
   case JT_REF_OR_NULL:
-    return !check_if_key_used(table, join_tab->ref.key, *fields) &&
-           !(table->triggers &&
-             table->triggers->has_before_update_triggers());
+    return !check_if_key_used(table, join_tab->ref.key, *fields);
   case JT_ALL:
     /* If range search on index */
     if (join_tab->quick)
-      return !join_tab->quick->check_if_keys_used(fields) &&
-             !(table->triggers &&
-               table->triggers->has_before_update_triggers());
+      return !join_tab->quick->check_if_keys_used(fields);
     /* If scanning in clustered key */
     if ((table->file->table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) &&
 	table->s->primary_key < MAX_KEY)
-      return !check_if_key_used(table, table->s->primary_key, *fields) &&
-             !(table->triggers &&
-               table->triggers->has_before_update_triggers());
+      return !check_if_key_used(table, table->s->primary_key, *fields);
     return TRUE;
   default:
     break;					// Avoid compler warning

--- 1.45/mysql-test/r/trigger.result	2006-09-08 14:29:04 +04:00
+++ 1.46/mysql-test/r/trigger.result	2006-09-08 14:29:04 +04:00
@@ -1173,4 +1173,16 @@ TRIGGER t2_bi BEFORE INSERT ON t2 FOR EA
 ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghijQWERTY'
is too long for host name (should be no longer than 60)
 DROP TABLE t1;
 DROP TABLE t2;
+drop table if exists t1;
+create table t1 (i int, j int key);
+insert into t1 values (1,1), (2,2), (3,3);
+create trigger t1_bu before update on t1 for each row
+set new.j = new.j + 10;
+update t1 set i= i+ 10 where j > 2;
+select * from t1;
+i	j
+1	1
+2	2
+13	13
+drop table t1;
 End of 5.0 tests

--- 1.51/mysql-test/t/trigger.test	2006-09-08 14:29:04 +04:00
+++ 1.52/mysql-test/t/trigger.test	2006-09-08 14:29:04 +04:00
@@ -1421,4 +1421,23 @@ DROP TABLE t1;
 DROP TABLE t2;
 
 
+#
+# Bug#20670 "UPDATE using key and invoking trigger that modifies
+#            this key does not stop"
+#
+
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+create table t1 (i int, j int key);
+insert into t1 values (1,1), (2,2), (3,3);
+create trigger t1_bu before update on t1 for each row
+  set new.j = new.j + 10;
+# This should not work indefinitely and should cause
+# expected result
+update t1 set i= i+ 10 where j > 2;
+select * from t1;
+drop table t1;
+
+
 --echo End of 5.0 tests

--- 1.55/sql/sql_trigger.cc	2006-09-08 14:29:04 +04:00
+++ 1.56/sql/sql_trigger.cc	2006-09-08 14:29:04 +04:00
@@ -1548,6 +1548,38 @@ void Table_triggers_list::mark_fields_us
 
 
 /*
+  Check if field of subject table can be changed in before update trigger.
+
+  SYNOPSIS
+    check_if_updated_in_before_update_triggers()
+      field  Field object for field to be checked
+
+  NOTE
+    Field passed to this function should be bound to the same
+    TABLE object as Table_triggers_list.
+
+  RETURN VALUE
+    TRUE   Field is changed
+    FALSE  Otherwise
+*/
+
+bool Table_triggers_list::check_if_updated_in_before_update_triggers(Field *fld)
+{
+  Item_trigger_field *trg_fld;
+  for (trg_fld= trigger_fields[TRG_EVENT_UPDATE][TRG_ACTION_BEFORE];
+       trg_fld != 0;
+       trg_fld= trg_fld->next_trg_field)
+  {
+    if (trg_fld->get_settable_routine_parameter() &&
+        trg_fld->field_idx != (uint)-1 &&
+        table->field[trg_fld->field_idx]->eq(fld))
+      return TRUE;
+  }
+  return FALSE;
+}
+
+
+/*
   Trigger BUG#14090 compatibility hook
 
   SYNOPSIS

--- 1.21/sql/sql_trigger.h	2006-09-08 14:29:04 +04:00
+++ 1.22/sql/sql_trigger.h	2006-09-08 14:29:04 +04:00
@@ -116,14 +116,11 @@ public:
             bodies[TRG_EVENT_DELETE][TRG_ACTION_AFTER]);
   }
 
-  bool has_before_update_triggers()
-  {
-    return test(bodies[TRG_EVENT_UPDATE][TRG_ACTION_BEFORE]);
-  }
-
   void set_table(TABLE *new_table);
 
   void mark_fields_used(THD *thd, trg_event_type event);
+
+  bool check_if_updated_in_before_update_triggers(Field *fld);
 
   friend class Item_trigger_field;
   friend int sp_cache_routines_and_add_tables_for_triggers(THD *thd, LEX *lex,