Commit e0d2f525 authored by unknown's avatar unknown

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.
Also check_if_key_used() routine and similar method were renamed
to is_key_used()/is_keys_used() in order to better reflect that
it is simple boolean predicate.
Finally, partition_key_modified() routine now also takes field
bitmap instead of field list as argument.


sql/key.cc:
  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.
  Also renamed this function to is_key_used().
sql/mysql_priv.h:
  Renamed check_if_key_used() to is_key_used(), also now this routine
  takes field bitmap instead of field list as argument.
sql/opt_range.cc:
  QUICK_SELECT_I::check_if_keys_used() method and check_if_key_used()
  routine were renamed to is_keys_used()/is_key_used(). Also now they
  take bitmap instead of field list as argument.
sql/opt_range.h:
  QUICK_SELECT_I::check_if_key_used() method was renamed to is_keys_used().
  Now it takes field bitmap instead of field list as argument and no
  longer responsible for checking if key uses automatically updated
  TIMESTAMP fields. Instead callers should properly mark such fields
  in field bitmap.
sql/sql_partition.cc:
  partition_key_modified() now takes field bitmap instead of list
  as argument.
sql/sql_partition.h:
  partition_key_modified() now takes field bitmap as argument
  instead of field list.
sql/sql_update.cc:
  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() routine and similar method (which were
  renamed to is_key_used()) 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.
