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.
Also check_if_key_used()/QUICK_SELECT_I::check_if_keys_used() were
renamed to is_key_used()/QUICK_SELECT_I::is_keys_used() in order to
better reflect that boolean predicate.

Note that this check is implemented in much more elegant way in 5.1 

mysql-test/r/trigger.result

@@ -1173,4 +1173,16 @@ TRIGGER t2_bi BEFORE INSERT ON t2 FOR EACH ROW SET @a = 2;
 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

mysql-test/t/trigger.test

@@ -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

sql/key.cc

@@ -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,uint idx)
 
 
 /*
-  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
+    is_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)
+bool is_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, uint idx, List<Item> &fields)
       if (key_part->field->eq(field->field))
 	return 1;
     }
+    if (triggers &&
+        triggers->is_updated_in_before_update_triggers(key_part->field))
+      return 1;
   }
 
   /*
@@ -374,7 +390,7 @@ bool check_if_key_used(TABLE *table, uint idx, List<Item> &fields)
   */
   if (idx != table->s->primary_key && table->s->primary_key < MAX_KEY &&
       (table->file->table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX))
-    return check_if_key_used(table, table->s->primary_key, fields);
+    return is_key_used(table, table->s->primary_key, fields);
   return 0;
 }
 

sql/mysql_priv.h

@@ -1101,7 +1101,7 @@ void key_restore(byte *to_record, byte *from_key, KEY *key_info,
                  uint key_length);
 bool key_cmp_if_same(TABLE *form,const byte *key,uint index,uint key_length);
 void key_unpack(String *to,TABLE *form,uint index);
-bool check_if_key_used(TABLE *table, uint idx, List<Item> &fields);
+bool is_key_used(TABLE *table, uint idx, List<Item> &fields);
 int key_cmp(KEY_PART_INFO *key_part, const byte *key, uint key_length);
 
 bool init_errmessage(void);

sql/opt_range.cc

@@ -6020,42 +6020,42 @@ static bool null_part_in_key(KEY_PART *key_part, const char *key, uint length)
 }
 
 
-bool QUICK_SELECT_I::check_if_keys_used(List<Item> *fields)
+bool QUICK_SELECT_I::is_keys_used(List<Item> *fields)
 {
-  return check_if_key_used(head, index, *fields);
+  return is_key_used(head, index, *fields);
 }
 
-bool QUICK_INDEX_MERGE_SELECT::check_if_keys_used(List<Item> *fields)
+bool QUICK_INDEX_MERGE_SELECT::is_keys_used(List<Item> *fields)
 {
   QUICK_RANGE_SELECT *quick;
   List_iterator_fast<QUICK_RANGE_SELECT> it(quick_selects);
   while ((quick= it++))
   {
-    if (check_if_key_used(head, quick->index, *fields))
+    if (is_key_used(head, quick->index, *fields))
       return 1;
   }
   return 0;
 }
 
-bool QUICK_ROR_INTERSECT_SELECT::check_if_keys_used(List<Item> *fields)
+bool QUICK_ROR_INTERSECT_SELECT::is_keys_used(List<Item> *fields)
 {
   QUICK_RANGE_SELECT *quick;
   List_iterator_fast<QUICK_RANGE_SELECT> it(quick_selects);
   while ((quick= it++))
   {
-    if (check_if_key_used(head, quick->index, *fields))
+    if (is_key_used(head, quick->index, *fields))
       return 1;
   }
   return 0;
 }
 
-bool QUICK_ROR_UNION_SELECT::check_if_keys_used(List<Item> *fields)
+bool QUICK_ROR_UNION_SELECT::is_keys_used(List<Item> *fields)
 {
   QUICK_SELECT_I *quick;
   List_iterator_fast<QUICK_SELECT_I> it(quick_selects);
   while ((quick= it++))
   {
-    if (quick->check_if_keys_used(fields))
+    if (quick->is_keys_used(fields))
       return 1;
   }
   return 0;

sql/opt_range.h

@@ -225,8 +225,9 @@ class QUICK_SELECT_I
     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);
+  virtual bool is_keys_used(List<Item> *fields);
 
   /*
     rowid of last row retrieved by this quick select. This is used only when
@@ -423,7 +424,7 @@ class QUICK_INDEX_MERGE_SELECT : public QUICK_SELECT_I
   int get_type() { return QS_TYPE_INDEX_MERGE; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
   void add_info_string(String *str);
-  bool check_if_keys_used(List<Item> *fields);
+  bool is_keys_used(List<Item> *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif
@@ -482,7 +483,7 @@ class QUICK_ROR_INTERSECT_SELECT : public QUICK_SELECT_I
   int get_type() { return QS_TYPE_ROR_INTERSECT; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
   void add_info_string(String *str);
-  bool check_if_keys_used(List<Item> *fields);
+  bool is_keys_used(List<Item> *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif
@@ -536,7 +537,7 @@ class QUICK_ROR_UNION_SELECT : public QUICK_SELECT_I
   int get_type() { return QS_TYPE_ROR_UNION; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
   void add_info_string(String *str);
-  bool check_if_keys_used(List<Item> *fields);
+  bool is_keys_used(List<Item> *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif

sql/sql_trigger.cc

@@ -1547,6 +1547,38 @@ void Table_triggers_list::mark_fields_used(THD *thd, trg_event_type event)
 }
 
 
+/*
+  Check if field of subject table can be changed in before update trigger.
+
+  SYNOPSIS
+    is_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::is_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
 

sql/sql_trigger.h

@@ -116,15 +116,12 @@ class Table_triggers_list: public Sql_alloc
             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 is_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,
                                                             TABLE_LIST *table);

sql/sql_update.cc

@@ -274,7 +274,7 @@ int mysql_update(THD *thd,
   {
     used_index= select->quick->index;
     used_key_is_modified= (!select->quick->unique_key_range() &&
-                          select->quick->check_if_keys_used(&fields));
+                          select->quick->is_keys_used(&fields));
   }
   else
   {
@@ -282,7 +282,7 @@ int mysql_update(THD *thd,
     if (used_index == MAX_KEY)                  // no index for sort order
       used_index= table->file->key_used_on_scan;
     if (used_index != MAX_KEY)
-      used_key_is_modified= check_if_key_used(table, used_index, fields);
+      used_key_is_modified= is_key_used(table, used_index, fields);
   }
 
   if (used_key_is_modified || order)
@@ -1188,21 +1188,15 @@ static bool safe_update_on_fly(JOIN_TAB *join_tab, List<Item> *fields)
     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 !is_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->is_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 !is_key_used(table, table->s->primary_key, *fields);
     return TRUE;
   default:
     break;					// Avoid compler warning