From 316847eab72022cd11351ea1bfa91234bee75839 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 22 Aug 2022 11:50:15 +0400 Subject: MDEV-27101 Subquery using the ALL keyword on TIMESTAMP columns produces a wrong result TIMESTAMP columns were compared as strings in ALL/ANY comparison, which did not work well near DST time change. Changing ALL/ANY comparison to use "Native" representation to compare TIMESTAMP columns, like simple comparison does. --- mysql-test/main/timezone2.result | 22 +++++++++++ mysql-test/main/timezone2.test | 15 ++++++++ sql/sql_class.cc | 80 ++++++++++++++++++++++++++++------------ sql/sql_class.h | 2 + 4 files changed, 95 insertions(+), 24 deletions(-) diff --git a/mysql-test/main/timezone2.result b/mysql-test/main/timezone2.result index f943e285951..806255f26f5 100644 --- a/mysql-test/main/timezone2.result +++ b/mysql-test/main/timezone2.result @@ -654,3 +654,25 @@ SET time_zone=DEFAULT; # # End of 10.4 tests # +# +# MDEV-27101 Subquery using the ALL keyword on TIMESTAMP columns produces a wrong result +# +SET time_zone='Europe/Moscow'; +CREATE TABLE t1 (a TIMESTAMP NULL); +SET timestamp=1288477526; +/* this is summer time, earlier */ +INSERT INTO t1 VALUES (NOW()); +SET timestamp=1288477526+3599; +/* this is winter time, later */ +INSERT INTO t1 VALUES (NOW()); +SELECT a, UNIX_TIMESTAMP(a) FROM t1 ORDER BY a; +a UNIX_TIMESTAMP(a) +2010-10-31 02:25:26 1288477526 +2010-10-31 02:25:25 1288481125 +SELECT a, UNIX_TIMESTAMP(a) FROM t1 WHERE a <= ALL (SELECT * FROM t1); +a UNIX_TIMESTAMP(a) +2010-10-31 02:25:26 1288477526 +SELECT a, UNIX_TIMESTAMP(a) FROM t1 WHERE a >= ALL (SELECT * FROM t1); +a UNIX_TIMESTAMP(a) +2010-10-31 02:25:25 1288481125 +DROP TABLE t1; diff --git a/mysql-test/main/timezone2.test b/mysql-test/main/timezone2.test index 6a8c9f258e4..1feb8916871 100644 --- a/mysql-test/main/timezone2.test +++ b/mysql-test/main/timezone2.test @@ -598,3 +598,18 @@ SET time_zone=DEFAULT; --echo # --echo # End of 10.4 tests --echo # + +--echo # +--echo # MDEV-27101 Subquery using the ALL keyword on TIMESTAMP columns produces a wrong result +--echo # + +SET time_zone='Europe/Moscow'; +CREATE TABLE t1 (a TIMESTAMP NULL); +SET timestamp=1288477526; /* this is summer time, earlier */ +INSERT INTO t1 VALUES (NOW()); +SET timestamp=1288477526+3599; /* this is winter time, later */ +INSERT INTO t1 VALUES (NOW()); +SELECT a, UNIX_TIMESTAMP(a) FROM t1 ORDER BY a; +SELECT a, UNIX_TIMESTAMP(a) FROM t1 WHERE a <= ALL (SELECT * FROM t1); +SELECT a, UNIX_TIMESTAMP(a) FROM t1 WHERE a >= ALL (SELECT * FROM t1); +DROP TABLE t1; diff --git a/sql/sql_class.cc b/sql/sql_class.cc index bc4c54be605..b431fc75981 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -3698,6 +3698,41 @@ void select_max_min_finder_subselect::cleanup() } +void select_max_min_finder_subselect::set_op(const Type_handler *th) +{ + if (th->is_val_native_ready()) + { + op= &select_max_min_finder_subselect::cmp_native; + return; + } + + switch (th->cmp_type()) { + case REAL_RESULT: + op= &select_max_min_finder_subselect::cmp_real; + break; + case INT_RESULT: + op= &select_max_min_finder_subselect::cmp_int; + break; + case STRING_RESULT: + op= &select_max_min_finder_subselect::cmp_str; + break; + case DECIMAL_RESULT: + op= &select_max_min_finder_subselect::cmp_decimal; + break; + case TIME_RESULT: + if (th->field_type() == MYSQL_TYPE_TIME) + op= &select_max_min_finder_subselect::cmp_time; + else + op= &select_max_min_finder_subselect::cmp_str; + break; + case ROW_RESULT: + // This case should never be chosen + DBUG_ASSERT(0); + op= 0; + } +} + + int select_max_min_finder_subselect::send_data(List &items) { DBUG_ENTER("select_max_min_finder_subselect::send_data"); @@ -3716,30 +3751,7 @@ int select_max_min_finder_subselect::send_data(List &items) if (!cache) { cache= val_item->get_cache(thd); - switch (val_item->cmp_type()) { - case REAL_RESULT: - op= &select_max_min_finder_subselect::cmp_real; - break; - case INT_RESULT: - op= &select_max_min_finder_subselect::cmp_int; - break; - case STRING_RESULT: - op= &select_max_min_finder_subselect::cmp_str; - break; - case DECIMAL_RESULT: - op= &select_max_min_finder_subselect::cmp_decimal; - break; - case TIME_RESULT: - if (val_item->field_type() == MYSQL_TYPE_TIME) - op= &select_max_min_finder_subselect::cmp_time; - else - op= &select_max_min_finder_subselect::cmp_str; - break; - case ROW_RESULT: - // This case should never be choosen - DBUG_ASSERT(0); - op= 0; - } + set_op(val_item->type_handler()); } cache->store(val_item); it->store(0, cache); @@ -3833,6 +3845,26 @@ bool select_max_min_finder_subselect::cmp_str() return (sortcmp(val1, val2, cache->collation.collation) < 0); } + +bool select_max_min_finder_subselect::cmp_native() +{ + NativeBuffer cvalue, mvalue; + Item *maxmin= ((Item_singlerow_subselect *)item)->element_index(0); + bool cvalue_is_null= cache->val_native(thd, &cvalue); + bool mvalue_is_null= maxmin->val_native(thd, &mvalue); + + /* Ignore NULLs for ANY and keep them for ALL subqueries */ + if (cvalue_is_null) + return (is_all && !mvalue_is_null) || (!is_all && mvalue_is_null); + if (mvalue_is_null) + return !is_all; + + const Type_handler *th= cache->type_handler(); + return fmax ? th->cmp_native(cvalue, mvalue) > 0 : + th->cmp_native(cvalue, mvalue) < 0; +} + + int select_exists_subselect::send_data(List &items) { DBUG_ENTER("select_exists_subselect::send_data"); diff --git a/sql/sql_class.h b/sql/sql_class.h index 9aa243309a5..28c3fed6570 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -6068,6 +6068,7 @@ class select_max_min_finder_subselect :public select_subselect bool (select_max_min_finder_subselect::*op)(); bool fmax; bool is_all; + void set_op(const Type_handler *ha); public: select_max_min_finder_subselect(THD *thd_arg, Item_subselect *item_arg, bool mx, bool all): @@ -6080,6 +6081,7 @@ public: bool cmp_decimal(); bool cmp_str(); bool cmp_time(); + bool cmp_native(); }; /* EXISTS subselect interface class */ -- cgit v1.2.1 From c7f8cfc9e733517cff4aaa6f6eaca625a3afc098 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Tue, 16 Aug 2022 15:31:49 +0530 Subject: MDEV-27700 ASAN: Heap_use_after_free in btr_search_drop_page_hash_index() Reason: ======= Race condition between btr_search_drop_hash_index() and btr_search_lazy_free(). One thread does resizing of buffer pool and clears the ahi on all pages in the buffer pool, frees the index and table while removing the last reference. At the same time, other thread access index->heap in btr_search_drop_hash_index(). Solution: ========= Acquire the respective ahi latch before checking index->freed() btr_search_drop_page_hash_index(): Added new parameter to indicate that drop ahi entries only if the index is marked as freed btr_search_check_marked_free_index(): Acquire all ahi latches and return true if the index was freed --- storage/innobase/btr/btr0btr.cc | 5 +++-- storage/innobase/btr/btr0sea.cc | 36 ++++++++++++++++++++++++++++++++---- storage/innobase/buf/buf0buf.cc | 13 ++++--------- storage/innobase/include/btr0sea.h | 18 +++++++++++++++--- storage/innobase/include/btr0sea.inl | 13 +++++++++---- 5 files changed, 63 insertions(+), 22 deletions(-) diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc index b2c3b55cbaa..2ef5495a510 100644 --- a/storage/innobase/btr/btr0btr.cc +++ b/storage/innobase/btr/btr0btr.cc @@ -721,8 +721,9 @@ void btr_page_free(dict_index_t* index, buf_block_t* block, mtr_t* mtr, bool blob) { ut_ad(mtr_memo_contains(mtr, block, MTR_MEMO_PAGE_X_FIX)); -#ifdef BTR_CUR_HASH_ADAPT - if (block->index && !block->index->freed()) { +#if defined BTR_CUR_HASH_ADAPT && defined UNIV_DEBUG + if (block->index + && !btr_search_check_marked_free_index(block)) { ut_ad(!blob); ut_ad(page_is_leaf(block->frame)); } diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc index 77c8845729d..8cc30de0dd8 100644 --- a/storage/innobase/btr/btr0sea.cc +++ b/storage/innobase/btr/btr0sea.cc @@ -1102,8 +1102,11 @@ fail_and_release_page: index page for which we know that block->buf_fix_count == 0 or it is an index page which has already been removed from the buf_pool->page_hash - i.e.: it is in state BUF_BLOCK_REMOVE_HASH */ -void btr_search_drop_page_hash_index(buf_block_t* block) + i.e.: it is in state BUF_BLOCK_REMOVE_HASH +@param[in] garbage_collect drop ahi only if the index is marked + as freed */ +void btr_search_drop_page_hash_index(buf_block_t* block, + bool garbage_collect) { ulint n_fields; ulint n_bytes; @@ -1149,13 +1152,21 @@ retry: % btr_ahi_parts; latch = btr_search_latches[ahi_slot]; + rw_lock_s_lock(latch); + dict_index_t* index = block->index; bool is_freed = index && index->freed(); if (is_freed) { + rw_lock_s_unlock(latch); rw_lock_x_lock(latch); - } else { - rw_lock_s_lock(latch); + if (index != block->index) { + rw_lock_x_unlock(latch); + goto retry; + } + } else if (garbage_collect) { + rw_lock_s_unlock(latch); + return; } assert_block_ahi_valid(block); @@ -2220,5 +2231,22 @@ btr_search_validate() return(true); } +#ifdef UNIV_DEBUG +bool btr_search_check_marked_free_index(const buf_block_t *block) +{ + const index_id_t index_id= btr_page_get_index_id(block->frame); + + rw_lock_t *ahi_latch= btr_get_search_latch( + index_id, block->page.id.space()); + + rw_lock_s_lock(ahi_latch); + + bool is_freed= block->index && block->index->freed(); + + rw_lock_s_unlock(ahi_latch); + + return is_freed; +} +#endif /* UNIV_DEBUG */ #endif /* defined UNIV_AHI_DEBUG || defined UNIV_DEBUG */ #endif /* BTR_CUR_HASH_ADAPT */ diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index e4332fc3f32..8658b3a4a89 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -3935,18 +3935,14 @@ static void buf_defer_drop_ahi(buf_block_t *block, mtr_memo_type_t fix_type) /* Temporarily release our S-latch. */ rw_lock_s_unlock(&block->lock); rw_lock_x_lock(&block->lock); - if (dict_index_t *index= block->index) - if (index->freed()) - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, true); rw_lock_x_unlock(&block->lock); rw_lock_s_lock(&block->lock); break; case MTR_MEMO_PAGE_SX_FIX: rw_lock_sx_unlock(&block->lock); rw_lock_x_lock(&block->lock); - if (dict_index_t *index= block->index) - if (index->freed()) - btr_search_drop_page_hash_index(block); + btr_search_drop_page_hash_index(block, true); rw_lock_x_unlock(&block->lock); rw_lock_sx_lock(&block->lock); break; @@ -3993,8 +3989,7 @@ static buf_block_t* buf_page_mtr_lock(buf_block_t *block, #ifdef BTR_CUR_HASH_ADAPT { - dict_index_t *index= block->index; - if (index && index->freed()) + if (block->index) buf_defer_drop_ahi(block, fix_type); } #endif /* BTR_CUR_HASH_ADAPT */ @@ -4916,7 +4911,7 @@ buf_page_get_known_nowait( # ifdef BTR_CUR_HASH_ADAPT ut_ad(!block->page.file_page_was_freed - || (block->index && block->index->freed())); + || btr_search_check_marked_free_index(block)); # else /* BTR_CUR_HASH_ADAPT */ ut_ad(!block->page.file_page_was_freed); # endif /* BTR_CUR_HASH_ADAPT */ diff --git a/storage/innobase/include/btr0sea.h b/storage/innobase/include/btr0sea.h index 8ed0a13f0b5..8277be8ac14 100644 --- a/storage/innobase/include/btr0sea.h +++ b/storage/innobase/include/btr0sea.h @@ -99,8 +99,11 @@ btr_search_move_or_delete_hash_entries( index page for which we know that block->buf_fix_count == 0 or it is an index page which has already been removed from the buf_pool->page_hash - i.e.: it is in state BUF_BLOCK_REMOVE_HASH */ -void btr_search_drop_page_hash_index(buf_block_t* block); + i.e.: it is in state BUF_BLOCK_REMOVE_HASH +@param[in] garbage_collect drop ahi only if the index is marked + as freed */ +void btr_search_drop_page_hash_index(buf_block_t* block, + bool garbage_collect= false); /** Drop possible adaptive hash index entries when a page is evicted from the buffer pool or freed in a file, or the index is being dropped. @@ -173,16 +176,25 @@ A table is selected from an array of tables using pair of index-id, space-id. @param[in] index index handler @return hash table */ static inline hash_table_t* btr_get_search_table(const dict_index_t* index); + +#ifdef UNIV_DEBUG +/** @return if the index is marked as freed */ +bool btr_search_check_marked_free_index(const buf_block_t *block); +#endif /* UNIV_DEBUG */ + #else /* BTR_CUR_HASH_ADAPT */ # define btr_search_sys_create(size) # define btr_search_sys_free() -# define btr_search_drop_page_hash_index(block) +# define btr_search_drop_page_hash_index(block, garbage_collect) # define btr_search_s_lock_all(index) # define btr_search_s_unlock_all(index) # define btr_search_info_update(index, cursor) # define btr_search_move_or_delete_hash_entries(new_block, block) # define btr_search_update_hash_on_insert(cursor, ahi_latch) # define btr_search_update_hash_on_delete(cursor) +#ifdef UNIV_DEBUG +# define btr_search_check_marked_free_index(block) +#endif /* UNIV_DEBUG */ #endif /* BTR_CUR_HASH_ADAPT */ #ifdef BTR_CUR_ADAPT diff --git a/storage/innobase/include/btr0sea.inl b/storage/innobase/include/btr0sea.inl index 9db0084ce59..b07f3882fd2 100644 --- a/storage/innobase/include/btr0sea.inl +++ b/storage/innobase/include/btr0sea.inl @@ -158,6 +158,14 @@ static inline bool btr_search_own_any() } #endif /* UNIV_DEBUG */ +static inline rw_lock_t* btr_get_search_latch( + index_id_t index_id, ulint space_id) +{ + ulint ifold = ut_fold_ulint_pair(ulint(index_id), space_id); + + return(btr_search_latches[ifold % btr_ahi_parts]); +} + /** Get the adaptive hash search index latch for a b-tree. @param[in] index b-tree index @return latch */ @@ -167,10 +175,7 @@ static inline rw_lock_t* btr_get_search_latch(const dict_index_t* index) ut_ad(!index->table->space || index->table->space->id == index->table->space_id); - ulint ifold = ut_fold_ulint_pair(ulint(index->id), - index->table->space_id); - - return(btr_search_latches[ifold % btr_ahi_parts]); + return btr_get_search_latch(index->id, index->table->space_id); } /** Get the hash-table based on index attributes. -- cgit v1.2.1 From 55c648a73880013351dfe07edcd2af4a487ad873 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 23 Aug 2022 08:58:23 +0400 Subject: MDEV-27099 Subquery using the ALL keyword on INET6 columns produces a wrong result This problem was earlier fixed by MDEV-27101. Adding INET6 tests only. --- plugin/type_inet/mysql-test/type_inet/type_inet6.result | 16 ++++++++++++++++ plugin/type_inet/mysql-test/type_inet/type_inet6.test | 11 +++++++++++ 2 files changed, 27 insertions(+) diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6.result b/plugin/type_inet/mysql-test/type_inet/type_inet6.result index 2cc6942aa52..de2f504445d 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6.result +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6.result @@ -2213,3 +2213,19 @@ SELECT * FROM companies; id name DROP TABLE divisions; DROP TABLE companies; +# +# MDEV-27099 Subquery using the ALL keyword on INET6 columns produces a wrong result +# +CREATE TABLE t1 (d INET6); +INSERT INTO t1 VALUES ('1::0'), ('12::0'); +SELECT * FROM t1 ORDER BY d; +d +1:: +12:: +SELECT * FROM t1 WHERE d <= ALL (SELECT * FROM t1); +d +1:: +SELECT * FROM t1 WHERE d >= ALL (SELECT * FROM t1); +d +12:: +DROP TABLE t1; diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6.test b/plugin/type_inet/mysql-test/type_inet/type_inet6.test index ef8399d981f..becc063ddc9 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6.test +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6.test @@ -1630,3 +1630,14 @@ DELETE FROM companies WHERE id IN (SELECT company_id FROM divisions); SELECT * FROM companies; DROP TABLE divisions; DROP TABLE companies; + +--echo # +--echo # MDEV-27099 Subquery using the ALL keyword on INET6 columns produces a wrong result +--echo # + +CREATE TABLE t1 (d INET6); +INSERT INTO t1 VALUES ('1::0'), ('12::0'); +SELECT * FROM t1 ORDER BY d; +SELECT * FROM t1 WHERE d <= ALL (SELECT * FROM t1); +SELECT * FROM t1 WHERE d >= ALL (SELECT * FROM t1); +DROP TABLE t1; -- cgit v1.2.1