From 190ec75c3540b5e9578472f637ff03b904b24042 Mon Sep 17 00:00:00 2001 From: Jimmy Yang Date: Sun, 1 Aug 2010 22:25:57 -0700 Subject: [PATCH 01/25] Fix Bug #55382 Assignment with SELECT expressions takes unexpected S locks in READ COMMITTED rb://410 Approved by Sunny Bains --- storage/innobase/handler/ha_innodb.cc | 9 +++++---- storage/innodb_plugin/ChangeLog | 7 +++++++ storage/innodb_plugin/handler/ha_innodb.cc | 9 +++++---- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index d10fcb8d31e..f29dd3be990 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -7814,16 +7814,17 @@ ha_innobase::store_lock( && (lock_type == TL_READ || lock_type == TL_READ_NO_INSERT) && (sql_command == SQLCOM_INSERT_SELECT || sql_command == SQLCOM_UPDATE - || sql_command == SQLCOM_CREATE_TABLE)) { + || sql_command == SQLCOM_CREATE_TABLE + || sql_command == SQLCOM_SET_OPTION)) { /* If we either have innobase_locks_unsafe_for_binlog option set or this session is using READ COMMITTED isolation level and isolation level of the transaction is not set to serializable and MySQL is doing INSERT INTO...SELECT or UPDATE ... = (SELECT ...) or - CREATE ... SELECT... without FOR UPDATE or - IN SHARE MODE in select, then we use consistent - read for select. */ + CREATE ... SELECT... or SET ... = (SELECT ...) + without FOR UPDATE or IN SHARE MODE in select, + then we use consistent read for select. */ prebuilt->select_lock_type = LOCK_NONE; prebuilt->stored_select_lock_type = LOCK_NONE; diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 3e802360d23..1f8491baa51 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,10 @@ +2010-08-01 The InnoDB Team + + * handler/ha_innodb.cc + Fix Bug #55382 Assignment with SELECT expressions takes unexpected + S locks in READ COMMITTED + + 2010-07-27 The InnoDB Team * include/mem0pool.h, mem/mem0mem.c, mem/mem0pool.c, srv/srv0start.c: diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index e0a62ed3ac5..a70921806b4 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/handler/ha_innodb.cc @@ -9235,7 +9235,8 @@ ha_innobase::store_lock( && (sql_command == SQLCOM_INSERT_SELECT || sql_command == SQLCOM_REPLACE_SELECT || sql_command == SQLCOM_UPDATE - || sql_command == SQLCOM_CREATE_TABLE)) { + || sql_command == SQLCOM_CREATE_TABLE + || sql_command == SQLCOM_SET_OPTION)) { /* If we either have innobase_locks_unsafe_for_binlog option set or this session is using READ COMMITTED @@ -9243,9 +9244,9 @@ ha_innobase::store_lock( is not set to serializable and MySQL is doing INSERT INTO...SELECT or REPLACE INTO...SELECT or UPDATE ... = (SELECT ...) or CREATE ... - SELECT... without FOR UPDATE or IN SHARE - MODE in select, then we use consistent read - for select. */ + SELECT... or SET ... = (SELECT ...) without + FOR UPDATE or IN SHARE MODE in select, + then we use consistent read for select. */ prebuilt->select_lock_type = LOCK_NONE; prebuilt->stored_select_lock_type = LOCK_NONE; From 666dfaf090c801db93ff6d583c22203291bc2030 Mon Sep 17 00:00:00 2001 From: Jimmy Yang Date: Tue, 3 Aug 2010 20:20:55 -0700 Subject: [PATCH 02/25] Backport "NULL pointer check for ut_free()" from mysql-trunk-innodb to mysql-5.1-innodb plugin to fix bug #55627 segv in ut_free pars_lexer_close innobase_shutdown innodb-use-sys-malloc=0. --- storage/innodb_plugin/ChangeLog | 7 ++++++- storage/innodb_plugin/include/ut0mem.h | 3 ++- storage/innodb_plugin/ut/ut0mem.c | 7 +++++-- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 1f8491baa51..9fe5c1e7ee6 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,10 +1,15 @@ +2010-08-03 The InnoDB Team + + * include/ut0mem.h, ut/ut0mem.c: + Fix Bug #55627 segv in ut_free pars_lexer_close innobase_shutdown + innodb-use-sys-malloc=0 + 2010-08-01 The InnoDB Team * handler/ha_innodb.cc Fix Bug #55382 Assignment with SELECT expressions takes unexpected S locks in READ COMMITTED - 2010-07-27 The InnoDB Team * include/mem0pool.h, mem/mem0mem.c, mem/mem0pool.c, srv/srv0start.c: diff --git a/storage/innodb_plugin/include/ut0mem.h b/storage/innodb_plugin/include/ut0mem.h index cf41cba4643..f14606be966 100644 --- a/storage/innodb_plugin/include/ut0mem.h +++ b/storage/innodb_plugin/include/ut0mem.h @@ -113,7 +113,8 @@ ut_test_malloc( ulint n); /*!< in: try to allocate this many bytes */ #endif /* !UNIV_HOTBACKUP */ /**********************************************************************//** -Frees a memory block allocated with ut_malloc. */ +Frees a memory block allocated with ut_malloc. Freeing a NULL pointer is +a nop. */ UNIV_INTERN void ut_free( diff --git a/storage/innodb_plugin/ut/ut0mem.c b/storage/innodb_plugin/ut/ut0mem.c index 35a325b9ccd..bf55e4273b6 100644 --- a/storage/innodb_plugin/ut/ut0mem.c +++ b/storage/innodb_plugin/ut/ut0mem.c @@ -290,7 +290,8 @@ ut_test_malloc( #endif /* !UNIV_HOTBACKUP */ /**********************************************************************//** -Frees a memory block allocated with ut_malloc. */ +Frees a memory block allocated with ut_malloc. Freeing a NULL pointer is +a nop. */ UNIV_INTERN void ut_free( @@ -300,7 +301,9 @@ ut_free( #ifndef UNIV_HOTBACKUP ut_mem_block_t* block; - if (UNIV_LIKELY(srv_use_sys_malloc)) { + if (ptr == NULL) { + return; + } else if (UNIV_LIKELY(srv_use_sys_malloc)) { free(ptr); return; } From 0812058d023a5261d40b31e8427669b5b5e8db20 Mon Sep 17 00:00:00 2001 From: Inaam Rana Date: Thu, 5 Aug 2010 11:34:44 -0400 Subject: [PATCH 03/25] Backport of revno 3148 mysql-innodb-trunk Currently we do a full validation of AHI whenever check tables is called on any table. This patch fixes this by only doing this full check in debug versions. bug#55716 rb://423 approved by: Marko --- storage/innodb_plugin/btr/btr0sea.c | 2 ++ storage/innodb_plugin/ha/ha0ha.c | 2 ++ storage/innodb_plugin/include/btr0sea.h | 4 ++++ storage/innodb_plugin/include/ha0ha.h | 2 ++ 4 files changed, 10 insertions(+) diff --git a/storage/innodb_plugin/btr/btr0sea.c b/storage/innodb_plugin/btr/btr0sea.c index ac7248fef20..f3ffe07a951 100644 --- a/storage/innodb_plugin/btr/btr0sea.c +++ b/storage/innodb_plugin/btr/btr0sea.c @@ -1734,6 +1734,7 @@ function_exit: } } +#if defined UNIV_AHI_DEBUG || defined UNIV_DEBUG /********************************************************************//** Validates the search system. @return TRUE if ok */ @@ -1897,3 +1898,4 @@ btr_search_validate(void) return(ok); } +#endif /* defined UNIV_AHI_DEBUG || defined UNIV_DEBUG */ diff --git a/storage/innodb_plugin/ha/ha0ha.c b/storage/innodb_plugin/ha/ha0ha.c index f9e798012f8..7f11917de0a 100644 --- a/storage/innodb_plugin/ha/ha0ha.c +++ b/storage/innodb_plugin/ha/ha0ha.c @@ -354,6 +354,7 @@ ha_remove_all_nodes_to_page( #endif } +#if defined UNIV_AHI_DEBUG || defined UNIV_DEBUG /*************************************************************//** Validates a given range of the cells in hash table. @return TRUE if ok */ @@ -400,6 +401,7 @@ ha_validate( return(ok); } +#endif /* defined UNIV_AHI_DEBUG || defined UNIV_DEBUG */ /*************************************************************//** Prints info of a hash table. */ diff --git a/storage/innodb_plugin/include/btr0sea.h b/storage/innodb_plugin/include/btr0sea.h index 20a2be7f877..6493689a969 100644 --- a/storage/innodb_plugin/include/btr0sea.h +++ b/storage/innodb_plugin/include/btr0sea.h @@ -180,6 +180,7 @@ btr_search_update_hash_on_delete( btr_cur_t* cursor);/*!< in: cursor which was positioned on the record to delete using btr_cur_search_..., the record is not yet deleted */ +#if defined UNIV_AHI_DEBUG || defined UNIV_DEBUG /********************************************************************//** Validates the search system. @return TRUE if ok */ @@ -187,6 +188,9 @@ UNIV_INTERN ibool btr_search_validate(void); /*======================*/ +#else +# define btr_search_validate() TRUE +#endif /* defined UNIV_AHI_DEBUG || defined UNIV_DEBUG */ /** Flag: has the search system been enabled? Protected by btr_search_latch and btr_search_enabled_mutex. */ diff --git a/storage/innodb_plugin/include/ha0ha.h b/storage/innodb_plugin/include/ha0ha.h index 1ffbd3440aa..3299000bf3c 100644 --- a/storage/innodb_plugin/include/ha0ha.h +++ b/storage/innodb_plugin/include/ha0ha.h @@ -186,6 +186,7 @@ ha_remove_all_nodes_to_page( hash_table_t* table, /*!< in: hash table */ ulint fold, /*!< in: fold value */ const page_t* page); /*!< in: buffer page */ +#if defined UNIV_AHI_DEBUG || defined UNIV_DEBUG /*************************************************************//** Validates a given range of the cells in hash table. @return TRUE if ok */ @@ -196,6 +197,7 @@ ha_validate( hash_table_t* table, /*!< in: hash table */ ulint start_index, /*!< in: start index */ ulint end_index); /*!< in: end index */ +#endif /* defined UNIV_AHI_DEBUG || defined UNIV_DEBUG */ /*************************************************************//** Prints info of a hash table. */ UNIV_INTERN From 531c0eee52bf96ffc3f10ed408ce1fb143b9fd0a Mon Sep 17 00:00:00 2001 From: Jimmy Yang Date: Fri, 6 Aug 2010 02:49:22 -0700 Subject: [PATCH 04/25] Address bug #55465 ERROR 1280 (42000): Incorrect index name '', adding a couple FK related messages. rb://409 approved by Sunny Bains --- storage/innobase/dict/dict0dict.c | 4 ++-- storage/innobase/handler/ha_innodb.cc | 26 ++++++++++++++++++++++++-- storage/innobase/include/db0err.h | 6 ++++++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/storage/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c index d2b59469cdc..9f9c4da46f4 100644 --- a/storage/innobase/dict/dict0dict.c +++ b/storage/innobase/dict/dict0dict.c @@ -2140,7 +2140,7 @@ dict_foreign_add_to_cache( mem_heap_free(foreign->heap); } - return(DB_CANNOT_ADD_CONSTRAINT); + return(DB_FOREIGN_NO_INDEX); } for_in_cache->referenced_table = ref_table; @@ -2184,7 +2184,7 @@ dict_foreign_add_to_cache( mem_heap_free(foreign->heap); } - return(DB_CANNOT_ADD_CONSTRAINT); + return(DB_REFERENCING_NO_INDEX); } for_in_cache->foreign_table = for_table; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index f29dd3be990..991b3a394d7 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -707,7 +707,9 @@ convert_error_code_to_mysql( return(HA_ERR_ROW_IS_REFERENCED); - } else if (error == (int) DB_CANNOT_ADD_CONSTRAINT) { + } else if (error == (int) DB_CANNOT_ADD_CONSTRAINT + || error == (int) DB_FOREIGN_NO_INDEX + || error == (int) DB_REFERENCING_NO_INDEX) { return(HA_ERR_CANNOT_ADD_FOREIGN); @@ -6099,6 +6101,8 @@ ha_innobase::rename_table( innobase_commit_low(trx); trx_free_for_mysql(trx); + switch (error) { + case DB_DUPLICATE_KEY: /* Add a special case to handle the Duplicated Key error and return DB_ERROR instead. This is to avoid a possible SIGSEGV error from mysql error @@ -6111,10 +6115,28 @@ ha_innobase::rename_table( the dup key error here is due to an existing table whose name is the one we are trying to rename to) and return the generic error code. */ - if (error == (int) DB_DUPLICATE_KEY) { my_error(ER_TABLE_EXISTS_ERROR, MYF(0), to); error = DB_ERROR; + break; + case DB_FOREIGN_NO_INDEX: + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + HA_ERR_CANNOT_ADD_FOREIGN, + "Alter or rename of table '%s' failed" + " because the new table is a child table" + " in a FK relationship and it does not" + " have an index that contains foreign" + " keys as its prefix columns.", norm_to); + break; + case DB_REFERENCING_NO_INDEX: + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + HA_ERR_CANNOT_ADD_FOREIGN, + "Alter or rename of table '%s' failed" + " because the new table is a parent table" + " in a FK relationship and it does not" + " have an index that contains foreign" + " keys as its prefix columns.", norm_to); + break; } error = convert_error_code_to_mysql(error, NULL); diff --git a/storage/innobase/include/db0err.h b/storage/innobase/include/db0err.h index 2be6005622d..cc6b0745b97 100644 --- a/storage/innobase/include/db0err.h +++ b/storage/innobase/include/db0err.h @@ -73,6 +73,12 @@ Created 5/24/1996 Heikki Tuuri a later version of the engine. */ #define DB_INTERRUPTED 49 /* the query has been interrupted with "KILL QUERY N;" */ +#define DB_FOREIGN_NO_INDEX 50 /* the child (foreign) table does not + have an index that contains the + foreign keys as its prefix columns */ +#define DB_REFERENCING_NO_INDEX 51 /* the parent (referencing) table does + not have an index that contains the + foreign keys as its prefix columns */ /* The following are partial failure codes */ #define DB_FAIL 1000 From 61f842444cc4ef8caa7342ae5e93706819c851c7 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Tue, 17 Aug 2010 22:37:18 +0300 Subject: [PATCH 05/25] Disable all innodb_plugin tests on Solaris until the problem is resolved. Track this via: Bug#56063 InnoDB Plugin mysql-tests fail on Solaris --- mysql-test/collections/default.experimental | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mysql-test/collections/default.experimental b/mysql-test/collections/default.experimental index f84337660ea..6d29134316d 100644 --- a/mysql-test/collections/default.experimental +++ b/mysql-test/collections/default.experimental @@ -12,6 +12,8 @@ funcs_1.ndb* # joro : NDB tests marked as experiment funcs_2.ndb_charset # joro : NDB tests marked as experimental as agreed with bochklin +innodb_plugin.* @solaris # Bug#56063 InnoDB Plugin mysql-tests fail on Solaris + main.ctype_gbk_binlog @solaris # Bug#46010: main.ctype_gbk_binlog fails sporadically : Table 't2' already exists main.func_str @solaris # joro: Bug#40928 main.plugin_load @solaris # Bug#42144 From 5a296bb07c2d335f7826e3aad91e6070817996c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 18 Aug 2010 14:01:10 +0300 Subject: [PATCH 06/25] Bug#55626: MIN and MAX reading a delete-marked record from secondary index Remove a bogus debug assertion that triggered the bug. Add assertions precisely where records must not be delete-marked. And a comment to clarify when the record is allowed to be delete-marked. --- storage/innodb_plugin/row/row0sel.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/storage/innodb_plugin/row/row0sel.c b/storage/innodb_plugin/row/row0sel.c index 76c144e5a8c..aff36b65124 100644 --- a/storage/innodb_plugin/row/row0sel.c +++ b/storage/innodb_plugin/row/row0sel.c @@ -2690,7 +2690,6 @@ row_sel_store_mysql_rec( ut_ad(prebuilt->mysql_template); ut_ad(prebuilt->default_rec); ut_ad(rec_offs_validate(rec, NULL, offsets)); - ut_ad(!rec_get_deleted_flag(rec, rec_offs_comp(offsets))); if (UNIV_LIKELY_NULL(prebuilt->blob_heap)) { mem_heap_free(prebuilt->blob_heap); @@ -3611,6 +3610,7 @@ row_search_for_mysql( row_sel_try_search_shortcut_for_mysql(). The latch will not be released until mtr_commit(&mtr). */ + ut_ad(!rec_get_deleted_flag(rec, comp)); if (!row_sel_store_mysql_rec(buf, prebuilt, rec, offsets)) { @@ -4238,7 +4238,7 @@ no_gap_lock: rec = old_vers; } - } else if (!lock_sec_rec_cons_read_sees(rec, trx->read_view)) { + } else { /* We are looking into a non-clustered index, and to get the right version of the record we have to look also into the clustered index: this @@ -4246,8 +4246,12 @@ no_gap_lock: information via the clustered index record. */ ut_ad(index != clust_index); + ut_ad(!dict_index_is_clust(index)); - goto requires_clust_rec; + if (!lock_sec_rec_cons_read_sees( + rec, trx->read_view)) { + goto requires_clust_rec; + } } } @@ -4370,8 +4374,13 @@ requires_clust_rec: ULINT_UNDEFINED, &heap); result_rec = rec; } + + /* result_rec can legitimately be delete-marked + now that it has been established that it points to a + clustered index record that exists in the read view. */ } else { result_rec = rec; + ut_ad(!rec_get_deleted_flag(rec, comp)); } /* We found a qualifying record 'result_rec'. At this point, From b6c5f4537be428578e55b7c73fa6b624046c3089 Mon Sep 17 00:00:00 2001 From: Sunny Bains Date: Fri, 20 Aug 2010 12:55:52 +1000 Subject: [PATCH 07/25] Fix bug#55699 - Assertion failure in innodb plugin with large number of threads Fix a debug assertion that was missed in svnrev:2380 (fix for Bug# 35352). Approved by Marko on IRC --- storage/innodb_plugin/trx/trx0undo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/innodb_plugin/trx/trx0undo.c b/storage/innodb_plugin/trx/trx0undo.c index 3bb1b1cdf6c..eb5112c4d31 100644 --- a/storage/innodb_plugin/trx/trx0undo.c +++ b/storage/innodb_plugin/trx/trx0undo.c @@ -1938,7 +1938,8 @@ trx_undo_update_cleanup( UT_LIST_ADD_FIRST(undo_list, rseg->update_undo_cached, undo); } else { - ut_ad(undo->state == TRX_UNDO_TO_PURGE); + ut_ad(undo->state == TRX_UNDO_TO_PURGE + || undo->state == TRX_UNDO_TO_FREE); trx_undo_mem_free(undo); } From 3daf6d3d73168971dfba30f7a248a6a48fca5691 Mon Sep 17 00:00:00 2001 From: Sunny Bains Date: Fri, 20 Aug 2010 12:57:04 +1000 Subject: [PATCH 08/25] Fix Bug #55027: assertion: mutex_own(&dict_sys->mutex) in dict_table_get_on_id() The callers should indicate that the dictionary is locked or not using the trx->dict_operation_lock_mode == RW_X_LATCH mode. Checking explicitly for system tables is unnecessary. Approved by Marko on IRC. --- storage/innobase/dict/dict0dict.c | 3 +-- storage/innodb_plugin/dict/dict0dict.c | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/storage/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c index 9f9c4da46f4..0e323d2bf24 100644 --- a/storage/innobase/dict/dict0dict.c +++ b/storage/innobase/dict/dict0dict.c @@ -616,8 +616,7 @@ dict_table_get_on_id( { dict_table_t* table; - if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0 - || trx->dict_operation_lock_mode == RW_X_LATCH) { + if (trx->dict_operation_lock_mode == RW_X_LATCH) { /* Note: An X latch implies that the transaction already owns the dictionary mutex. */ diff --git a/storage/innodb_plugin/dict/dict0dict.c b/storage/innodb_plugin/dict/dict0dict.c index fe4e058e122..a79f084e327 100644 --- a/storage/innodb_plugin/dict/dict0dict.c +++ b/storage/innodb_plugin/dict/dict0dict.c @@ -568,8 +568,7 @@ dict_table_get_on_id( { dict_table_t* table; - if (ut_dulint_cmp(table_id, DICT_FIELDS_ID) <= 0 - || trx->dict_operation_lock_mode == RW_X_LATCH) { + if (trx->dict_operation_lock_mode == RW_X_LATCH) { /* Note: An X latch implies that the transaction already owns the dictionary mutex. */ From 81da9b3d72a6d971f686613129679661524e2dd4 Mon Sep 17 00:00:00 2001 From: Sunny Bains Date: Fri, 20 Aug 2010 17:49:43 +1000 Subject: [PATCH 09/25] Fix Bug #54538 - use of exclusive innodb dictionary lock limits performance. This patch doesn't get rid of the need to acquire the dict_sys->mutex but reduces the need to keep the mutex locked for the duration of the query to fsp_get_available_space_in_free_extents() from ha_innobase::info(). rb://390. --- storage/innobase/fil/fil0fil.c | 104 +++++++++++++++++++++----- storage/innobase/fsp/fsp0fsp.c | 49 ++++++++++++ storage/innobase/handler/ha_innodb.cc | 20 ++--- storage/innobase/include/fil0fil.h | 14 +++- storage/innobase/include/univ.i | 6 ++ 5 files changed, 160 insertions(+), 33 deletions(-) diff --git a/storage/innobase/fil/fil0fil.c b/storage/innobase/fil/fil0fil.c index 3810fefd3cb..6ca8381ebdf 100644 --- a/storage/innobase/fil/fil0fil.c +++ b/storage/innobase/fil/fil0fil.c @@ -966,6 +966,8 @@ try_again: HASH_SEARCH(name_hash, system->name_hash, ut_fold_string(name), space, 0 == strcmp(name, space->name)); if (space != NULL) { + ibool success; + ut_print_timestamp(stderr); fprintf(stderr, " InnoDB: Warning: trying to init to the" @@ -1002,9 +1004,10 @@ try_again: namesake_id = space->id; - mutex_exit(&(system->mutex)); + success = fil_space_free(namesake_id, FALSE); + ut_a(success); - fil_space_free(namesake_id); + mutex_exit(&(system->mutex)); goto try_again; } @@ -1127,6 +1130,33 @@ fil_assign_new_space_id(void) return(id); } +/*********************************************************************** +Check if the space id exists in the cache, complain to stderr if the +space id cannot be found. */ +static +fil_space_t* +fil_space_search( +/*=============*/ + /* out: file space instance*/ + ulint id) /* in: space id */ +{ + fil_space_t* space; + + ut_ad(mutex_own(&fil_system->mutex)); + + HASH_SEARCH(hash, fil_system->spaces, id, space, space->id == id); + + if (space == NULL) { + ut_print_timestamp(stderr); + fprintf(stderr, + " InnoDB: Error: trying to remove tablespace %lu" + " from the cache but\n" + "InnoDB: it is not there.\n", (ulong) id); + } + + return(space); +} + /*********************************************************************** Frees a space object from the tablespace memory cache. Closes the files in the chain but does not delete them. There must not be any pending i/o's or @@ -1135,27 +1165,21 @@ flushes on the files. */ ibool fil_space_free( /*===========*/ - /* out: TRUE if success */ - ulint id) /* in: space id */ + /* out: TRUE if success */ + ulint id, /* in: space id */ + ibool x_latched) /* in: TRUE if caller has space->latch + in X mode */ { fil_system_t* system = fil_system; fil_space_t* space; fil_space_t* namespace; fil_node_t* fil_node; - mutex_enter(&(system->mutex)); + ut_ad(mutex_own(&fil_system->mutex)); - HASH_SEARCH(hash, system->spaces, id, space, space->id == id); - - if (!space) { - ut_print_timestamp(stderr); - fprintf(stderr, - " InnoDB: Error: trying to remove tablespace %lu" - " from the cache but\n" - "InnoDB: it is not there.\n", (ulong) id); - - mutex_exit(&(system->mutex)); + space = fil_space_search(id); + if (space == NULL) { return(FALSE); } @@ -1191,7 +1215,9 @@ fil_space_free( ut_a(0 == UT_LIST_GET_LEN(space->chain)); - mutex_exit(&(system->mutex)); + if (x_latched) { + rw_lock_x_unlock(&space->latch); + } rw_lock_free(&(space->latch)); @@ -2048,6 +2074,19 @@ try_again: path = mem_strdup(space->name); mutex_exit(&(system->mutex)); + + /* Important: We rely on the data dictionary mutex to ensure + that a race is not possible here. It should serialize the tablespace + drop/free. We acquire an X latch only to avoid a race condition + when accessing the tablespace instance via: + + fsp_get_available_space_in_free_extents(). + + There our main motivation is to reduce the contention on the + dictionary mutex and not correctness. */ + + rw_lock_x_lock(&space->latch); + #ifndef UNIV_HOTBACKUP /* Invalidate in the buffer pool all pages belonging to the tablespace. Since we have set space->is_being_deleted = TRUE, readahead @@ -2060,7 +2099,11 @@ try_again: #endif /* printf("Deleting tablespace %s id %lu\n", space->name, id); */ - success = fil_space_free(id); + mutex_enter(&system->mutex); + + success = fil_space_free(id, TRUE); + + mutex_exit(&system->mutex); if (success) { success = os_file_delete(path); @@ -2068,6 +2111,8 @@ try_again: if (!success) { success = os_file_delete_if_exists(path); } + } else { + rw_lock_x_unlock(&space->latch); } if (success) { @@ -4569,3 +4614,28 @@ fil_page_get_type( return(mach_read_from_2(page + FIL_PAGE_TYPE)); } + +/*********************************************************************** +Returns TRUE if a single-table tablespace is being deleted. */ + +ibool +fil_tablespace_is_being_deleted( +/*============================*/ + /* out: TRUE if space is being deleted */ + ulint id) /* in: space id */ +{ + fil_space_t* space; + ibool is_being_deleted; + + mutex_enter(&fil_system->mutex); + + HASH_SEARCH(hash, fil_system->spaces, id, space, space->id == id); + + ut_a(space != NULL); + + is_being_deleted = space->is_being_deleted; + + mutex_exit(&fil_system->mutex); + + return(is_being_deleted); +} diff --git a/storage/innobase/fsp/fsp0fsp.c b/storage/innobase/fsp/fsp0fsp.c index 1ec1c262a52..1f033c4b140 100644 --- a/storage/innobase/fsp/fsp0fsp.c +++ b/storage/innobase/fsp/fsp0fsp.c @@ -2842,12 +2842,61 @@ fsp_get_available_space_in_free_extents( ut_ad(!mutex_own(&kernel_mutex)); + /* The convoluted mutex acquire is to overcome latching order + issues: The problem is that the fil_mutex is at a lower level + than the tablespace latch and the buffer pool mutex. We have to + first prevent any operations on the file system by acquiring the + dictionary mutex. Then acquire the tablespace latch to obey the + latching order and then release the dictionary mutex. That way we + ensure that the tablespace instance can't be freed while we are + examining its contents (see fil_space_free()). + + However, there is one further complication, we release the fil_mutex + when we need to invalidate the the pages in the buffer pool and we + reacquire the fil_mutex when deleting and freeing the tablespace + instance in fil0fil.c. Here we need to account for that situation + too. */ + + dict_mutex_enter_for_mysql(); + + /* At this stage there is no guarantee that the tablespace even + exists in the cache. */ + + if (fil_tablespace_deleted_or_being_deleted_in_mem(space, -1)) { + + dict_mutex_exit_for_mysql(); + + return(ULLINT_UNDEFINED); + } + mtr_start(&mtr); latch = fil_space_get_latch(space); + /* This should ensure that the tablespace instance can't be freed + by another thread. However, the tablespace pages can still be freed + from the buffer pool. We need to check for that again. */ + mtr_x_lock(latch, &mtr); + dict_mutex_exit_for_mysql(); + + /* At this point it is possible for the tablespace to be deleted and + its pages removed from the buffer pool. We need to check for that + situation. However, the tablespace instance can't be deleted because + our latching above should ensure that. */ + + if (fil_tablespace_is_being_deleted(space)) { + + mtr_commit(&mtr); + + return(ULLINT_UNDEFINED); + } + + /* From here on even if the user has dropped the tablespace, the + pages _must_ still exist in the buffer pool and the tablespace + instance _must be in the file system hash table. */ + space_header = fsp_get_space_header(space, &mtr); size = mtr_read_ulint(space_header + FSP_SIZE, MLOG_4BYTES, &mtr); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 991b3a394d7..38394544c87 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -6485,20 +6485,12 @@ ha_innobase::info( so the "old" value can remain. delete_length is initialized to 0 in the ha_statistics' constructor. */ if (!(flag & HA_STATUS_NO_LOCK)) { + ullint avail_space; - /* lock the data dictionary to avoid races with - ibd_file_missing and tablespace_discarded */ - row_mysql_lock_data_dictionary(prebuilt->trx); - - /* ib_table->space must be an existent tablespace */ - if (!ib_table->ibd_file_missing - && !ib_table->tablespace_discarded) { - - stats.delete_length = - fsp_get_available_space_in_free_extents( - ib_table->space) * 1024; - } else { + avail_space = fsp_get_available_space_in_free_extents( + ib_table->space); + if (avail_space == ULLINT_UNDEFINED) { THD* thd; thd = ha_thd(); @@ -6515,9 +6507,9 @@ ha_innobase::info( ib_table->name); stats.delete_length = 0; + } else { + stats.delete_length = avail_space * 1024; } - - row_mysql_unlock_data_dictionary(prebuilt->trx); } stats.check_time = 0; diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h index 251d6c22547..7e85a0b412b 100644 --- a/storage/innobase/include/fil0fil.h +++ b/storage/innobase/include/fil0fil.h @@ -202,8 +202,10 @@ the chain but does not delete them. */ ibool fil_space_free( /*===========*/ - /* out: TRUE if success */ - ulint id); /* in: space id */ + /* out: TRUE if success */ + ulint id, /* in: space id */ + ibool x_latched); /* in: TRUE if caller has space->latch + in X mode */ /*********************************************************************** Returns the size of the space in pages. The tablespace must be cached in the memory cache. */ @@ -710,6 +712,14 @@ fil_page_get_type( written to page, the return value not defined */ byte* page); /* in: file page */ +/*********************************************************************** +Returns TRUE if a single-table tablespace is being deleted. */ + +ibool +fil_tablespace_is_being_deleted( +/*============================*/ + /* out: TRUE if space is being deleted */ + ulint id); /* in: space id */ typedef struct fil_space_struct fil_space_t; diff --git a/storage/innobase/include/univ.i b/storage/innobase/include/univ.i index 97d022d284e..ce5d8a092bf 100644 --- a/storage/innobase/include/univ.i +++ b/storage/innobase/include/univ.i @@ -234,6 +234,12 @@ typedef unsigned long long int ullint; /* Maximum value for a ulint */ #define ULINT_MAX ((ulint)(-2)) +/* THe 'undefined' value for ullint */ +#define ULLINT_UNDEFINED ((ullint)(-1)) + +/* Maximum value for a ullint */ +#define ULLINT_MAX ((ullint)(-2)) + /* This 'ibool' type is used within Innobase. Remember that different included headers may define 'bool' differently. Do not assume that 'bool' is a ulint! */ #define ibool ulint From bc05efd8aa116e9609941ad1f19665b79eca0eef Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Fri, 20 Aug 2010 21:17:51 +0200 Subject: [PATCH 10/25] post push test fix --- mysql-test/t/partition_not_blackhole.test | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql-test/t/partition_not_blackhole.test b/mysql-test/t/partition_not_blackhole.test index 222c1bb091e..7352aeaa230 100644 --- a/mysql-test/t/partition_not_blackhole.test +++ b/mysql-test/t/partition_not_blackhole.test @@ -16,6 +16,7 @@ let $MYSQLD_DATADIR= `SELECT @@datadir`; --copy_file std_data/parts/t1_blackhole.frm $MYSQLD_DATADIR/test/t1.frm --copy_file std_data/parts/t1_blackhole.par $MYSQLD_DATADIR/test/t1.par SHOW TABLES; +--replace_result $MYSQLD_DATADIR ./ --error ER_NOT_FORM_FILE SHOW CREATE TABLE t1; --error ER_BAD_TABLE_ERROR From 634af8f446c473b46fcdcf49957a1c595e363fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 23 Aug 2010 13:28:54 +0300 Subject: [PATCH 11/25] Bug#55832: selects crash too easily when innodb_force_recovery>3 dict_update_statistics_low(): Create bogus statistics for those indexes that cannot be accessed because of the innodb_force_recovery setting. ha_innobase::info(): Calculate statistics for each index, even if innodb_force_recovery is set. Fill in bogus data for those indexes that are not accessed because of the innodb_force_recovery setting. --- storage/innobase/dict/dict0dict.c | 57 ++++++++++++++++----------- storage/innobase/handler/ha_innodb.cc | 38 ++++++++++-------- 2 files changed, 56 insertions(+), 39 deletions(-) diff --git a/storage/innobase/dict/dict0dict.c b/storage/innobase/dict/dict0dict.c index 0e323d2bf24..b52a94c3348 100644 --- a/storage/innobase/dict/dict0dict.c +++ b/storage/innobase/dict/dict0dict.c @@ -3753,7 +3753,6 @@ dict_update_statistics_low( dictionary mutex */ { dict_index_t* index; - ulint size; ulint sum_of_index_sizes = 0; if (table->ibd_file_missing) { @@ -3769,14 +3768,6 @@ dict_update_statistics_low( return; } - /* If we have set a high innodb_force_recovery level, do not calculate - statistics, as a badly corrupted index can cause a crash in it. */ - - if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { - - return; - } - /* Find out the sizes of the indexes and how many different values for the key they approximately have */ @@ -3788,26 +3779,48 @@ dict_update_statistics_low( return; } - while (index) { - size = btr_get_size(index, BTR_TOTAL_SIZE); - index->stat_index_size = size; + do { + if (UNIV_LIKELY + (srv_force_recovery < SRV_FORCE_NO_IBUF_MERGE + || (srv_force_recovery < SRV_FORCE_NO_LOG_REDO + && (index->type & DICT_CLUSTERED)))) { + ulint size; + size = btr_get_size(index, BTR_TOTAL_SIZE); - sum_of_index_sizes += size; + index->stat_index_size = size; - size = btr_get_size(index, BTR_N_LEAF_PAGES); + sum_of_index_sizes += size; - if (size == 0) { - /* The root node of the tree is a leaf */ - size = 1; + size = btr_get_size(index, BTR_N_LEAF_PAGES); + + if (size == 0) { + /* The root node of the tree is a leaf */ + size = 1; + } + + index->stat_n_leaf_pages = size; + + btr_estimate_number_of_different_key_vals(index); + } else { + /* If we have set a high innodb_force_recovery + level, do not calculate statistics, as a badly + corrupted index can cause a crash in it. + Initialize some bogus index cardinality + statistics, so that the data can be queried in + various means, also via secondary indexes. */ + ulint i; + + sum_of_index_sizes++; + index->stat_index_size = index->stat_n_leaf_pages = 1; + + for (i = dict_index_get_n_unique(index); i; ) { + index->stat_n_diff_key_vals[i--] = 1; + } } - index->stat_n_leaf_pages = size; - - btr_estimate_number_of_different_key_vals(index); - index = dict_table_get_next_index(index); - } + } while (index); index = dict_table_get_first_index(table); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 38394544c87..4688b5fd16c 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -6365,8 +6365,6 @@ ha_innobase::info( dict_index_t* index; ha_rows rec_per_key; ib_longlong n_rows; - ulong j; - ulong i; char path[FN_REFLEN]; os_file_stat_t stat_info; @@ -6376,16 +6374,6 @@ ha_innobase::info( statistics calculation on tables, because that may crash the server if an index is badly corrupted. */ - if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { - - /* We return success (0) instead of HA_ERR_CRASHED, - because we want MySQL to process this query and not - stop, like it would do if it received the error code - HA_ERR_CRASHED. */ - - DBUG_RETURN(0); - } - /* We do not know if MySQL can call this function before calling external_lock(). To be safe, update the thd of the current table handle. */ @@ -6480,11 +6468,18 @@ ha_innobase::info( acquiring latches inside InnoDB, we do not call it if we are asked by MySQL to avoid locking. Another reason to avoid the call is that it uses quite a lot of CPU. - See Bug#38185. - We do not update delete_length if no locking is requested - so the "old" value can remain. delete_length is initialized - to 0 in the ha_statistics' constructor. */ - if (!(flag & HA_STATUS_NO_LOCK)) { + See Bug#38185. */ + if (flag & HA_STATUS_NO_LOCK) { + /* We do not update delete_length if no + locking is requested so the "old" value can + remain. delete_length is initialized to 0 in + the ha_statistics' constructor. */ + } else if (UNIV_UNLIKELY + (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE)) { + /* Avoid accessing the tablespace if + innodb_crash_recovery is set to a high value. */ + stats.delete_length = 0; + } else { ullint avail_space; avail_space = fsp_get_available_space_in_free_extents( @@ -6522,6 +6517,7 @@ ha_innobase::info( } if (flag & HA_STATUS_CONST) { + ulong i = 0; index = dict_table_get_first_index_noninline(ib_table); if (prebuilt->clust_index_was_generated) { @@ -6529,6 +6525,8 @@ ha_innobase::info( } for (i = 0; i < table->s->keys; i++) { + ulong j; + if (index == NULL) { sql_print_error("Table %s contains fewer " "indexes inside InnoDB than " @@ -6585,6 +6583,11 @@ ha_innobase::info( } } + if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { + + goto func_exit; + } + if (flag & HA_STATUS_ERRKEY) { ut_a(prebuilt->trx); ut_a(prebuilt->trx->magic_n == TRX_MAGIC_N); @@ -6597,6 +6600,7 @@ ha_innobase::info( stats.auto_increment_value = innobase_peek_autoinc(); } +func_exit: prebuilt->trx->op_info = (char*)""; DBUG_RETURN(0); From fed2531f02d9bd9def3ce75dbfa9f3a41411109a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 24 Aug 2010 11:10:03 +0300 Subject: [PATCH 12/25] Bug#55832: selects crash too easily when innodb_force_recovery>3 dict_update_statistics_low(): Create bogus statistics for those indexes that cannot be accessed because of the innodb_force_recovery setting. ha_innobase::info(): Calculate statistics for each index, even if innodb_force_recovery is set. Fill in bogus data for those indexes that are not accessed because of the innodb_force_recovery setting. --- storage/innodb_plugin/ChangeLog | 5 ++ storage/innodb_plugin/dict/dict0dict.c | 57 +++++++++++++--------- storage/innodb_plugin/handler/ha_innodb.cc | 39 +++++++-------- 3 files changed, 60 insertions(+), 41 deletions(-) diff --git a/storage/innodb_plugin/ChangeLog b/storage/innodb_plugin/ChangeLog index 9fe5c1e7ee6..538cd40dd5b 100644 --- a/storage/innodb_plugin/ChangeLog +++ b/storage/innodb_plugin/ChangeLog @@ -1,3 +1,8 @@ +2010-08-24 The InnoDB Team + + * handler/ha_innodb.c, dict/dict0dict.c: + Fix Bug #55832 selects crash too easily when innodb_force_recovery>3 + 2010-08-03 The InnoDB Team * include/ut0mem.h, ut/ut0mem.c: diff --git a/storage/innodb_plugin/dict/dict0dict.c b/storage/innodb_plugin/dict/dict0dict.c index a79f084e327..560534345f9 100644 --- a/storage/innodb_plugin/dict/dict0dict.c +++ b/storage/innodb_plugin/dict/dict0dict.c @@ -4191,7 +4191,6 @@ dict_update_statistics_low( dictionary mutex */ { dict_index_t* index; - ulint size; ulint sum_of_index_sizes = 0; if (table->ibd_file_missing) { @@ -4206,14 +4205,6 @@ dict_update_statistics_low( return; } - /* If we have set a high innodb_force_recovery level, do not calculate - statistics, as a badly corrupted index can cause a crash in it. */ - - if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { - - return; - } - /* Find out the sizes of the indexes and how many different values for the key they approximately have */ @@ -4225,26 +4216,48 @@ dict_update_statistics_low( return; } - while (index) { - size = btr_get_size(index, BTR_TOTAL_SIZE); - index->stat_index_size = size; + do { + if (UNIV_LIKELY + (srv_force_recovery < SRV_FORCE_NO_IBUF_MERGE + || (srv_force_recovery < SRV_FORCE_NO_LOG_REDO + && dict_index_is_clust(index)))) { + ulint size; + size = btr_get_size(index, BTR_TOTAL_SIZE); - sum_of_index_sizes += size; + index->stat_index_size = size; - size = btr_get_size(index, BTR_N_LEAF_PAGES); + sum_of_index_sizes += size; - if (size == 0) { - /* The root node of the tree is a leaf */ - size = 1; + size = btr_get_size(index, BTR_N_LEAF_PAGES); + + if (size == 0) { + /* The root node of the tree is a leaf */ + size = 1; + } + + index->stat_n_leaf_pages = size; + + btr_estimate_number_of_different_key_vals(index); + } else { + /* If we have set a high innodb_force_recovery + level, do not calculate statistics, as a badly + corrupted index can cause a crash in it. + Initialize some bogus index cardinality + statistics, so that the data can be queried in + various means, also via secondary indexes. */ + ulint i; + + sum_of_index_sizes++; + index->stat_index_size = index->stat_n_leaf_pages = 1; + + for (i = dict_index_get_n_unique(index); i; ) { + index->stat_n_diff_key_vals[i--] = 1; + } } - index->stat_n_leaf_pages = size; - - btr_estimate_number_of_different_key_vals(index); - index = dict_table_get_next_index(index); - } + } while (index); index = dict_table_get_first_index(table); diff --git a/storage/innodb_plugin/handler/ha_innodb.cc b/storage/innodb_plugin/handler/ha_innodb.cc index a70921806b4..c7474109d4f 100644 --- a/storage/innodb_plugin/handler/ha_innodb.cc +++ b/storage/innodb_plugin/handler/ha_innodb.cc @@ -7511,28 +7511,15 @@ ha_innobase::info( dict_index_t* index; ha_rows rec_per_key; ib_int64_t n_rows; - ulong j; - ulong i; char path[FN_REFLEN]; os_file_stat_t stat_info; - DBUG_ENTER("info"); /* If we are forcing recovery at a high level, we will suppress statistics calculation on tables, because that may crash the server if an index is badly corrupted. */ - if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { - - /* We return success (0) instead of HA_ERR_CRASHED, - because we want MySQL to process this query and not - stop, like it would do if it received the error code - HA_ERR_CRASHED. */ - - DBUG_RETURN(0); - } - /* We do not know if MySQL can call this function before calling external_lock(). To be safe, update the thd of the current table handle. */ @@ -7627,12 +7614,18 @@ ha_innobase::info( acquiring latches inside InnoDB, we do not call it if we are asked by MySQL to avoid locking. Another reason to avoid the call is that it uses quite a lot of CPU. - See Bug#38185. - We do not update delete_length if no locking is requested - so the "old" value can remain. delete_length is initialized - to 0 in the ha_statistics' constructor. */ - if (!(flag & HA_STATUS_NO_LOCK)) { - + See Bug#38185. */ + if (flag & HA_STATUS_NO_LOCK) { + /* We do not update delete_length if no + locking is requested so the "old" value can + remain. delete_length is initialized to 0 in + the ha_statistics' constructor. */ + } else if (UNIV_UNLIKELY + (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE)) { + /* Avoid accessing the tablespace if + innodb_crash_recovery is set to a high value. */ + stats.delete_length = 0; + } else { /* lock the data dictionary to avoid races with ibd_file_missing and tablespace_discarded */ row_mysql_lock_data_dictionary(prebuilt->trx); @@ -7677,6 +7670,7 @@ ha_innobase::info( } if (flag & HA_STATUS_CONST) { + ulong i; /* Verify the number of index in InnoDB and MySQL matches up. If prebuilt->clust_index_was_generated holds, InnoDB defines GEN_CLUST_INDEX internally */ @@ -7693,6 +7687,7 @@ ha_innobase::info( } for (i = 0; i < table->s->keys; i++) { + ulong j; /* We could get index quickly through internal index mapping with the index translation table. The identity of index (match up index name with @@ -7758,6 +7753,11 @@ ha_innobase::info( } } + if (srv_force_recovery >= SRV_FORCE_NO_IBUF_MERGE) { + + goto func_exit; + } + if (flag & HA_STATUS_ERRKEY) { const dict_index_t* err_index; @@ -7778,6 +7778,7 @@ ha_innobase::info( stats.auto_increment_value = innobase_peek_autoinc(); } +func_exit: prebuilt->trx->op_info = (char*)""; DBUG_RETURN(0); From e7b268827130b6bc5fa6c2c0f90a850ee309668c Mon Sep 17 00:00:00 2001 From: Alexey Kopytov Date: Tue, 24 Aug 2010 19:51:32 +0400 Subject: [PATCH 13/25] Bug #54802: 'NOT BETWEEN' evaluation is incorrect Queries involving predicates of the form "const NOT BETWEEN not_indexed_column AND indexed_column" could return wrong data due to incorrect handling by the range optimizer. For "c NOT BETWEEN f1 AND f2" predicates, get_mm_tree() produces a disjunction of the SEL_ARG trees for "f1 > c" and "f2 < c". If one of the trees is empty (i.e. one of the arguments is not sargable) the resulting tree should be empty as well, since the whole expression in this case is not sargable. The above logic is implemented in get_mm_tree() as follows. The initial state of the resulting tree is NULL (aka empty). We then iterate through arguments and compute the corresponding SEL_ARG tree (either "f1 > c" or "f2 < c"). If the resulting tree is NULL, it is simply replaced by the generated tree. Otherwise it is replaced by a disjunction of itself and the generated tree. The obvious flaw in this implementation is that if the first argument is not sargable and thus produces a NULL tree, the resulting tree will simply be replaced by the tree for the second argument. As a result, "c NOT BETWEEN f1 AND f2" will end up as just "f2 < c". Fixed by adding a check so that when the first argument produces an empty tree for the NOT BETWEEN case, the loop is aborted with an empty tree as a result. The whole idea of using a loop for 2 arguments does not make much sense, but it was probably used to avoid code duplication for several BETWEEN variants. --- mysql-test/r/range.result | 13 +++++++++++++ mysql-test/t/range.test | 12 ++++++++++++ sql/opt_range.cc | 4 ++++ 3 files changed, 29 insertions(+) diff --git a/mysql-test/r/range.result b/mysql-test/r/range.result index 64e00521cd2..d989896514c 100644 --- a/mysql-test/r/range.result +++ b/mysql-test/r/range.result @@ -1653,4 +1653,17 @@ a b 0 0 1 1 DROP TABLE t1; +# +# Bug #54802: 'NOT BETWEEN' evaluation is incorrect +# +CREATE TABLE t1 (c_key INT, c_notkey INT, KEY(c_key)); +INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); +EXPLAIN SELECT * FROM t1 WHERE 2 NOT BETWEEN c_notkey AND c_key; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL c_key NULL NULL NULL 3 Using where +SELECT * FROM t1 WHERE 2 NOT BETWEEN c_notkey AND c_key; +c_key c_notkey +1 1 +3 3 +DROP TABLE t1; End of 5.1 tests diff --git a/mysql-test/t/range.test b/mysql-test/t/range.test index 5d5ad180f1a..0ad3d3e8504 100644 --- a/mysql-test/t/range.test +++ b/mysql-test/t/range.test @@ -1313,4 +1313,16 @@ SELECT * FROM t1 FORCE INDEX (PRIMARY) DROP TABLE t1; +--echo # +--echo # Bug #54802: 'NOT BETWEEN' evaluation is incorrect +--echo # + +CREATE TABLE t1 (c_key INT, c_notkey INT, KEY(c_key)); +INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3); + +EXPLAIN SELECT * FROM t1 WHERE 2 NOT BETWEEN c_notkey AND c_key; +SELECT * FROM t1 WHERE 2 NOT BETWEEN c_notkey AND c_key; + +DROP TABLE t1; + --echo End of 5.1 tests diff --git a/sql/opt_range.cc b/sql/opt_range.cc index 1f42d0567a7..eae79e63c19 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -5526,7 +5526,11 @@ static SEL_TREE *get_mm_tree(RANGE_OPT_PARAM *param,COND *cond) SEL_TREE *tmp= get_full_func_mm_tree(param, cond_func, field_item, (Item*)(intptr)i, inv); if (inv) + { tree= !tree ? tmp : tree_or(param, tree, tmp); + if (tree == NULL) + break; + } else tree= tree_and(param, tree, tmp); } From 76907f8591d1c8d1fa91eca388304b3fb4d7d4c8 Mon Sep 17 00:00:00 2001 From: Bjorn Munch Date: Wed, 25 Aug 2010 10:23:19 +0200 Subject: [PATCH 14/25] Bug #52301 Add --protocol to mysqltest Added code resulted in strange linking problem for embedded on Windows Avoided by not doing this for embedded mode It's irrelevant for embedded server anyway, --protocol will be ignored --- client/mysqltest.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index d7a302912b4..756208a0f96 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -5220,8 +5220,10 @@ void do_connect(struct st_command *command) } #endif +#ifndef EMBEDDED_LIBRARY if (opt_protocol) mysql_options(&con_slot->mysql, MYSQL_OPT_PROTOCOL, (char*) &opt_protocol); +#endif #ifdef HAVE_SMEM if (con_shm) @@ -6179,8 +6181,10 @@ get_one_option(int optid, const struct my_option *opt, print_version(); exit(0); case OPT_MYSQL_PROTOCOL: +#ifndef EMBEDDED_LIBRARY opt_protocol= find_type_or_exit(argument, &sql_protocol_typelib, opt->name); +#endif break; case '?': usage(); @@ -7932,8 +7936,10 @@ int main(int argc, char **argv) mysql_options(&con->mysql, MYSQL_SET_CHARSET_DIR, opt_charsets_dir); +#ifndef EMBEDDED_LIBRARY if (opt_protocol) mysql_options(&con->mysql,MYSQL_OPT_PROTOCOL,(char*)&opt_protocol); +#endif #ifdef HAVE_OPENSSL From 800feb16cb9e3fcb363f0856581c8822bd29d549 Mon Sep 17 00:00:00 2001 From: Dmitry Shulga Date: Wed, 25 Aug 2010 15:47:45 +0700 Subject: [PATCH 15/25] Fixed bug #29751 - do not rename the error log at FLUSH LOGS. Added open log file with FILE_SHARE_DELETE flag on Windows. sql/log.cc: added reopen_fstreams(); modified redirect_std_streams(): call to sequence of freopen() replaced to reopen_fstreams(); modified flush_error_log(): removed file rename for flushed error log file. sql/mysqld.cc: modified main() and init_server_components(): do open log error file over call to reopen_fstreams(). --- sql/log.cc | 117 ++++++++++++++++++++++++++++++-------------------- sql/mysqld.cc | 19 +++++--- 2 files changed, 82 insertions(+), 54 deletions(-) diff --git a/sql/log.cc b/sql/log.cc index 3f41bf1c929..156c293e3aa 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -5063,70 +5063,93 @@ void sql_perror(const char *message) } +#ifdef __WIN__ +extern "C" my_bool reopen_fstreams(const char *filename, + FILE *outstream, FILE *errstream) +{ + int handle_fd; + int stream_fd; + HANDLE osfh; + + DBUG_ASSERT(filename && (outstream || errstream)); + + if ((osfh= CreateFile(filename, GENERIC_READ | GENERIC_WRITE, + FILE_SHARE_READ | FILE_SHARE_WRITE | + FILE_SHARE_DELETE, NULL, + OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, + NULL)) == INVALID_HANDLE_VALUE) + return TRUE; + + if ((handle_fd= _open_osfhandle((intptr_t)osfh, + _O_APPEND | _O_TEXT)) == -1) + { + CloseHandle(osfh); + return TRUE; + } + + if (outstream) + { + stream_fd= _fileno(outstream); + if (_dup2(handle_fd, stream_fd) < 0) + { + CloseHandle(osfh); + return TRUE; + } + } + + if (errstream) + { + stream_fd= _fileno(errstream); + if (_dup2(handle_fd, stream_fd) < 0) + { + CloseHandle(osfh); + return TRUE; + } + } + + _close(handle_fd); + return FALSE; +} +#else +extern "C" my_bool reopen_fstreams(const char *filename, + FILE *outstream, FILE *errstream) +{ + if (outstream && !freopen(filename, "a+", outstream)) + return TRUE; + + if (errstream && !freopen(filename, "a+", errstream)) + return TRUE; + + return FALSE; +} +#endif + + /* Unfortunately, there seems to be no good way to restore the original streams upon failure. */ static bool redirect_std_streams(const char *file) { - if (freopen(file, "a+", stdout) && freopen(file, "a+", stderr)) - { - setbuf(stderr, NULL); - return FALSE; - } + if (reopen_fstreams(file, stdout, stderr)) + return TRUE; - return TRUE; + setbuf(stderr, NULL); + return FALSE; } bool flush_error_log() { - bool result=0; + bool result= 0; if (opt_error_log) { - char err_renamed[FN_REFLEN], *end; - end= strmake(err_renamed,log_error_file,FN_REFLEN-5); - strmov(end, "-old"); VOID(pthread_mutex_lock(&LOCK_error_log)); -#ifdef __WIN__ - char err_temp[FN_REFLEN+5]; - /* - On Windows is necessary a temporary file for to rename - the current error file. - */ - strxmov(err_temp, err_renamed,"-tmp",NullS); - (void) my_delete(err_temp, MYF(0)); - if (freopen(err_temp,"a+",stdout)) - { - int fd; - size_t bytes; - uchar buf[IO_SIZE]; - - freopen(err_temp,"a+",stderr); - setbuf(stderr, NULL); - (void) my_delete(err_renamed, MYF(0)); - my_rename(log_error_file,err_renamed,MYF(0)); - redirect_std_streams(log_error_file); - - if ((fd = my_open(err_temp, O_RDONLY, MYF(0))) >= 0) - { - while ((bytes= my_read(fd, buf, IO_SIZE, MYF(0))) && - bytes != MY_FILE_ERROR) - my_fwrite(stderr, buf, bytes, MYF(0)); - my_close(fd, MYF(0)); - } - (void) my_delete(err_temp, MYF(0)); - } - else - result= 1; -#else - my_rename(log_error_file,err_renamed,MYF(0)); - if (redirect_std_streams(log_error_file)) - result= 1; -#endif + if (redirect_std_streams(log_error_file)) + result= 1; VOID(pthread_mutex_unlock(&LOCK_error_log)); } - return result; + return result; } void MYSQL_BIN_LOG::signal_update() diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 08407d52e09..d9c4c7fc3f5 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -199,6 +199,9 @@ typedef fp_except fp_except_t; # endif #endif +extern "C" my_bool reopen_fstreams(const char *filename, + FILE *outstream, FILE *errstream); + inline void setup_fpu() { #if defined(__FreeBSD__) && defined(HAVE_IEEEFP_H) @@ -3821,13 +3824,15 @@ static int init_server_components() opt_error_log= 1; // Too long file name else { + my_bool res; #ifndef EMBEDDED_LIBRARY - if (freopen(log_error_file, "a+", stdout)) + res= reopen_fstreams(log_error_file, stdout, stderr); +#else + res= reopen_fstreams(log_error_file, NULL, stderr); #endif - { - if (freopen(log_error_file, "a+", stderr)) - setbuf(stderr, NULL); - } + + if (!res) + setbuf(stderr, NULL); } } @@ -4475,8 +4480,8 @@ we force server id to 2, but this MySQL server will not act as a slave."); #ifdef __WIN__ if (!opt_console) { - freopen(log_error_file,"a+",stdout); - freopen(log_error_file,"a+",stderr); + if (reopen_fstreams(log_error_file, stdout, stderr)) + unireg_abort(1); setbuf(stderr, NULL); FreeConsole(); // Remove window } From 09c62e4ff81f0091608f67cf86ea219d40df4a1a Mon Sep 17 00:00:00 2001 From: Bjorn Munch Date: Wed, 25 Aug 2010 16:34:31 +0200 Subject: [PATCH 16/25] Cherry pick 55501 --- mysql-test/include/have_innodb_plugin.inc | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql-test/include/have_innodb_plugin.inc b/mysql-test/include/have_innodb_plugin.inc index 99a79465f52..df876deb2d7 100644 --- a/mysql-test/include/have_innodb_plugin.inc +++ b/mysql-test/include/have_innodb_plugin.inc @@ -1,3 +1,4 @@ +--source include/not_embedded.inc disable_query_log; --require r/true.require SELECT (plugin_library LIKE 'ha_innodb_plugin%') AS `TRUE` FROM information_schema.plugins WHERE LOWER(plugin_name) = 'innodb' AND LOWER(plugin_status) = 'active'; From b4dc600af919f8a7fa8403535847256161abc1db Mon Sep 17 00:00:00 2001 From: Evgeny Potemkin Date: Thu, 26 Aug 2010 13:31:04 +0400 Subject: [PATCH 17/25] Bug #55656: mysqldump can be slower after bug 39653 fix. After fix for bug 39653 the shortest available secondary index was used for full table scan. Primary clustered key was used only if no secondary index can be used. However, when chosen secondary index includes all fields of the table being scanned it's better to use primary index since the amount of data to scan is the same but the primary index is clustered. Now the find_shortest_key function takes this into account. mysql-test/suite/innodb/r/innodb_mysql.result: Added a test case for the bug#55656. mysql-test/suite/innodb/t/innodb_mysql.test: Added a test case for the bug#55656. sql/sql_select.cc: Bug #55656: mysqldump can be slower after bug #39653 fix. The find_shortest_key function now prefers clustered primary key if found secondary key includes all fields of the table. --- mysql-test/suite/innodb/r/innodb_mysql.result | 58 ++++++++++++++++++ mysql-test/suite/innodb/t/innodb_mysql.test | 25 ++++++++ sql/sql_select.cc | 60 +++++++++++++------ 3 files changed, 125 insertions(+), 18 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb_mysql.result b/mysql-test/suite/innodb/r/innodb_mysql.result index 23619b91fbc..ba8ac0ba86c 100644 --- a/mysql-test/suite/innodb/r/innodb_mysql.result +++ b/mysql-test/suite/innodb/r/innodb_mysql.result @@ -2529,4 +2529,62 @@ SELECT * FROM t1 FOR UPDATE; SELECT * FROM t1 GROUP BY (SELECT a FROM t2 LIMIT 1 FOR UPDATE) + t1.a; ERROR 40001: Deadlock found when trying to get lock; try restarting transaction DROP TABLE t1,t2; +# +# Bug#55656: mysqldump can be slower after bug #39653 fix +# +CREATE TABLE t1 (a INT , b INT, c INT, d INT, +KEY (b), PRIMARY KEY (a,b)) ENGINE=INNODB; +INSERT INTO t1 VALUES (1,1,1,1), (2,2,2,2), (3,3,3,3); +EXPLAIN SELECT COUNT(*) FROM t1; +id 1 +select_type SIMPLE +table t1 +type index +possible_keys NULL +key b +key_len 4 +ref NULL +rows 3 +Extra Using index +DROP INDEX b ON t1; +CREATE INDEX b ON t1(a,b); +EXPLAIN SELECT COUNT(*) FROM t1; +id 1 +select_type SIMPLE +table t1 +type index +possible_keys NULL +key b +key_len 8 +ref NULL +rows 3 +Extra Using index +DROP INDEX b ON t1; +CREATE INDEX b ON t1(a,b,c); +EXPLAIN SELECT COUNT(*) FROM t1; +id 1 +select_type SIMPLE +table t1 +type index +possible_keys NULL +key b +key_len 13 +ref NULL +rows 3 +Extra Using index +DROP INDEX b ON t1; +CREATE INDEX b ON t1(a,b,c,d); +EXPLAIN SELECT COUNT(*) FROM t1; +id 1 +select_type SIMPLE +table t1 +type index +possible_keys NULL +key PRIMARY +key_len 8 +ref NULL +rows 3 +Extra Using index +DROP TABLE t1; +# End of 5.1 tests diff --git a/mysql-test/suite/innodb/t/innodb_mysql.test b/mysql-test/suite/innodb/t/innodb_mysql.test index d633cb2222e..4f13e15a627 100644 --- a/mysql-test/suite/innodb/t/innodb_mysql.test +++ b/mysql-test/suite/innodb/t/innodb_mysql.test @@ -781,5 +781,30 @@ disconnect con2; DROP TABLE t1,t2; +--echo # +--echo # Bug#55656: mysqldump can be slower after bug #39653 fix +--echo # + +CREATE TABLE t1 (a INT , b INT, c INT, d INT, + KEY (b), PRIMARY KEY (a,b)) ENGINE=INNODB; +INSERT INTO t1 VALUES (1,1,1,1), (2,2,2,2), (3,3,3,3); +--query_vertical EXPLAIN SELECT COUNT(*) FROM t1 + +DROP INDEX b ON t1; +CREATE INDEX b ON t1(a,b); +--query_vertical EXPLAIN SELECT COUNT(*) FROM t1 + +DROP INDEX b ON t1; +CREATE INDEX b ON t1(a,b,c); +--query_vertical EXPLAIN SELECT COUNT(*) FROM t1 + +DROP INDEX b ON t1; +CREATE INDEX b ON t1(a,b,c,d); +--query_vertical EXPLAIN SELECT COUNT(*) FROM t1 + +DROP TABLE t1; + +--echo # + --echo End of 5.1 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 7ee1762295f..4a32ca34790 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -13017,6 +13017,34 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx, } +/** + Find shortest key suitable for full table scan. + + @param table Table to scan + @param usable_keys Allowed keys + + @note + As far as + 1) clustered primary key entry data set is a set of all record + fields (key fields and not key fields) and + 2) secondary index entry data is a union of its key fields and + primary key fields (at least InnoDB and its derivatives don't + duplicate primary key fields there, even if the primary and + the secondary keys have a common subset of key fields), + then secondary index entry data is always a subset of primary key entry. + Unfortunately, key_info[nr].key_length doesn't show the length + of key/pointer pair but a sum of key field lengths only, thus + we can't estimate index IO volume comparing only this key_length + value of secondary keys and clustered PK. + So, try secondary keys first, and choose PK only if there are no + usable secondary covering keys or found best secondary key include + all table fields (i.e. same as PK): + + @return + MAX_KEY no suitable key found + key index otherwise +*/ + uint find_shortest_key(TABLE *table, const key_map *usable_keys) { uint best= MAX_KEY; @@ -13029,23 +13057,6 @@ uint find_shortest_key(TABLE *table, const key_map *usable_keys) uint min_length= (uint) ~0; for (uint nr=0; nr < table->s->keys ; nr++) { - /* - As far as - 1) clustered primary key entry data set is a set of all record - fields (key fields and not key fields) and - 2) secondary index entry data is a union of its key fields and - primary key fields (at least InnoDB and its derivatives don't - duplicate primary key fields there, even if the primary and - the secondary keys have a common subset of key fields), - then secondary index entry data is always a subset of primary key - entry, and the PK is always longer. - Unfortunately, key_info[nr].key_length doesn't show the length - of key/pointer pair but a sum of key field lengths only, thus - we can't estimate index IO volume comparing only this key_length - value of seconday keys and clustered PK. - So, try secondary keys first, and choose PK only if there are no - usable secondary covering keys: - */ if (nr == usable_clustered_pk) continue; if (usable_keys->is_set(nr)) @@ -13058,7 +13069,20 @@ uint find_shortest_key(TABLE *table, const key_map *usable_keys) } } } - return best != MAX_KEY ? best : usable_clustered_pk; + if (usable_clustered_pk != MAX_KEY) + { + /* + If the primary key is clustered and found shorter key covers all table + fields then primary key scan normally would be faster because amount of + data to scan is the same but PK is clustered. + It's safe to compare key parts with table fields since duplicate key + parts aren't allowed. + */ + if (best == MAX_KEY || + table->key_info[best].key_parts >= table->s->fields) + best= usable_clustered_pk; + } + return best; } /** From d1fb4ba2995b3a0364d1a7bca21a20a203b5bfa1 Mon Sep 17 00:00:00 2001 From: Sergey Vojtovich Date: Thu, 26 Aug 2010 15:23:44 +0400 Subject: [PATCH 18/25] Fixed race condition in a test case for BUG#55580. --- mysql-test/suite/innodb/t/innodb_mysql.test | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mysql-test/suite/innodb/t/innodb_mysql.test b/mysql-test/suite/innodb/t/innodb_mysql.test index 4f13e15a627..ae03bebfbe3 100644 --- a/mysql-test/suite/innodb/t/innodb_mysql.test +++ b/mysql-test/suite/innodb/t/innodb_mysql.test @@ -768,9 +768,14 @@ START TRANSACTION; SELECT * FROM t1 LOCK IN SHARE MODE; connection con1; +let $conn_id= `SELECT CONNECTION_ID()`; --send SELECT * FROM t1 FOR UPDATE connection con2; +let $wait_timeout= 2; +let $wait_condition= SELECT 1 FROM INFORMATION_SCHEMA.PROCESSLIST +WHERE ID=$conn_id AND STATE='Sending data'; +--source include/wait_condition.inc --echo # should not crash --error ER_LOCK_DEADLOCK SELECT * FROM t1 GROUP BY (SELECT a FROM t2 LIMIT 1 FOR UPDATE) + t1.a; From a4154bd0a04ee863c07848e77ee727b4dc8244e8 Mon Sep 17 00:00:00 2001 From: Bjorn Munch Date: Thu, 26 Aug 2010 15:14:50 +0200 Subject: [PATCH 19/25] Reduced #ifndef for 52301, caused compile warning --- client/mysqltest.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index c39ba4ff6ce..07eb66bc2b8 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -5220,10 +5220,8 @@ void do_connect(struct st_command *command) } #endif -#ifndef EMBEDDED_LIBRARY if (opt_protocol) mysql_options(&con_slot->mysql, MYSQL_OPT_PROTOCOL, (char*) &opt_protocol); -#endif #ifdef HAVE_SMEM if (con_shm) @@ -7936,10 +7934,8 @@ int main(int argc, char **argv) mysql_options(&con->mysql, MYSQL_SET_CHARSET_DIR, opt_charsets_dir); -#ifndef EMBEDDED_LIBRARY if (opt_protocol) mysql_options(&con->mysql,MYSQL_OPT_PROTOCOL,(char*)&opt_protocol); -#endif #ifdef HAVE_OPENSSL From 6b246aaf8d61d3cf6822401cada37970e8f2467a Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 26 Aug 2010 18:06:07 +0300 Subject: [PATCH 20/25] Increment InnoDB Plugin version to 1.0.12. InnoDB Plugin 1.0.11 has been released with MySQL 5.1.50. --- storage/innodb_plugin/include/univ.i | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innodb_plugin/include/univ.i b/storage/innodb_plugin/include/univ.i index aa56c18e44e..66b712a0355 100644 --- a/storage/innodb_plugin/include/univ.i +++ b/storage/innodb_plugin/include/univ.i @@ -46,7 +46,7 @@ Created 1/20/1994 Heikki Tuuri #define INNODB_VERSION_MAJOR 1 #define INNODB_VERSION_MINOR 0 -#define INNODB_VERSION_BUGFIX 11 +#define INNODB_VERSION_BUGFIX 12 /* The following is the InnoDB version as shown in SELECT plugin_version FROM information_schema.plugins; From 7ebd2cd797f25013e2d2492ef6faa01fbf3cefc2 Mon Sep 17 00:00:00 2001 From: Ramil Kalimullin Date: Fri, 27 Aug 2010 11:44:06 +0400 Subject: [PATCH 21/25] Fix for bug #54253: memory leak when using I_S plugins w/o deinit method Free memory allocated by the server for all plugins, with or without deinit() method. --- sql/sql_show.cc | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 3e66cda16e1..e074461b452 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -6930,13 +6930,16 @@ int finalize_schema_table(st_plugin_int *plugin) ST_SCHEMA_TABLE *schema_table= (ST_SCHEMA_TABLE *)plugin->data; DBUG_ENTER("finalize_schema_table"); - if (schema_table && plugin->plugin->deinit) + if (schema_table) { - DBUG_PRINT("info", ("Deinitializing plugin: '%s'", plugin->name.str)); - if (plugin->plugin->deinit(NULL)) + if (plugin->plugin->deinit) { - DBUG_PRINT("warning", ("Plugin '%s' deinit function returned error.", - plugin->name.str)); + DBUG_PRINT("info", ("Deinitializing plugin: '%s'", plugin->name.str)); + if (plugin->plugin->deinit(NULL)) + { + DBUG_PRINT("warning", ("Plugin '%s' deinit function returned error.", + plugin->name.str)); + } } my_free(schema_table, MYF(0)); } From d7d0f6390b8d95b34ffb79f1a77ab11657e064a8 Mon Sep 17 00:00:00 2001 From: Alexey Kopytov Date: Fri, 27 Aug 2010 13:44:35 +0400 Subject: [PATCH 22/25] Bug #54465: assert: field_types == 0 || field_types[field_pos] == MYSQL_TYPE_LONGLONG A MIN/MAX() function with a subquery as its argument could lead to a debug assertion on debug builds or wrong data on release ones. The problem was a combination of the following factors: - Item_sum_hybrid::fix_fields() might use the argument (args[0]) to calculate 'hybrid_field_type' which was later used to decide how the data should be sent to the client. - Item_sum::make_field() might use the argument again to calculate the field's type when sending result set metadata to the client. - The argument could be changed in between these two calls via Item::set_arg() leading to inconsistent metadata being reported. Here is what was happening for the bug's test case: 1. Item_sum_hybrid::fix_fields() calculates hybrid_field_type as MYSQL_TYPE_LONGLONG based on args[0] which is an Item::SUBSELECT_ITEM at that time. 2. A temporary table is created to execute the query. create_tmp_field_from_item() creates a Field_long object according to the subselect's max_length. 3. The subselect item in Item_sum_hybrid is replaced by the Item_field object referencing the newly created Field_long. 4. Item_sum::make_field() rightfully returns the MYSQL_TYPE_LONG type when calculating the result set metadata. 5. When sending the actual data, Item::send() relies on the virtual field_type() function which in our case returns previously calculated hybrid_field_type == MYSQL_TYPE_LONGLONG. It looks like the only solution is to never refer to the argument's metadata after the result metadata has been calculated in fix_fields(), since the argument itself may be different by then. In this sense, Item_sum::make_field() should never be used, because it may rely on the argument's metadata and is only called after fix_fields(). The "default" implementation in Item::make_field() should be used instead as it relies only on field_type(), but not on the argument's type. Fixed by removing Item_sum::make_field() so that the superclass implementation Item::make_field() is always used. mysql-test/r/func_group.result: Added a test case for bug #54465. mysql-test/t/func_group.test: Added a test case for bug #54465. sql/item_sum.cc: Removed Item_sum::make_field() so that the superclass implementation Item::make_field() is always used. sql/item_sum.h: Removed Item_sum::make_field() so that the superclass implementation Item::make_field() is always used. --- mysql-test/r/func_group.result | 11 +++++++++++ mysql-test/t/func_group.test | 14 ++++++++++++++ sql/item_sum.cc | 20 -------------------- sql/item_sum.h | 1 - 4 files changed, 25 insertions(+), 21 deletions(-) diff --git a/mysql-test/r/func_group.result b/mysql-test/r/func_group.result index b36f561578b..606f879b47f 100644 --- a/mysql-test/r/func_group.result +++ b/mysql-test/r/func_group.result @@ -1713,4 +1713,15 @@ f1 f2 f3 f4 f1 = f2 NULL NULL NULL NULL NULL drop table t1; # +# Bug #54465: assert: field_types == 0 || field_types[field_pos] == +# MYSQL_TYPE_LONGLONG +# +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); +SELECT MAX((SELECT 1 FROM t1 ORDER BY @var LIMIT 1)) m FROM t1 t2, t1 +ORDER BY t1.a; +m +1 +DROP TABLE t1; +# End of 5.1 tests diff --git a/mysql-test/t/func_group.test b/mysql-test/t/func_group.test index 6dbc8a05789..72a78f612a2 100644 --- a/mysql-test/t/func_group.test +++ b/mysql-test/t/func_group.test @@ -1082,6 +1082,20 @@ select a.f1 as a, b.f4 as b, a.f1 > b.f4 as gt, from t1 a, t1 b; select *, f1 = f2 from t1; drop table t1; + +--echo # +--echo # Bug #54465: assert: field_types == 0 || field_types[field_pos] == +--echo # MYSQL_TYPE_LONGLONG +--echo # + +CREATE TABLE t1 (a INT); +INSERT INTO t1 VALUES (1), (2); + +SELECT MAX((SELECT 1 FROM t1 ORDER BY @var LIMIT 1)) m FROM t1 t2, t1 + ORDER BY t1.a; + +DROP TABLE t1; + --echo # --echo End of 5.1 tests diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 25b3bd5d91d..ae9e46e2abf 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -417,26 +417,6 @@ void Item_sum::mark_as_sum_func() } -void Item_sum::make_field(Send_field *tmp_field) -{ - if (args[0]->type() == Item::FIELD_ITEM && keep_field_type()) - { - ((Item_field*) args[0])->field->make_field(tmp_field); - /* For expressions only col_name should be non-empty string. */ - char *empty_string= (char*)""; - tmp_field->db_name= empty_string; - tmp_field->org_table_name= empty_string; - tmp_field->table_name= empty_string; - tmp_field->org_col_name= empty_string; - tmp_field->col_name= name; - if (maybe_null) - tmp_field->flags&= ~NOT_NULL_FLAG; - } - else - init_make_field(tmp_field, field_type()); -} - - void Item_sum::print(String *str, enum_query_type query_type) { /* orig_args is not filled with valid values until fix_fields() */ diff --git a/sql/item_sum.h b/sql/item_sum.h index fe05858ab1d..26290a812f4 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -339,7 +339,6 @@ public: forced_const= TRUE; } virtual bool const_item() const { return forced_const; } - void make_field(Send_field *field); virtual void print(String *str, enum_query_type query_type); void fix_num_length_and_dec(); From 6a113b215abde05ffe602111f54c2aef8e78bbd1 Mon Sep 17 00:00:00 2001 From: Ramil Kalimullin Date: Mon, 30 Aug 2010 11:51:46 +0400 Subject: [PATCH 23/25] Fix for bug #51875: crash when loading data into geometry function polyfromwkb Check for number of line strings in the incoming polygon data (wkb) and for number of points in the incoming linestring wkb. mysql-test/r/gis.result: Fix for bug #51875: crash when loading data into geometry function polyfromwkb - test result. mysql-test/t/gis.test: Fix for bug #51875: crash when loading data into geometry function polyfromwkb - test case. sql/spatial.cc: Fix for bug #51875: crash when loading data into geometry function polyfromwkb - creating a polygon from wkb check for number of line strings, - creating a linestring from wkb check for number of line points. --- mysql-test/r/gis.result | 7 +++++++ mysql-test/t/gis.test | 10 ++++++++++ sql/spatial.cc | 6 ++++-- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/gis.result b/mysql-test/r/gis.result index 3e28227d542..d39afa6f315 100644 --- a/mysql-test/r/gis.result +++ b/mysql-test/r/gis.result @@ -1057,4 +1057,11 @@ NULL SELECT Polygon(12345123,''); Polygon(12345123,'') NULL +# +# BUG#51875: crash when loading data into geometry function polyfromwkb +# +SET @a=0x00000000030000000100000000000000000000000000144000000000000014400000000000001840000000000000184000000000000014400000000000001440; +SET @a=POLYFROMWKB(@a); +SET @a=0x00000000030000000000000000000000000000000000144000000000000014400000000000001840000000000000184000000000000014400000000000001440; +SET @a=POLYFROMWKB(@a); End of 5.1 tests diff --git a/mysql-test/t/gis.test b/mysql-test/t/gis.test index bc0695aaa93..236b31efb79 100644 --- a/mysql-test/t/gis.test +++ b/mysql-test/t/gis.test @@ -722,4 +722,14 @@ SELECT Polygon(123451,''); SELECT Polygon(1234512,''); SELECT Polygon(12345123,''); + +--echo # +--echo # BUG#51875: crash when loading data into geometry function polyfromwkb +--echo # +SET @a=0x00000000030000000100000000000000000000000000144000000000000014400000000000001840000000000000184000000000000014400000000000001440; +SET @a=POLYFROMWKB(@a); +SET @a=0x00000000030000000000000000000000000000000000144000000000000014400000000000001840000000000000184000000000000014400000000000001440; +SET @a=POLYFROMWKB(@a); + + --echo End of 5.1 tests diff --git a/sql/spatial.cc b/sql/spatial.cc index 2305a8eb97d..8b869a5b1ca 100644 --- a/sql/spatial.cc +++ b/sql/spatial.cc @@ -528,7 +528,7 @@ uint Gis_line_string::init_from_wkb(const char *wkb, uint len, n_points= wkb_get_uint(wkb, bo); proper_length= 4 + n_points * POINT_DATA_SIZE; - if (len < proper_length || res->reserve(proper_length)) + if (!n_points || len < proper_length || res->reserve(proper_length)) return 0; res->q_append(n_points); @@ -746,7 +746,9 @@ uint Gis_polygon::init_from_wkb(const char *wkb, uint len, wkbByteOrder bo, if (len < 4) return 0; - n_linear_rings= wkb_get_uint(wkb, bo); + if (!(n_linear_rings= wkb_get_uint(wkb, bo))) + return 0; + if (res->reserve(4, 512)) return 0; wkb+= 4; From ccab4d8771b728e334b26d0be1150b4ecf0bc7a6 Mon Sep 17 00:00:00 2001 From: Gleb Shchepa Date: Tue, 31 Aug 2010 02:16:38 +0400 Subject: [PATCH 24/25] Bug #53034: Multiple-table DELETE statements not accepting "Access compatibility" syntax The "wild" "DELETE FROM table_name.* ... USING ..." syntax for multi-table DELETE statements is documented but it was lost in the fix for the bug 30234. The table_ident_opt_wild parser rule has been added to restore the lost syntax. mysql-test/r/delete.result: Test case for bug #53034. mysql-test/t/delete.test: Test case for bug #53034. sql/sql_yacc.yy: Bug #53034: Multiple-table DELETE statements not accepting "Access compatibility" syntax The table_ident_opt_wild parser rule has been added to restore the lost syntax. Note: simple extending of table_ident with opt_wild in the table_alias_ref rule is not acceptable, because a) it adds one conflict more and b) this conflict resolves in the inappropriate way. --- mysql-test/r/delete.result | 9 +++++++++ mysql-test/t/delete.test | 13 +++++++++++++ sql/sql_yacc.yy | 18 +++++++++++++++++- 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/delete.result b/mysql-test/r/delete.result index 36025cbfb35..7d6fc30be67 100644 --- a/mysql-test/r/delete.result +++ b/mysql-test/r/delete.result @@ -358,4 +358,13 @@ INDEX(a), INDEX(b), INDEX(c)); INSERT INTO t1 VALUES (1,2,3), (4,5,6), (7,8,9); DELETE FROM t1 WHERE a = 10 OR b = 20 ORDER BY c LIMIT 1; DROP TABLE t1; +# +# Bug #53034: Multiple-table DELETE statements not accepting +# "Access compatibility" syntax +# +CREATE TABLE t1 (id INT); +CREATE TABLE t2 LIKE t1; +CREATE TABLE t3 LIKE t1; +DELETE FROM t1.*, test.t2.*, a.* USING t1, t2, t3 AS a; +DROP TABLE t1, t2, t3; End of 5.1 tests diff --git a/mysql-test/t/delete.test b/mysql-test/t/delete.test index 5a0e86568f3..ea5c87babbb 100644 --- a/mysql-test/t/delete.test +++ b/mysql-test/t/delete.test @@ -387,4 +387,17 @@ DELETE FROM t1 WHERE a = 10 OR b = 20 ORDER BY c LIMIT 1; DROP TABLE t1; +--echo # +--echo # Bug #53034: Multiple-table DELETE statements not accepting +--echo # "Access compatibility" syntax +--echo # + +CREATE TABLE t1 (id INT); +CREATE TABLE t2 LIKE t1; +CREATE TABLE t3 LIKE t1; + +DELETE FROM t1.*, test.t2.*, a.* USING t1, t2, t3 AS a; + +DROP TABLE t1, t2, t3; + --echo End of 5.1 tests diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index ed367582ba5..aa41a408e5b 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -1297,6 +1297,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); %type table_ident table_ident_nodb references xid + table_ident_opt_wild %type remember_name remember_end opt_ident opt_db text_or_password @@ -9622,7 +9623,7 @@ table_alias_ref_list: ; table_alias_ref: - table_ident + table_ident_opt_wild { if (!Select->add_table_to_list(YYTHD, $1, NULL, TL_OPTION_UPDATING | TL_OPTION_ALIAS, @@ -11405,6 +11406,21 @@ table_ident: } ; +table_ident_opt_wild: + ident opt_wild + { + $$= new Table_ident($1); + if ($$ == NULL) + MYSQL_YYABORT; + } + | ident '.' ident opt_wild + { + $$= new Table_ident(YYTHD, $1,$3,0); + if ($$ == NULL) + MYSQL_YYABORT; + } + ; + table_ident_nodb: ident { From 8822ecff2dea2e83fdad84d7a9307e8aa5905bce Mon Sep 17 00:00:00 2001 From: Davi Arnaut Date: Tue, 24 Aug 2010 10:48:45 -0300 Subject: [PATCH 25/25] Bug#55846: Link tests fail on Windows - my_compiler.h missing Make the my_compiler.h header, like my_attribute.h, part of the distribution. This is required due to the dependency of the former on the latter (which can undefine __attribute__). --- include/Makefile.am | 5 +++-- scripts/make_win_bin_dist | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/include/Makefile.am b/include/Makefile.am index 182011c25a3..08532db1731 100644 --- a/include/Makefile.am +++ b/include/Makefile.am @@ -26,7 +26,8 @@ pkginclude_HEADERS = $(HEADERS_ABI) my_dbug.h m_string.h my_sys.h \ decimal.h errmsg.h my_global.h my_net.h \ my_getopt.h sslopt-longopts.h my_dir.h \ sslopt-vars.h sslopt-case.h sql_common.h keycache.h \ - m_ctype.h my_attribute.h $(HEADERS_GEN_CONFIGURE) \ + m_ctype.h my_attribute.h my_compiler.h \ + $(HEADERS_GEN_CONFIGURE) \ $(HEADERS_GEN_MAKE) noinst_HEADERS = config-win.h config-netware.h my_bit.h \ @@ -37,7 +38,7 @@ noinst_HEADERS = config-win.h config-netware.h my_bit.h \ my_aes.h my_tree.h my_trie.h hash.h thr_alarm.h \ thr_lock.h t_ctype.h violite.h my_md5.h base64.h \ my_handler.h my_time.h my_vle.h my_user.h \ - my_libwrap.h my_stacktrace.h my_compiler.h + my_libwrap.h my_stacktrace.h EXTRA_DIST = mysql.h.pp mysql/plugin.h.pp diff --git a/scripts/make_win_bin_dist b/scripts/make_win_bin_dist index a08a41bb283..22970c95ddd 100755 --- a/scripts/make_win_bin_dist +++ b/scripts/make_win_bin_dist @@ -263,6 +263,7 @@ cp include/mysql.h \ include/keycache.h \ include/m_ctype.h \ include/my_attribute.h \ + include/my_compiler.h \ include/mysqld_error.h \ include/sql_state.h \ include/mysqld_ername.h \