parent f5e8d19f
...@@ -359,31 +359,29 @@ void key_unpack(String *to,TABLE *table,uint idx) ...@@ -359,31 +359,29 @@ 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 Check if key uses field that is marked in passed field bitmap.
that is automaticly updated (like a timestamp)
SYNOPSIS
is_key_used()
table TABLE object with which keys and fields are associated.
idx Key to be checked.
fields Bitmap of fields to be checked.
NOTE
This function uses TABLE::tmp_set bitmap so the caller should care
about saving/restoring its state if it also uses this bitmap.
RETURN VALUE
TRUE Key uses field from bitmap
FALSE Otherwise
*/ */
bool check_if_key_used(TABLE *table, uint idx, List<Item> &fields) bool is_key_used(TABLE *table, uint idx, const MY_BITMAP *fields)
{ {
List_iterator_fast<Item> f(fields); bitmap_clear_all(&table->tmp_set);
KEY_PART_INFO *key_part,*key_part_end; table->mark_columns_used_by_index_no_reset(idx, &table->tmp_set);
for (key_part=table->key_info[idx].key_part,key_part_end=key_part+ if (bitmap_is_overlapping(&table->tmp_set, fields))
table->key_info[idx].key_parts ; return 1;
key_part < key_part_end;
key_part++)
{
Item_field *field;
if (key_part->field == table->timestamp_field)
return 1; // Can't be used for update
f.rewind();
while ((field=(Item_field*) f++))
{
if (key_part->field->eq(field->field))
return 1;
}
}
/* /*
If table handler has primary key as part of the index, check that primary If table handler has primary key as part of the index, check that primary
...@@ -391,7 +389,7 @@ bool check_if_key_used(TABLE *table, uint idx, List<Item> &fields) ...@@ -391,7 +389,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 && if (idx != table->s->primary_key && table->s->primary_key < MAX_KEY &&
(table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX)) (table->file->ha_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; return 0;
} }
......
...@@ -1402,7 +1402,7 @@ void key_restore(byte *to_record, byte *from_key, KEY *key_info, ...@@ -1402,7 +1402,7 @@ void key_restore(byte *to_record, byte *from_key, KEY *key_info,
uint key_length); uint key_length);
bool key_cmp_if_same(TABLE *form,const byte *key,uint index,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); 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, const MY_BITMAP *fields);
int key_cmp(KEY_PART_INFO *key_part, const byte *key, uint key_length); 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); int key_rec_cmp(void *key_info, byte *a, byte *b);
......
...@@ -7418,42 +7418,42 @@ static bool null_part_in_key(KEY_PART *key_part, const char *key, uint length) ...@@ -7418,42 +7418,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(const MY_BITMAP *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(const MY_BITMAP *fields)
{ {
QUICK_RANGE_SELECT *quick; QUICK_RANGE_SELECT *quick;
List_iterator_fast<QUICK_RANGE_SELECT> it(quick_selects); List_iterator_fast<QUICK_RANGE_SELECT> it(quick_selects);
while ((quick= it++)) while ((quick= it++))
{ {
if (check_if_key_used(head, quick->index, *fields)) if (is_key_used(head, quick->index, fields))
return 1; return 1;
} }
return 0; return 0;
} }
bool QUICK_ROR_INTERSECT_SELECT::check_if_keys_used(List<Item> *fields) bool QUICK_ROR_INTERSECT_SELECT::is_keys_used(const MY_BITMAP *fields)
{ {
QUICK_RANGE_SELECT *quick; QUICK_RANGE_SELECT *quick;
List_iterator_fast<QUICK_RANGE_SELECT> it(quick_selects); List_iterator_fast<QUICK_RANGE_SELECT> it(quick_selects);
while ((quick= it++)) while ((quick= it++))
{ {
if (check_if_key_used(head, quick->index, *fields)) if (is_key_used(head, quick->index, fields))
return 1; return 1;
} }
return 0; return 0;
} }
bool QUICK_ROR_UNION_SELECT::check_if_keys_used(List<Item> *fields) bool QUICK_ROR_UNION_SELECT::is_keys_used(const MY_BITMAP *fields)
{ {
QUICK_SELECT_I *quick; QUICK_SELECT_I *quick;
List_iterator_fast<QUICK_SELECT_I> it(quick_selects); List_iterator_fast<QUICK_SELECT_I> it(quick_selects);
while ((quick= it++)) while ((quick= it++))
{ {
if (quick->check_if_keys_used(fields)) if (quick->is_keys_used(fields))
return 1; return 1;
} }
return 0; return 0;
......
...@@ -224,10 +224,9 @@ class QUICK_SELECT_I ...@@ -224,10 +224,9 @@ class QUICK_SELECT_I
virtual void add_info_string(String *str) {}; virtual void add_info_string(String *str) {};
/* /*
Return 1 if any index used by this quick select Return 1 if any index used by this quick select
a) uses field that is listed in passed field list or uses field which is marked in passed bitmap.
b) is automatically updated (like a timestamp)
*/ */
virtual bool check_if_keys_used(List<Item> *fields); virtual bool is_keys_used(const MY_BITMAP *fields);
/* /*
rowid of last row retrieved by this quick select. This is used only when rowid of last row retrieved by this quick select. This is used only when
...@@ -425,7 +424,7 @@ class QUICK_INDEX_MERGE_SELECT : public QUICK_SELECT_I ...@@ -425,7 +424,7 @@ class QUICK_INDEX_MERGE_SELECT : public QUICK_SELECT_I
int get_type() { return QS_TYPE_INDEX_MERGE; } int get_type() { return QS_TYPE_INDEX_MERGE; }
void add_keys_and_lengths(String *key_names, String *used_lengths); void add_keys_and_lengths(String *key_names, String *used_lengths);
void add_info_string(String *str); void add_info_string(String *str);
bool check_if_keys_used(List<Item> *fields); bool is_keys_used(const MY_BITMAP *fields);
#ifndef DBUG_OFF #ifndef DBUG_OFF
void dbug_dump(int indent, bool verbose); void dbug_dump(int indent, bool verbose);
#endif #endif
...@@ -484,7 +483,7 @@ class QUICK_ROR_INTERSECT_SELECT : public QUICK_SELECT_I ...@@ -484,7 +483,7 @@ class QUICK_ROR_INTERSECT_SELECT : public QUICK_SELECT_I
int get_type() { return QS_TYPE_ROR_INTERSECT; } int get_type() { return QS_TYPE_ROR_INTERSECT; }
void add_keys_and_lengths(String *key_names, String *used_lengths); void add_keys_and_lengths(String *key_names, String *used_lengths);
void add_info_string(String *str); void add_info_string(String *str);
bool check_if_keys_used(List<Item> *fields); bool is_keys_used(const MY_BITMAP *fields);
#ifndef DBUG_OFF #ifndef DBUG_OFF
void dbug_dump(int indent, bool verbose); void dbug_dump(int indent, bool verbose);
#endif #endif
...@@ -538,7 +537,7 @@ class QUICK_ROR_UNION_SELECT : public QUICK_SELECT_I ...@@ -538,7 +537,7 @@ class QUICK_ROR_UNION_SELECT : public QUICK_SELECT_I
int get_type() { return QS_TYPE_ROR_UNION; } int get_type() { return QS_TYPE_ROR_UNION; }
void add_keys_and_lengths(String *key_names, String *used_lengths); void add_keys_and_lengths(String *key_names, String *used_lengths);
void add_info_string(String *str); void add_info_string(String *str);
bool check_if_keys_used(List<Item> *fields); bool is_keys_used(const MY_BITMAP *fields);
#ifndef DBUG_OFF #ifndef DBUG_OFF
void dbug_dump(int indent, bool verbose); void dbug_dump(int indent, bool verbose);
#endif #endif
......
...@@ -2021,18 +2021,17 @@ char *generate_partition_syntax(partition_info *part_info, ...@@ -2021,18 +2021,17 @@ char *generate_partition_syntax(partition_info *part_info,
SYNOPSIS SYNOPSIS
partition_key_modified partition_key_modified
table TABLE object for which partition fields are set-up table TABLE object for which partition fields are set-up
fields A list of the to be modifed fields Bitmap representing fields to be modified
RETURN VALUES RETURN VALUES
TRUE Need special handling of UPDATE TRUE Need special handling of UPDATE
FALSE Normal UPDATE handling is ok FALSE Normal UPDATE handling is ok
*/ */
bool partition_key_modified(TABLE *table, List<Item> &fields) bool partition_key_modified(TABLE *table, const MY_BITMAP *fields)
{ {
List_iterator_fast<Item> f(fields); Field **fld;
partition_info *part_info= table->part_info; partition_info *part_info= table->part_info;
Item_field *item_field;
DBUG_ENTER("partition_key_modified"); DBUG_ENTER("partition_key_modified");
if (!part_info) if (!part_info)
...@@ -2040,9 +2039,8 @@ bool partition_key_modified(TABLE *table, List<Item> &fields) ...@@ -2040,9 +2039,8 @@ bool partition_key_modified(TABLE *table, List<Item> &fields)
if (table->s->db_type->partition_flags && if (table->s->db_type->partition_flags &&
(table->s->db_type->partition_flags() & HA_CAN_UPDATE_PARTITION_KEY)) (table->s->db_type->partition_flags() & HA_CAN_UPDATE_PARTITION_KEY))
DBUG_RETURN(FALSE); DBUG_RETURN(FALSE);
f.rewind(); for (fld= part_info->full_part_field_array; *fld; fld++)
while ((item_field=(Item_field*) f++)) if (bitmap_is_set(fields, (*fld)->field_index))
if (item_field->field->flags & FIELD_IN_PART_FUNC_FLAG)
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
DBUG_RETURN(FALSE); DBUG_RETURN(FALSE);
} }
......
...@@ -70,7 +70,7 @@ bool fix_partition_func(THD *thd, TABLE *table, bool create_table_ind); ...@@ -70,7 +70,7 @@ bool fix_partition_func(THD *thd, TABLE *table, bool create_table_ind);
char *generate_partition_syntax(partition_info *part_info, char *generate_partition_syntax(partition_info *part_info,
uint *buf_length, bool use_sql_alloc, uint *buf_length, bool use_sql_alloc,
bool show_partition_options); bool show_partition_options);
bool partition_key_modified(TABLE *table, List<Item> &fields); bool partition_key_modified(TABLE *table, const MY_BITMAP *fields);
void get_partition_set(const TABLE *table, byte *buf, const uint index, void get_partition_set(const TABLE *table, byte *buf, const uint index,
const key_range *key_spec, const key_range *key_spec,
part_id_range *part_spec); part_id_range *part_spec);
......
...@@ -25,7 +25,7 @@ ...@@ -25,7 +25,7 @@
#include "sp_head.h" #include "sp_head.h"
#include "sql_trigger.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 */ /* Return 0 if row hasn't changed */
...@@ -271,13 +271,16 @@ int mysql_update(THD *thd, ...@@ -271,13 +271,16 @@ int mysql_update(THD *thd,
} }
} }
init_ftfuncs(thd, select_lex, 1); 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 */ /* Check if we are modifying a key that we are used to search with */
if (select && select->quick) if (select && select->quick)
{ {
used_index= select->quick->index; used_index= select->quick->index;
used_key_is_modified= (!select->quick->unique_key_range() && used_key_is_modified= (!select->quick->unique_key_range() &&
select->quick->check_if_keys_used(&fields)); select->quick->is_keys_used(table->write_set));
} }
else else
{ {
...@@ -285,12 +288,13 @@ int mysql_update(THD *thd, ...@@ -285,12 +288,13 @@ int mysql_update(THD *thd,
if (used_index == MAX_KEY) // no index for sort order if (used_index == MAX_KEY) // no index for sort order
used_index= table->file->key_used_on_scan; used_index= table->file->key_used_on_scan;
if (used_index != MAX_KEY) 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, table->write_set);
} }
#ifdef WITH_PARTITION_STORAGE_ENGINE #ifdef WITH_PARTITION_STORAGE_ENGINE
if (used_key_is_modified || order || if (used_key_is_modified || order ||
partition_key_modified(table, fields)) partition_key_modified(table, table->write_set))
#else #else
if (used_key_is_modified || order) if (used_key_is_modified || order)
#endif #endif
...@@ -449,8 +453,6 @@ int mysql_update(THD *thd, ...@@ -449,8 +453,6 @@ int mysql_update(THD *thd,
MODE_STRICT_ALL_TABLES))); MODE_STRICT_ALL_TABLES)));
will_batch= !table->file->start_bulk_update(); will_batch= !table->file->start_bulk_update();
table->mark_columns_needed_for_update();
/* /*
We can use compare_record() to optimize away updates if We can use compare_record() to optimize away updates if
the table handler is returning all columns OR if the table handler is returning all columns OR if
...@@ -1216,7 +1218,7 @@ multi_update::initialize_tables(JOIN *join) ...@@ -1216,7 +1218,7 @@ multi_update::initialize_tables(JOIN *join)
table->mark_columns_needed_for_update(); table->mark_columns_needed_for_update();
if (table == main_table) // First table in join 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 table_to_update= main_table; // Update table on the fly
continue; continue;
...@@ -1274,7 +1276,6 @@ multi_update::initialize_tables(JOIN *join) ...@@ -1274,7 +1276,6 @@ multi_update::initialize_tables(JOIN *join)
SYNOPSIS SYNOPSIS
safe_update_on_fly safe_update_on_fly
join_tab How table is used in join join_tab How table is used in join
fields Fields that are updated
NOTES NOTES
We can update the first table in join on the fly if we know that We can update the first table in join on the fly if we know that
...@@ -1287,9 +1288,8 @@ multi_update::initialize_tables(JOIN *join) ...@@ -1287,9 +1288,8 @@ multi_update::initialize_tables(JOIN *join)
- We are doing a range scan and we don't update the scan key or - We are doing a range scan and we don't update the scan key or
the primary key for a clustered table handler. the primary key for a clustered table handler.
When checking for above cases we also should take into account that This function gets information about fields to be updated from
BEFORE UPDATE trigger potentially may change value of any field in row the TABLE::write_set bitmap.
being updated.
WARNING WARNING
This code is a bit dependent of how make_join_readinfo() works. This code is a bit dependent of how make_join_readinfo() works.
...@@ -1299,7 +1299,7 @@ multi_update::initialize_tables(JOIN *join) ...@@ -1299,7 +1299,7 @@ multi_update::initialize_tables(JOIN *join)
1 Safe to update 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; TABLE *table= join_tab->table;
switch (join_tab->type) { switch (join_tab->type) {
...@@ -1309,21 +1309,15 @@ static bool safe_update_on_fly(JOIN_TAB *join_tab, List<Item> *fields) ...@@ -1309,21 +1309,15 @@ static bool safe_update_on_fly(JOIN_TAB *join_tab, List<Item> *fields)
return TRUE; // At most one matching row return TRUE; // At most one matching row
case JT_REF: case JT_REF:
case JT_REF_OR_NULL: case JT_REF_OR_NULL:
return !check_if_key_used(table, join_tab->ref.key, *fields) && return !is_key_used(table, join_tab->ref.key, table->write_set);
!(table->triggers &&
table->triggers->has_before_update_triggers());
case JT_ALL: case JT_ALL:
/* If range search on index */ /* If range search on index */
if (join_tab->quick) if (join_tab->quick)
return !join_tab->quick->check_if_keys_used(fields) && return !join_tab->quick->is_keys_used(table->write_set);
!(table->triggers &&
table->triggers->has_before_update_triggers());
/* If scanning in clustered key */ /* If scanning in clustered key */
if ((table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) && if ((table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) &&
table->s->primary_key < MAX_KEY) table->s->primary_key < MAX_KEY)
return !check_if_key_used(table, table->s->primary_key, *fields) && return !is_key_used(table, table->s->primary_key, table->write_set);
!(table->triggers &&
table->triggers->has_before_update_triggers());
return TRUE; return TRUE;
default: default:
break; // Avoid compler warning break; // Avoid compler warning
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment