From 09c4253619e839feebf782ac9d84ca6ccb95f707 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Fri, 28 Oct 2022 16:08:43 +0200 Subject: [PATCH] MDEV-29895 prepared view crash server (unit.conc_view) it's incorrect to use change_item_tree() to replace arguments of top-level AND/OR, because they (arguments) are stored in a List, so a pointer to an argument is in the list_node, and individual list_node's of top-level AND/OR can be deleted in Item_cond::build_equal_items(). In that case rollback_item_tree_changes() will modify the deleted object. Luckily, it's not needed to use change_item_tree() for top-level AND/OR, because the whole top-level item is copied and preserved in prep_where and prep_on, and restored from there. So, just don't. --- mysql-test/main/func_in.result | 17 +++++++++++++++-- mysql-test/main/func_in.test | 22 +++++++++++++++++----- sql/item.h | 5 +++++ sql/item_cmpfunc.cc | 29 +++++++++++++++++++++++++++++ sql/item_cmpfunc.h | 1 + sql/sql_select.cc | 4 ++-- 6 files changed, 69 insertions(+), 9 deletions(-) diff --git a/mysql-test/main/func_in.result b/mysql-test/main/func_in.result index 672e45e877b..b18aa26777e 100644 --- a/mysql-test/main/func_in.result +++ b/mysql-test/main/func_in.result @@ -1,4 +1,3 @@ -drop table if exists t1, t2; select 1 in (1,2,3); 1 in (1,2,3) 1 @@ -942,6 +941,9 @@ SELECT ('0x',1) IN ((0,1),(1,1)); Warnings: Warning 1292 Truncated incorrect DECIMAL value: '0x' # +# End of 10.4 tests +# +# # MDEV-29662 same values in `IN` set vs equal comparison produces # the different performance # @@ -1153,5 +1155,16 @@ DROP TABLE t1, t2, t3, t4; DROP VIEW v1; DROP PROCEDURE p1; # -# End of 10.4 tests +# MDEV-29895 prepared view crash server (unit.conc_view) +# +create table t1 (username varchar(12) not null, id int(11) not null); +create view v1 as select username from t1 where id = 0; +prepare stmt from "select username from v1 where username in (?, ?)"; +execute stmt using "1", "1"; +username +deallocate prepare stmt; +drop view v1; +drop table t1; +# +# End of 10.6 tests # diff --git a/mysql-test/main/func_in.test b/mysql-test/main/func_in.test index 99161e76fb8..02483c482ac 100644 --- a/mysql-test/main/func_in.test +++ b/mysql-test/main/func_in.test @@ -1,7 +1,3 @@ -# Initialise ---disable_warnings -drop table if exists t1, t2; ---enable_warnings # # test of IN (NULL) # @@ -721,6 +717,10 @@ SELECT '0x' IN (0,1); SELECT ('0x',1) IN ((0,1)); SELECT ('0x',1) IN ((0,1),(1,1)); +--echo # +--echo # End of 10.4 tests +--echo # + --echo # --echo # MDEV-29662 same values in `IN` set vs equal comparison produces --echo # the different performance @@ -840,6 +840,18 @@ DROP VIEW v1; DROP PROCEDURE p1; --echo # ---echo # End of 10.4 tests +--echo # MDEV-29895 prepared view crash server (unit.conc_view) +--echo # + +create table t1 (username varchar(12) not null, id int(11) not null); +create view v1 as select username from t1 where id = 0; +prepare stmt from "select username from v1 where username in (?, ?)"; +execute stmt using "1", "1"; +deallocate prepare stmt; +drop view v1; +drop table t1; + +--echo # +--echo # End of 10.6 tests --echo # diff --git a/sql/item.h b/sql/item.h index 04e469e8fde..1ea748f2578 100644 --- a/sql/item.h +++ b/sql/item.h @@ -2112,6 +2112,11 @@ public: } virtual Item* transform(THD *thd, Item_transformer transformer, uchar *arg); + virtual Item* top_level_transform(THD *thd, Item_transformer transformer, + uchar *arg) + { + return transform(thd, transformer, arg); + } /* This function performs a generic "compilation" of the Item tree. diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 5ee29bd5dfb..b08d3a799f9 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -5226,6 +5226,35 @@ Item *Item_cond::transform(THD *thd, Item_transformer transformer, uchar *arg) } +/** + Transform an Item_cond object with a transformer callback function. + + This is like transform() but doesn't use change_item_tree(), + because top-level expression is stored in prep_where/prep_on anyway and + is restored from there, there is no need to use change_item_tree(). + + Furthermore, it can be actually harmful to use it, if build_equal_items() + had replaced Item_eq with Item_equal and deleted list_node with a pointer + to Item_eq. In this case rollback_item_tree_changes() would modify the + deleted list_node. +*/ +Item *Item_cond::top_level_transform(THD *thd, Item_transformer transformer, uchar *arg) +{ + DBUG_ASSERT(!thd->stmt_arena->is_stmt_prepare()); + + List_iterator li(list); + Item *item; + while ((item= li++)) + { + Item *new_item= item->transform(thd, transformer, arg); + if (!new_item) + return 0; + *li.ref()= new_item; + } + return Item_func::transform(thd, transformer, arg); +} + + /** Compile Item_cond object with a processor and a transformer callback functions. diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 098bad90166..d0cbc492a20 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -3193,6 +3193,7 @@ public: void copy_andor_arguments(THD *thd, Item_cond *item); bool walk(Item_processor processor, bool walk_subquery, void *arg) override; Item *transform(THD *thd, Item_transformer transformer, uchar *arg) override; + Item *top_level_transform(THD *thd, Item_transformer transformer, uchar *arg) override; void traverse_cond(Cond_traverser, void *arg, traverse_order order) override; void neg_arguments(THD *thd); Item* propagate_equal_fields(THD *, const Context &, COND_EQUAL *) override; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index e416be036d1..71b9dad3dd1 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -30523,7 +30523,7 @@ bool JOIN::transform_all_conds_and_on_exprs(THD *thd, { if (conds) { - conds= conds->transform(thd, transformer, (uchar *) 0); + conds= conds->top_level_transform(thd, transformer, (uchar *) 0); if (!conds) return true; } @@ -30553,7 +30553,7 @@ bool JOIN::transform_all_conds_and_on_exprs_in_join_list( } if (table->on_expr) { - table->on_expr= table->on_expr->transform(thd, transformer, (uchar *) 0); + table->on_expr= table->on_expr->top_level_transform(thd, transformer, 0); if (!table->on_expr) return true; }