diff --git a/mysql-test/main/subselect_cache.result b/mysql-test/main/subselect_cache.result index 401f84b336c0e..c7d79e541a8e9 100644 --- a/mysql-test/main/subselect_cache.result +++ b/mysql-test/main/subselect_cache.result @@ -3933,3 +3933,40 @@ drop table t1,t2,t3,t4; SET optimizer_switch=@save_optimizer_switch; # restore default set @@optimizer_switch= default; +# +# MDEV-38801 Item_sum & Item_cache implement only shallow copy +# +CREATE TABLE t1 (c YEAR KEY); +INSERT INTO t1 VALUES (2000),(2001); +INSERT INTO t1 VALUES ((c IN (SELECT * FROM (SELECT * FROM t1 GROUP BY c) AS d +NATURAL JOIN (SELECT * FROM t1) AS e))); +DROP TABLE t1; +CREATE TABLE t1 (pk INT PRIMARY KEY, d DATE, q INT, c CHAR(8)); +INSERT INTO t1 SELECT seq, DATE'1998-01-01' + INTERVAL (seq%700) DAY, seq%50, +CONCAT('n', seq%9) FROM seq_1_to_40000; +ANALYZE TABLE t1 PERSISTENT FOR ALL; +Table Op Msg_type Msg_text +test.t1 analyze status Engine-independent statistics collected +test.t1 analyze status OK +# Item_cache_date +EXPLAIN SELECT q FROM t1 WHERE d <= DATE'1998-12-01' - INTERVAL '63' DAY; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 40000 Using where +# Item_cache_int +EXPLAIN SELECT q FROM t1 WHERE q <= 3 + 4; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 40000 Using where +# Item_cache_decimal +EXPLAIN SELECT q FROM t1 WHERE q <= 7.5 * 2; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 40000 Using where +# Item_cache_double +EXPLAIN SELECT q FROM t1 WHERE q <= SQRT(2) * 10; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 40000 Using where +# Item_cache_str +EXPLAIN SELECT q FROM t1 WHERE c <= CONCAT('n', '4'); +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 40000 Using where +DROP TABLE t1; +# end of 10.11 tests diff --git a/mysql-test/main/subselect_cache.test b/mysql-test/main/subselect_cache.test index 7bc897ea06ba1..0752f901a8ad4 100644 --- a/mysql-test/main/subselect_cache.test +++ b/mysql-test/main/subselect_cache.test @@ -1,6 +1,7 @@ # Tests will be skipped for the view protocol because the view protocol creates # an additional util connection and other statistics data -- source include/no_view_protocol.inc +-- source include/have_sequence.inc --disable_warnings drop table if exists t0,t1,t2,t3,t4,t5,t6,t7,t8,t9; @@ -1743,3 +1744,35 @@ SET optimizer_switch=@save_optimizer_switch; --echo # restore default set @@optimizer_switch= default; + + +--echo # +--echo # MDEV-38801 Item_sum & Item_cache implement only shallow copy +--echo # + + +CREATE TABLE t1 (c YEAR KEY); +INSERT INTO t1 VALUES (2000),(2001); +INSERT INTO t1 VALUES ((c IN (SELECT * FROM (SELECT * FROM t1 GROUP BY c) AS d + NATURAL JOIN (SELECT * FROM t1) AS e))); +DROP TABLE t1; + +CREATE TABLE t1 (pk INT PRIMARY KEY, d DATE, q INT, c CHAR(8)); +INSERT INTO t1 SELECT seq, DATE'1998-01-01' + INTERVAL (seq%700) DAY, seq%50, + CONCAT('n', seq%9) FROM seq_1_to_40000; +ANALYZE TABLE t1 PERSISTENT FOR ALL; + +--echo # Item_cache_date +EXPLAIN SELECT q FROM t1 WHERE d <= DATE'1998-12-01' - INTERVAL '63' DAY; +--echo # Item_cache_int +EXPLAIN SELECT q FROM t1 WHERE q <= 3 + 4; +--echo # Item_cache_decimal +EXPLAIN SELECT q FROM t1 WHERE q <= 7.5 * 2; +--echo # Item_cache_double +EXPLAIN SELECT q FROM t1 WHERE q <= SQRT(2) * 10; +--echo # Item_cache_str +EXPLAIN SELECT q FROM t1 WHERE c <= CONCAT('n', '4'); + +DROP TABLE t1; + +--echo # end of 10.11 tests diff --git a/sql/item.cc b/sql/item.cc index d51b931a3d178..fbc52feb226c5 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -10739,6 +10739,72 @@ void Item_cache::store(Item *item) value_cached= FALSE; } + +#ifndef DBUG_OFF +/* + @brief + Whether 'clone' reaches any item object that 'src' reaches too. + + @description + A copy is only a deep copy iff it shares no nodes at all with the item + it came from. Some classes implement deep_copy() as a shallow copy while + still holding child items -- Item_outer_ref and Item_copy_string -- so their + copy keeps pointing at the original's children. + + Walking with find_item_processor asks whether a tree reaches one given + object, so collecting the copy's nodes and asking that of the original + covers both, and covers the classes not yet met rather than the two above. +*/ +bool item_clone_shares_nodes(Item *src, Item *clone) +{ + List clone_nodes; + if (clone->walk(&Item::collect_all_items_processor, true, &clone_nodes)) + return true; // could not collect them + + List_iterator_fast it(clone_nodes); + Item *node; + while ((node= it++)) + if (src->walk(&Item::find_item_processor, true, (void*) node)) + return true; + return false; +} +#endif + + +/** + @brief + Build a clone of an Item_cache. + + @details + 'example' is an ordinary pointer, so the copy constructor alone would give a + new cache still reading the expression the original caches, and every node + below the cache would then belong to both trees. Clone the expression as + well, the way Item_func_or_sum::deep_copy() clones its arguments, and keep + the invariant setup() establishes between 'example' and 'cached_field'. + + An expression that cannot be cloned -- a subquery, for one -- makes the + cache unclonable too, rather than half copied. + + @return clone of the item + @retval 0 on a failure, or if 'example' cannot be cloned +*/ + +Item* Item_cache::deep_copy(THD *thd) const +{ + Item *example_clone= NULL; + if (example && !(example_clone= example->deep_copy_with_checks(thd))) + return NULL; + Item_cache *copy= static_cast(shallow_copy_with_checks(thd)); + if (unlikely(!copy)) + return NULL; + copy->example= example_clone; + if (cached_field && example_clone && + example_clone->type() == Item::FIELD_ITEM) + copy->cached_field= ((Item_field *) example_clone)->field; + DBUG_ASSERT(!item_clone_shares_nodes((Item*)this, copy)); + return copy; +} + void Item_cache::print(String *str, enum_query_type query_type) { if (example && // There is a cached item @@ -11391,6 +11457,46 @@ void Item_cache_row::set_null() }; +/** + @brief + Build a clone of an Item_cache_row. + + @details + A row cache holds a cache per column in values[], and those are not reached + through 'example', so Item_cache::deep_copy() leaves them shared. Clone the + array too, on a fresh allocation: the copy must not write through the + original's. + + @return clone of the item + @retval 0 on a failure, or if any element cannot be cloned +*/ + +Item* Item_cache_row::deep_copy(THD *thd) const +{ + Item_cache_row *copy= + static_cast(Item_cache::deep_copy(thd)); + if (unlikely(!copy) || !values) + return copy; + + Item_cache **values_clone= (Item_cache**)thd->calloc( + item_count*sizeof(Item_cache*)); + if (unlikely(!values_clone)) + return NULL; + for (uint i= 0; i < item_count; i++) + { + if (!values[i]) + continue; + Item *el_clone= values[i]->deep_copy_with_checks(thd); + if (unlikely(!el_clone)) + return NULL; + values_clone[i]= static_cast(el_clone); + } + copy->values= values_clone; + DBUG_ASSERT(!item_clone_shares_nodes((Item*)this, copy)); + return copy; +} + + double Item_type_holder::val_real() { DBUG_ASSERT(0); // should never be called @@ -11722,3 +11828,10 @@ bool ignored_list_includes_table(ignored_tables_list_t list, TABLE_LIST *tbl) } return false; } + + +bool Item::collect_all_items_processor(void *arg) +{ + List *items= (List *) arg; + return items->push_back(this); // stops the walk if it cannot record +} diff --git a/sql/item.h b/sql/item.h index fe0f17e4d0618..f1cfa22ed9e56 100644 --- a/sql/item.h +++ b/sql/item.h @@ -2292,6 +2292,7 @@ class Item :public Value_source, virtual bool check_inner_refs_processor(void *arg) { return 0; } virtual bool find_item_in_field_list_processor(void *arg) { return 0; } virtual bool find_item_processor(void *arg); + bool collect_all_items_processor(void *arg); virtual bool change_context_processor(void *arg) { return 0; } virtual bool reset_query_id_processor(void *arg) { return 0; } virtual bool is_expensive_processor(void *arg) { return 0; } @@ -7827,6 +7828,14 @@ class Item_cache: public Item_fixed_hybrid, { return convert_to_basic_const_item(thd); } Item *in_subq_field_transformer_for_having(THD *thd, uchar *) override { return convert_to_basic_const_item(thd); } + +protected: + /* + A shallow copy would leave the copy's 'example' pointing at the original's + expression, so the two would share every node below the cache. Defined here + once for the whole family: it dispatches to each class's shallow_copy(). + */ + Item *deep_copy(THD *thd) const override; }; @@ -7850,8 +7859,6 @@ class Item_cache_int: public Item_cache protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } }; @@ -7886,8 +7893,9 @@ class Item_cache_year: public Item_cache_int { return type_handler_year.Item_get_date_with_warn(thd, this, to, mode); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } +protected: + Item *shallow_copy(THD *thd) const override + { return get_item_copy(thd, this); } }; @@ -8039,8 +8047,6 @@ class Item_cache_timestamp: public Item_cache protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } public: bool cache_value() override; String* val_str(String *to) override @@ -8103,8 +8109,6 @@ class Item_cache_double: public Item_cache_real protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } }; @@ -8118,8 +8122,6 @@ class Item_cache_float: public Item_cache_real protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } }; @@ -8144,8 +8146,6 @@ class Item_cache_decimal: public Item_cache protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } }; @@ -8177,8 +8177,6 @@ class Item_cache_str: public Item_cache protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } }; @@ -8205,10 +8203,6 @@ class Item_cache_str_for_nullif: public Item_cache_str protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { - return shallow_copy_with_checks(thd); - } }; @@ -8287,10 +8281,8 @@ class Item_cache_row: public Item_cache protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { - return shallow_copy_with_checks(thd); - } + /* The row's element caches are held in values[], not in 'example' alone. */ + Item *deep_copy(THD *thd) const override; }; diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 910a94facf129..4771e50778783 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -629,6 +629,52 @@ void Item_sum::cleanup() const_item_cache= false; } + +#ifndef DBUG_OFF +extern bool item_clone_shares_nodes(Item *src, Item *clone); +#endif +/** + @brief + Build a clone of an Item_sum. + + @details + Item_func_or_sum::deep_copy() clones the arguments and repoints args at the + copy's own array, then two members of an aggregate need the same treatment. + + orig_args addresses tmp_orig_args, inside the object, whenever there are no + more than two arguments, so a copied pointer would leave get_args() handing + the rest of the server the original's array, and fix_fields() writing into + it. Point it at the clone's own, holding the cloned arguments. + + The aggregator belongs to the item it was made for, whose cleanup() deletes + it, and both items sit on the statement's free list, so a copied pointer + would be deleted twice. Give the clone one of its own, which is what + Item_sum::Item_sum(THD*, Item_sum*) does for the ROLLUP copies. + + @return clone of the item + @retval 0 on a failure +*/ + +Item* Item_sum::deep_copy(THD *thd) const +{ + Item_sum *clone= static_cast(Item_func_or_sum::deep_copy(thd)); + if (unlikely(!clone)) + return NULL; + + clone->orig_args= clone->tmp_orig_args; + if (arg_count > 2 && + unlikely(!(clone->orig_args= (Item**)thd->alloc(arg_count*sizeof(Item*))))) + return NULL; + if (arg_count) + memcpy(clone->orig_args, clone->args, sizeof(Item *) * arg_count); + + clone->aggr= NULL; + if (aggr && unlikely(clone->set_aggregator(thd, aggr->Aggrtype()))) + return NULL; + DBUG_ASSERT(!item_clone_shares_nodes((Item*)this, clone)); + return clone; +} + Item *Item_sum::result_item(THD *thd, Field *field) { return new (thd->mem_root) Item_field(thd, field); @@ -2512,6 +2558,44 @@ void Item_sum_min_max::cleanup() DBUG_VOID_RETURN; } + +/** + @brief + Build a clone of an Item_sum_min_max. + + @details + Besides the aggregator, MIN() and MAX() hold an Arg_comparator that + cleanup() deletes, and it was given the addresses of this object's own + 'value' and 'arg_cache' members, so a copy of the pointer would compare the + original's caches and be deleted twice. setup_hybrid() builds all three + together, which is what fix_fields() calls, so call it for the clone. A + clone of a fixed item is fixed, so nothing else will do it. + + @return clone of the item + @retval 0 on a failure +*/ + +Item* Item_sum_min_max::deep_copy(THD *thd) const +{ + Item_sum_min_max *clone= + static_cast(Item_sum::deep_copy(thd)); + if (unlikely(!clone)) + return NULL; + clone->direct_added= FALSE; + clone->direct_item= NULL; + clone->value= clone->arg_cache= NULL; + clone->cmp= NULL; + if (!clone->is_window_func_sum_expr()) + { + clone->setup_hybrid(thd, clone->args[0], NULL); + if (unlikely(!clone->cmp)) // setup_hybrid() ran out of memory + return NULL; + } + DBUG_ASSERT(!item_clone_shares_nodes((Item*)this, clone)); + return clone; +} + + void Item_sum_min_max::no_rows_in_result() { DBUG_ENTER("Item_sum_min_max::no_rows_in_result"); diff --git a/sql/item_sum.h b/sql/item_sum.h index 329ee69499f9d..06469e01f3ba5 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -605,6 +605,13 @@ class Item_sum :public Item_func_or_sum */ virtual bool uses_non_standard_aggregator_for_distinct() const { return false; } + +protected: + /* + orig_args addresses this object's own tmp_orig_args, and the aggregator is + deleted by cleanup(), so neither pointer may be handed to a clone as it is. + */ + Item *deep_copy(THD *thd) const override; }; @@ -1204,6 +1211,13 @@ class Item_sum_min_max :public Item_sum_hybrid Field *create_tmp_field(MEM_ROOT *root, bool group, TABLE *table) override; void setup_caches(THD *thd) override { setup_hybrid(thd, arguments()[0], NULL); } + +protected: + /* + 'cmp' is deleted by cleanup(), and it is bound to the addresses of this + object's own 'value' and 'arg_cache', so a clone needs its own set. + */ + Item *deep_copy(THD *thd) const override; }; diff --git a/sql/sql_type_fixedbin.h b/sql/sql_type_fixedbin.h index 28d141fd73a7e..2eada6234e214 100644 --- a/sql/sql_type_fixedbin.h +++ b/sql/sql_type_fixedbin.h @@ -1075,8 +1075,6 @@ class Type_handler_fbt: public Type_handler protected: Item *shallow_copy(THD *thd) const override { return get_item_copy(thd, this); } - Item *deep_copy(THD *thd) const override - { return shallow_copy_with_checks(thd); } }; /* =[ methods ]=============================================== */