From 4122188cc2d3ce836f88681fc5e0d41cacfda3f9 Mon Sep 17 00:00:00 2001 From: "dlenev@brandersnatch.localdomain" <> Date: Wed, 31 Mar 2004 21:25:55 +0400 Subject: [PATCH] Fix for Bug #3307 "FLUSH TABLES sometimes breaks prepared statement table resolution". Added members to Item_ident for storing original db, table and field names since those that set later from Field have shorter life-time than required by prep. stmt. So we need to restore original names in Item_ident::cleanup(). Also now using special construnctor for creation of Item_field from Field object that ensures that table and field name have big enough life-time. "Fix" for bug #2050 "10 to 1 performance drop with server 4.1.1" Clean ups in implementation of caching of field number in table. Added caching of table in which field is found in find_field_in_tables(). --- sql/item.cc | 25 ++++++++++++++++++++++--- sql/item.h | 26 ++++++++++++++++++++++++-- sql/mysql_priv.h | 2 +- sql/sql_acl.cc | 2 +- sql/sql_base.cc | 47 +++++++++++++++++++++++++++++++++++------------ 5 files changed, 83 insertions(+), 19 deletions(-) diff --git a/sql/item.cc b/sql/item.cc index a919dc4c133..b4aa87cd4df 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -103,9 +103,12 @@ void Item::print_item_w_name(String *str) Item_ident::Item_ident(const char *db_name_par,const char *table_name_par, const char *field_name_par) - :changed_during_fix_field(0), db_name(db_name_par), - table_name(table_name_par), field_name(field_name_par), - cached_field_index(-1), depended_from(0) + : + orig_db_name(db_name_par), orig_table_name(table_name_par), + orig_field_name(field_name_par), changed_during_fix_field(0), + db_name(db_name_par), table_name(table_name_par), + field_name(field_name_par), cached_field_index(NO_CACHED_FIELD_INDEX), + cached_table(0), depended_from(0) { name = (char*) field_name_par; } @@ -113,11 +116,15 @@ Item_ident::Item_ident(const char *db_name_par,const char *table_name_par, // Constructor used by Item_field & Item_ref (see Item comment) Item_ident::Item_ident(THD *thd, Item_ident *item) :Item(thd, item), + orig_db_name(item->orig_db_name), + orig_table_name(item->orig_table_name), + orig_field_name(item->orig_field_name), changed_during_fix_field(0), db_name(item->db_name), table_name(item->table_name), field_name(item->field_name), cached_field_index(item->cached_field_index), + cached_table(item->cached_table), depended_from(item->depended_from) {} @@ -129,6 +136,9 @@ void Item_ident::cleanup() *changed_during_fix_field= this; changed_during_fix_field= 0; } + db_name= orig_db_name; + table_name= orig_table_name; + field_name= orig_field_name; } bool Item_ident::remove_dependence_processor(byte * arg) @@ -310,6 +320,15 @@ Item_field::Item_field(Field *f) fixed= 1; } +Item_field::Item_field(THD *thd, Field *f) + :Item_ident(NullS, thd->strdup(f->table_name), + thd->strdup(f->field_name)) +{ + set_field(f); + collation.set(DERIVATION_IMPLICIT); + fixed= 1; +} + // Constructor need to process subselect with temporary tables (see Item) Item_field::Item_field(THD *thd, Item_field *item) :Item_ident(thd, item), diff --git a/sql/item.h b/sql/item.h index fccdd7f0732..faee51af97e 100644 --- a/sql/item.h +++ b/sql/item.h @@ -253,10 +253,20 @@ public: virtual Item_num* neg()= 0; }; +#define NO_CACHED_FIELD_INDEX ((uint)(-1)) class st_select_lex; class Item_ident :public Item { + /* + We have to store initial values of db_name, table_name and field_name + to be able to restore them during cleanup() because they can be + updated during fix_fields() to values from Field object and life-time + of those is shorter than life-time of Item_field. + */ + const char *orig_db_name; + const char *orig_table_name; + const char *orig_field_name; Item **changed_during_fix_field; public: const char *db_name; @@ -264,9 +274,16 @@ public: const char *field_name; /* Cached value of index for this field in table->field array, used by prep. - stmts for speeding up their re-execution, -1 if index value is not known. + stmts for speeding up their re-execution. Holds NO_CACHED_FIELD_INDEX + if index value is not known. */ - int cached_field_index; + uint cached_field_index; + /* + Cached pointer to table which contains this field, used for the same reason + by prep. stmt. too in case then we have not-fully qualified field. + 0 - means no cached value. + */ + TABLE_LIST *cached_table; st_select_lex *depended_from; Item_ident(const char *db_name_par,const char *table_name_par, const char *field_name_par); @@ -298,6 +315,11 @@ public: { collation.set(DERIVATION_IMPLICIT); } // Constructor need to process subselect with temporary tables (see Item) Item_field(THD *thd, Item_field *item); + /* + Constructor used inside setup_wild(), ensures that field and table + names will live as long as Item_field (important in prep. stmt.) + */ + Item_field(THD *thd, Field *field); Item_field(Field *field); enum Type type() const { return FIELD_ITEM; } bool eq(const Item *item, bool binary_cmp) const; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 572e41b36d2..260cd8c812f 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -559,7 +559,7 @@ Field *find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables, TABLE_LIST **where, bool report_error); Field *find_field_in_table(THD *thd,TABLE *table,const char *name,uint length, bool check_grant,bool allow_rowid, - int *cached_field_index_ptr); + uint *cached_field_index_ptr); #ifdef HAVE_OPENSSL #include struct st_des_keyblock diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 86a535ba493..564f43cd495 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -2194,7 +2194,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, DBUG_RETURN(-1); while ((column = column_iter++)) { - int unused_field_idx= -1; + uint unused_field_idx= NO_CACHED_FIELD_INDEX; if (!find_field_in_table(thd,table,column->column.ptr(), column->column.length(),0,0, &unused_field_idx)) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 375ebb069a3..4d8a70887b9 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1791,12 +1791,13 @@ bool rm_temporary_table(enum db_type base, char *path) Field *find_field_in_table(THD *thd,TABLE *table,const char *name,uint length, bool check_grants, bool allow_rowid, - int *cached_field_index_ptr) + uint *cached_field_index_ptr) { - Field **field_ptr= 0, *field; - int cached_field_index= *cached_field_index_ptr; + Field **field_ptr, *field; + uint cached_field_index= *cached_field_index_ptr; - if (cached_field_index >= 0 && cached_field_index < table->fields && + /* We assume here that table->field < NO_CACHED_FIELD_INDEX = UINT_MAX */ + if (cached_field_index < table->fields && !my_strcasecmp(system_charset_info, table->field[cached_field_index]->field_name, name)) field_ptr= table->field + cached_field_index; @@ -1805,14 +1806,11 @@ Field *find_field_in_table(THD *thd,TABLE *table,const char *name,uint length, length); else { - if (!(field_ptr=table->field)) + if (!(field_ptr= table->field)) return (Field *)0; - while (*field_ptr) - { + for (; *field_ptr; ++field_ptr) if (!my_strcasecmp(system_charset_info, (*field_ptr)->field_name, name)) break; - ++field_ptr; - } } if (field_ptr && *field_ptr) @@ -1882,6 +1880,31 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables, uint length=(uint) strlen(name); char name_buff[NAME_LEN+1]; + + if (item->cached_table) + { + /* + This shortcut is used by prepared statements. We assuming that + TABLE_LIST *tables is not changed during query execution (which + is true for all queries except RENAME but luckily RENAME doesn't + use fields...) so we can rely on reusing pointer to its member. + With this optimisation we also miss case when addition of one more + field makes some prepared query ambiguous and so erronous, but we + accept this trade off. + */ + found= find_field_in_table(thd,tables->table,name,length, + test(tables->table->grant.want_privilege), + 1, &(item->cached_field_index)); + + if (found) + { + (*where)= tables; + if (found == WRONG_GRANT) + return (Field*) 0; + return found; + } + } + if (db && lower_case_table_names) { /* @@ -1909,7 +1932,7 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables, 1, &(item->cached_field_index)); if (find) { - (*where)= tables; + (*where)= item->cached_table= tables; if (find == WRONG_GRANT) return (Field*) 0; if (db || !thd->where) @@ -1968,7 +1991,7 @@ find_field_in_tables(THD *thd, Item_ident *item, TABLE_LIST *tables, { if (field == WRONG_GRANT) return (Field*) 0; - (*where)= tables; + (*where)= item->cached_table= tables; if (found) { if (!thd->where) // Returns first found @@ -2304,7 +2327,7 @@ insert_fields(THD *thd,TABLE_LIST *tables, const char *db_name, thd->used_tables|=table->map; while ((field = *ptr++)) { - Item_field *item= new Item_field(field); + Item_field *item= new Item_field(thd, field); if (!found++) (void) it->replace(item); // Replace '*' else