bk commit into 5.1 tree (dlenev:1.2291) BUG#20670

[dlenev]

Below is the list of changes that have just been committed into a local
5.1 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-10 18:51:57+04:00, dlenev@stripped +5 -0
  Proposed fix for bug#20670 "UPDATE using key and invoking trigger that
  modifies this key does not stop" (5.1 version).
  
  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 with help of
  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. We do this by
  changing check_if_key_used() to take field bitmap instead
  field list as argument and passing TABLE::write_set
  to it (we also have to add info about fields used in
  triggers to this bitmap a bit earlier).
  As nice side-effect we have more precise and thus more optimal
  perfomance-wise check for the MULTI-UPDATE.

  sql/key.cc@stripped, 2006-09-10 18:51:54+04:00, dlenev@stripped +20 -22
    Now check_if_key_used() takes field bitmap instead of field list as
    argument (bitmaps are also used in its implementation).
    It is no longer responsible for checking if key uses automatically
    updated TIMESTAMP fields, instead callers should properly mark such
    fields in field bitmap.

  sql/mysql_priv.h@stripped, 2006-09-10 18:51:54+04:00, dlenev@stripped +1 -1
    Now check_if_key_used() takes field bitmap instead of field list as
    argument.

  sql/opt_range.cc@stripped, 2006-09-10 18:51:54+04:00, dlenev@stripped +7 -7
    QUICK_SELECT_I::check_if_keys_used() method and check_if_key_used()
    routine now take bitmap instead of field list as argument.

  sql/opt_range.h@stripped, 2006-09-10 18:51:54+04:00, dlenev@stripped +5 -6
    QUICK_SELECT_I::check_if_key_used() method now takes field bitmap instead
    of field list as argument. Also it is no longer responsible for checking
    if key uses automatically updated TIMESTAMP fields. Instead callers should
    properly mark such fields in field bitmap.

  sql/sql_update.cc@stripped, 2006-09-10 18:51:54+04:00, dlenev@stripped +15 -20
    To detect situation in which trigger modifies part of key which is
    going to be used for processing of where clause and thus makes
    processing of this update with update-on-the-fly method unsafe
    we use check_if_key_used() rotuine (and similar method) plus
    information from TABLE::write_map bitmap. Note that we have to
    call TABLE::mark_columns_needed_for_update() method earlier now
    to fill this bitmap with information about fields updated in
    triggers.
    safe_update_on_fly() routine now uses the same approach and no
    longer needs list of fields as argument. 

# 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.1-bg20670

--- 1.43/sql/key.cc	2006-09-10 18:52:03 +04:00
+++ 1.44/sql/key.cc	2006-09-10 18:52:03 +04:00
@@ -359,31 +359,29 @@ 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 marked in passed field bitmap.
 
