diff options
author | Jimmy Yang <jimmy.yang@oracle.com> | 2010-10-06 06:55:34 -0700 |
---|---|---|
committer | Jimmy Yang <jimmy.yang@oracle.com> | 2010-10-06 06:55:34 -0700 |
commit | 35f5429eda94f2ce87e453653cd22298d5851cfa (patch) | |
tree | e0bcd9c7e8e83214a1ffc233a312d342a96b127e | |
parent | 72f61e31d7aabf5f0612761530508b0cb49a95c8 (diff) | |
download | mariadb-git-35f5429eda94f2ce87e453653cd22298d5851cfa.tar.gz |
Manual port Bug #Bug #54582 "stack overflow when opening many tables
linked with foreign keys at once" from mysql-5.1-security to
mysql-5.5-security again.
rb://391 approved by Heikki
-rw-r--r-- | storage/innobase/dict/dict0load.c | 83 | ||||
-rw-r--r-- | storage/innobase/handler/ha_innodb.cc | 13 | ||||
-rw-r--r-- | storage/innobase/include/db0err.h | 3 | ||||
-rw-r--r-- | storage/innobase/include/dict0load.h | 2 | ||||
-rw-r--r-- | storage/innobase/include/dict0mem.h | 21 | ||||
-rw-r--r-- | storage/innobase/include/que0que.h | 3 | ||||
-rw-r--r-- | storage/innobase/row/row0merge.c | 2 | ||||
-rw-r--r-- | storage/innobase/row/row0mysql.c | 21 | ||||
-rw-r--r-- | storage/innobase/ut/ut0ut.c | 2 |
9 files changed, 139 insertions, 11 deletions
diff --git a/storage/innobase/dict/dict0load.c b/storage/innobase/dict/dict0load.c index 6bd15f0556a..74f108fcf8e 100644 --- a/storage/innobase/dict/dict0load.c +++ b/storage/innobase/dict/dict0load.c @@ -1718,17 +1718,28 @@ err_exit: err = dict_load_indexes(table, heap); + /* Initialize table foreign_child value. Its value could be + changed when dict_load_foreigns() is called below */ + table->fk_max_recusive_level = 0; + /* If the force recovery flag is set, we open the table irrespective of the error condition, since the user may want to dump data from the clustered index. However we load the foreign key information only if all indexes were loaded. */ if (!cached) { } else if (err == DB_SUCCESS) { - err = dict_load_foreigns(table->name, TRUE); + err = dict_load_foreigns(table->name, TRUE, TRUE); + + if (err != DB_SUCCESS) { + dict_table_remove_from_cache(table); + table = NULL; + } } else if (!srv_force_recovery) { dict_table_remove_from_cache(table); table = NULL; } + + table->fk_max_recusive_level = 0; #if 0 if (err != DB_SUCCESS && table != NULL) { @@ -1952,8 +1963,12 @@ dict_load_foreign( /*==============*/ const char* id, /*!< in: foreign constraint id as a null-terminated string */ - ibool check_charsets) + ibool check_charsets, /*!< in: TRUE=check charset compatibility */ + ibool check_recursive) + /*!< in: Whether to record the foreign table + parent count to avoid unlimited recursive + load of chained foreign tables */ { dict_foreign_t* foreign; dict_table_t* sys_foreign; @@ -1967,6 +1982,8 @@ dict_load_foreign( ulint len; ulint n_fields_and_type; mtr_t mtr; + dict_table_t* for_table; + dict_table_t* ref_table; ut_ad(mutex_own(&(dict_sys->mutex))); @@ -2051,11 +2068,54 @@ dict_load_foreign( dict_load_foreign_cols(id, foreign); - /* If the foreign table is not yet in the dictionary cache, we - have to load it so that we are able to make type comparisons - in the next function call. */ - - dict_table_get_low(foreign->foreign_table_name); + ref_table = dict_table_check_if_in_cache_low( + foreign->referenced_table_name); + + /* We could possibly wind up in a deep recursive calls if + we call dict_table_get_low() again here if there + is a chain of tables concatenated together with + foreign constraints. In such case, each table is + both a parent and child of the other tables, and + act as a "link" in such table chains. + To avoid such scenario, we would need to check the + number of ancesters the current table has. If that + exceeds DICT_FK_MAX_CHAIN_LEN, we will stop loading + the child table. + Foreign constraints are loaded in a Breath First fashion, + that is, the index on FOR_NAME is scanned first, and then + index on REF_NAME. So foreign constrains in which + current table is a child (foreign table) are loaded first, + and then those constraints where current table is a + parent (referenced) table. + Thus we could check the parent (ref_table) table's + reference count (fk_max_recusive_level) to know how deep the + recursive call is. If the parent table (ref_table) is already + loaded, and its fk_max_recusive_level is larger than + DICT_FK_MAX_CHAIN_LEN, we will stop the recursive loading + by skipping loading the child table. It will not affect foreign + constraint check for DMLs since child table will be loaded + at that time for the constraint check. */ + if (!ref_table + || ref_table->fk_max_recusive_level < DICT_FK_MAX_RECURSIVE_LOAD) { + + /* If the foreign table is not yet in the dictionary cache, we + have to load it so that we are able to make type comparisons + in the next function call. */ + + for_table = dict_table_get_low(foreign->foreign_table_name); + + if (for_table && ref_table && check_recursive) { + /* This is to record the longest chain of ancesters + this table has, if the parent has more ancesters + than this table has, record it after add 1 (for this + parent */ + if (ref_table->fk_max_recusive_level + >= for_table->fk_max_recusive_level) { + for_table->fk_max_recusive_level = + ref_table->fk_max_recusive_level + 1; + } + } + } /* Note that there may already be a foreign constraint object in the dictionary cache for this constraint: then the following @@ -2080,6 +2140,8 @@ ulint dict_load_foreigns( /*===============*/ const char* table_name, /*!< in: table name */ + ibool check_recursive,/*!< in: Whether to check recursive + load of tables chained by FK */ ibool check_charsets) /*!< in: TRUE=check charset compatibility */ { @@ -2181,7 +2243,7 @@ loop: /* Load the foreign constraint definition to the dictionary cache */ - err = dict_load_foreign(id, check_charsets); + err = dict_load_foreign(id, check_charsets, check_recursive); if (err != DB_SUCCESS) { btr_pcur_close(&pcur); @@ -2209,6 +2271,11 @@ load_next_index: mtr_start(&mtr); + /* Switch to scan index on REF_NAME, fk_max_recusive_level + already been updated when scanning FOR_NAME index, no need to + update again */ + check_recursive = FALSE; + goto start_load; } diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index eafc73b4f87..25148fce24d 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -901,6 +901,19 @@ convert_error_code_to_mysql( case DB_INTERRUPTED: my_error(ER_QUERY_INTERRUPTED, MYF(0)); /* fall through */ + + case DB_FOREIGN_EXCEED_MAX_CASCADE: + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + HA_ERR_ROW_IS_REFERENCED, + "InnoDB: Cannot delete/update " + "rows with cascading foreign key " + "constraints that exceed max " + "depth of %d. Please " + "drop extra constraints and try " + "again", DICT_FK_MAX_RECURSIVE_LOAD); + + /* fall through */ + case DB_ERROR: default: return(-1); /* unspecified error */ diff --git a/storage/innobase/include/db0err.h b/storage/innobase/include/db0err.h index f84ce2d15aa..01bb1b80754 100644 --- a/storage/innobase/include/db0err.h +++ b/storage/innobase/include/db0err.h @@ -101,6 +101,9 @@ enum db_err { requested but this storage does not exist itself or the stats for a given table do not exist */ + DB_FOREIGN_EXCEED_MAX_CASCADE, /* Foreign key constraint related + cascading delete/update exceeds + maximum allowed depth */ /* The following are partial failure codes */ DB_FAIL = 1000, diff --git a/storage/innobase/include/dict0load.h b/storage/innobase/include/dict0load.h index 05d3532d59a..f009f221f32 100644 --- a/storage/innobase/include/dict0load.h +++ b/storage/innobase/include/dict0load.h @@ -200,6 +200,8 @@ ulint dict_load_foreigns( /*===============*/ const char* table_name, /*!< in: table name */ + ibool check_recursive,/*!< in: Whether to check recursive + load of tables chained by FK */ ibool check_charsets);/*!< in: TRUE=check charsets compatibility */ /********************************************************************//** diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 75f9acd6b26..d448e57e395 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -116,6 +116,21 @@ ROW_FORMAT=REDUNDANT. */ in table->flags. */ /* @} */ +/** Tables could be chained together with Foreign key constraint. When +first load the parent table, we would load all of its descedents. +This could result in rescursive calls and out of stack error eventually. +DICT_FK_MAX_RECURSIVE_LOAD defines the maximum number of recursive loads, +when exceeded, the child table will not be loaded. It will be loaded when +the foreign constraint check needs to be run. */ +#define DICT_FK_MAX_RECURSIVE_LOAD 255 + +/** Similarly, when tables are chained together with foreign key constraints +with on cascading delete/update clause, delete from parent table could +result in recursive cascading calls. This defines the maximum number of +such cascading deletes/updates allowed. When exceeded, the delete from +parent table will fail, and user has to drop excessive foreign constraint +before proceeds. */ +#define FK_MAX_CASCADE_DEL 255 /**********************************************************************//** Creates a table memory object. @@ -469,6 +484,12 @@ struct dict_table_struct{ NOT allowed until this count gets to zero; MySQL does NOT itself check the number of open handles at drop */ + unsigned fk_max_recusive_level:8; + /*!< maximum recursive level we support when + loading tables chained together with FK + constraints. If exceeds this level, we will + stop loading child table into memory along with + its parent table */ ulint n_foreign_key_checks_running; /*!< count of how many foreign key check operations are currently being performed diff --git a/storage/innobase/include/que0que.h b/storage/innobase/include/que0que.h index 09734bbb197..720da6dcb46 100644 --- a/storage/innobase/include/que0que.h +++ b/storage/innobase/include/que0que.h @@ -381,6 +381,9 @@ struct que_thr_struct{ thus far */ ulint lock_state; /*!< lock state of thread (table or row) */ + ulint fk_cascade_depth; /*!< maximum cascading call depth + supported for foreign key constraint + related delete/updates */ }; #define QUE_THR_MAGIC_N 8476583 diff --git a/storage/innobase/row/row0merge.c b/storage/innobase/row/row0merge.c index 38ec4bff08f..1b96ff50e66 100644 --- a/storage/innobase/row/row0merge.c +++ b/storage/innobase/row/row0merge.c @@ -2418,7 +2418,7 @@ row_merge_rename_tables( goto err_exit; } - err = dict_load_foreigns(old_name, TRUE); + err = dict_load_foreigns(old_name, FALSE, TRUE); if (err != DB_SUCCESS) { err_exit: diff --git a/storage/innobase/row/row0mysql.c b/storage/innobase/row/row0mysql.c index c12970b40ed..e0f85b3ebb4 100644 --- a/storage/innobase/row/row0mysql.c +++ b/storage/innobase/row/row0mysql.c @@ -635,6 +635,13 @@ handle_new_error: "InnoDB: " REFMAN "forcing-recovery.html" " for help.\n", stderr); break; + case DB_FOREIGN_EXCEED_MAX_CASCADE: + fprintf(stderr, "InnoDB: Cannot delete/update rows with" + " cascading foreign key constraints that exceed max" + " depth of %lu\n" + "Please drop excessive foreign constraints" + " and try again\n", (ulong) DICT_FK_MAX_RECURSIVE_LOAD); + break; default: fprintf(stderr, "InnoDB: unknown error code %lu\n", (ulong) err); @@ -1440,11 +1447,15 @@ row_update_for_mysql( run_again: thr->run_node = node; thr->prev_node = node; + thr->fk_cascade_depth = 0; row_upd_step(thr); err = trx->error_state; + /* Reset fk_cascade_depth back to 0 */ + thr->fk_cascade_depth = 0; + if (err != DB_SUCCESS) { que_thr_stop_for_mysql(thr); @@ -1640,6 +1651,12 @@ row_update_cascade_for_mysql( trx_t* trx; trx = thr_get_trx(thr); + + thr->fk_cascade_depth++; + + if (thr->fk_cascade_depth > FK_MAX_CASCADE_DEL) { + return (DB_FOREIGN_EXCEED_MAX_CASCADE); + } run_again: thr->run_node = node; thr->prev_node = node; @@ -2120,7 +2137,7 @@ row_table_add_foreign_constraints( name, reject_fks); if (err == DB_SUCCESS) { /* Check that also referencing constraints are ok */ - err = dict_load_foreigns(name, TRUE); + err = dict_load_foreigns(name, FALSE, TRUE); } if (err != DB_SUCCESS) { @@ -3993,7 +4010,7 @@ end: an ALTER, not in a RENAME. */ err = dict_load_foreigns( - new_name, !old_is_tmp || trx->check_foreigns); + new_name, FALSE, !old_is_tmp || trx->check_foreigns); if (err != DB_SUCCESS) { ut_print_timestamp(stderr); diff --git a/storage/innobase/ut/ut0ut.c b/storage/innobase/ut/ut0ut.c index 39978304696..39c60d2bc2f 100644 --- a/storage/innobase/ut/ut0ut.c +++ b/storage/innobase/ut/ut0ut.c @@ -693,6 +693,8 @@ ut_strerr( return("Lock structs have exhausted the buffer pool"); case DB_FOREIGN_DUPLICATE_KEY: return("Foreign key activated with duplicate keys"); + case DB_FOREIGN_EXCEED_MAX_CASCADE: + return("Foreign key cascade delete/update exceeds max depth"); case DB_TOO_MANY_CONCURRENT_TRXS: return("Too many concurrent transactions"); case DB_UNSUPPORTED: |