-bool check_if_key_used(TABLE *table, uint idx, List<Item> &fields)
-{
-  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+
-	 table->key_info[idx].key_parts ;
-       key_part < key_part_end;
-       key_part++)
-  {
-    Item_field *field;
+  SYNOPSIS
+    check_if_key_used()
+      table   TABLE object with which keys and fields are associated.
+      idx     Key to be checked.
+      fields  Bitmap of fields to be checked.
 
-    if (key_part->field == table->timestamp_field)
-      return 1;					// Can't be used for update
+  NOTE
+    This function uses TABLE::tmp_set bitmap so the caller should care
+    about saving/restoring its state if it also uses this bitmap.
 
-    f.rewind();
-    while ((field=(Item_field*) f++))
-    {
-      if (key_part->field->eq(field->field))
-	return 1;
-    }
-  }
+  RETURN VALUE
+    TRUE   Key uses field from bitmap
+    FALSE  Otherwise
+*/
+
+bool check_if_key_used(TABLE *table, uint idx, const MY_BITMAP *fields)
+{
+  bitmap_clear_all(&table->tmp_set);
+  table->mark_columns_used_by_index_no_reset(idx, &table->tmp_set);
+  if (bitmap_is_overlapping(&table->tmp_set, fields))
+    return 1;
 
   /*
     If table handler has primary key as part of the index, check that primary

--- 1.432/sql/mysql_priv.h	2006-09-10 18:52:03 +04:00
+++ 1.433/sql/mysql_priv.h	2006-09-10 18:52:03 +04:00
@@ -1402,7 +1402,7 @@ void key_restore(byte *to_record, byte *
                  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 check_if_key_used(TABLE *table, uint idx, const MY_BITMAP *fields);
 int key_cmp(KEY_PART_INFO *key_part, const byte *key, uint key_length);
 int key_rec_cmp(void *key_info, byte *a, byte *b);
 

--- 1.228/sql/opt_range.cc	2006-09-10 18:52:04 +04:00
+++ 1.229/sql/opt_range.cc	2006-09-10 18:52:04 +04:00
@@ -7418,36 +7418,36 @@ static bool null_part_in_key(KEY_PART *k
 }
 
 
-bool QUICK_SELECT_I::check_if_keys_used(List<Item> *fields)
+bool QUICK_SELECT_I::check_if_keys_used(const MY_BITMAP *fields)
 {
-  return check_if_key_used(head, index, *fields);
+  return check_if_key_used(head, index, fields);
 }
 
-bool QUICK_INDEX_MERGE_SELECT::check_if_keys_used(List<Item> *fields)
+bool QUICK_INDEX_MERGE_SELECT::check_if_keys_used(const MY_BITMAP *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 (check_if_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::check_if_keys_used(const MY_BITMAP *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 (check_if_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::check_if_keys_used(const MY_BITMAP *fields)
 {
   QUICK_SELECT_I *quick;
   List_iterator_fast<QUICK_SELECT_I> it(quick_selects);

--- 1.62/sql/opt_range.h	2006-09-10 18:52:04 +04:00
+++ 1.63/sql/opt_range.h	2006-09-10 18:52:04 +04:00
@@ -224,10 +224,9 @@ public:
   virtual void add_info_string(String *str) {};
   /*
     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)
+    uses field which is marked in passed bitmap.
   */
-  virtual bool check_if_keys_used(List<Item> *fields);
+  virtual bool check_if_keys_used(const MY_BITMAP *fields);
 
   /*
     rowid of last row retrieved by this quick select. This is used only when
@@ -425,7 +424,7 @@ public:
   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 check_if_keys_used(const MY_BITMAP *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif
@@ -484,7 +483,7 @@ public:
   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 check_if_keys_used(const MY_BITMAP *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif
@@ -538,7 +537,7 @@ public:
   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 check_if_keys_used(const MY_BITMAP *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif

--- 1.204/sql/sql_update.cc	2006-09-10 18:52:04 +04:00
+++ 1.205/sql/sql_update.cc	2006-09-10 18:52:04 +04:00
@@ -25,7 +25,7 @@
 #include "sp_head.h"
 #include "sql_trigger.h"
 
-static bool safe_update_on_fly(JOIN_TAB *join_tab, List<Item> *fields);
+static bool safe_update_on_fly(JOIN_TAB *join_tab);
 
 /* Return 0 if row hasn't changed */
 
@@ -271,13 +271,16 @@ int mysql_update(THD *thd,
     }
   }
   init_ftfuncs(thd, select_lex, 1);
+
+  table->mark_columns_needed_for_update();
+
   /* Check if we are modifying a key that we are used to search with */
   
   if (select && select->quick)
   {
     used_index= select->quick->index;
     used_key_is_modified= (!select->quick->unique_key_range() &&
-                          select->quick->check_if_keys_used(&fields));
+                          select->quick->check_if_keys_used(table->write_set));
   }
   else
   {
@@ -285,9 +288,11 @@ 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= check_if_key_used(table, used_index, table->write_set);
   }
 
+  /* QQ: should we fix partition_key_modified() as well ? */
+
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   if (used_key_is_modified || order ||
       partition_key_modified(table, fields))
@@ -449,8 +454,6 @@ int mysql_update(THD *thd,
                                 MODE_STRICT_ALL_TABLES)));
   will_batch= !table->file->start_bulk_update();
 
-  table->mark_columns_needed_for_update();
-
   /*
     We can use compare_record() to optimize away updates if
     the table handler is returning all columns OR if
@@ -1216,7 +1219,7 @@ multi_update::initialize_tables(JOIN *jo
     table->mark_columns_needed_for_update();
     if (table == main_table)			// First table in join
     {
-      if (safe_update_on_fly(join->join_tab, &temp_fields))
+      if (safe_update_on_fly(join->join_tab))
       {
 	table_to_update= main_table;		// Update table on the fly
 	continue;
@@ -1274,7 +1277,6 @@ multi_update::initialize_tables(JOIN *jo
   SYNOPSIS
     safe_update_on_fly
     join_tab		How table is used in join
-    fields		Fields that are updated
 
   NOTES
     We can update the first table in join on the fly if we know that
@@ -1287,9 +1289,8 @@ multi_update::initialize_tables(JOIN *jo
     - We are doing a range scan and we don't update the scan key or
       the primary key for a clustered table handler.
 
-    When checking for above cases we also should take into account that
-    BEFORE UPDATE trigger potentially may change value of any field in row
-    being updated.
+    This function gets information about fields to be updated from
+    the TABLE::write_set bitmap.
 
   WARNING
     This code is a bit dependent of how make_join_readinfo() works.
@@ -1299,7 +1300,7 @@ multi_update::initialize_tables(JOIN *jo
     1		Safe to update
 */
 
-static bool safe_update_on_fly(JOIN_TAB *join_tab, List<Item> *fields)
+static bool safe_update_on_fly(JOIN_TAB *join_tab)
 {
   TABLE *table= join_tab->table;
   switch (join_tab->type) {
@@ -1309,21 +1310,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, table->write_set);
   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(table->write_set);
     /* If scanning in clustered key */
     if ((table->file->ha_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, table->write_set);
     return TRUE;
   default:
     break;					// Avoid compler warning