Skip to content

Commit

Permalink
MDEV-29895 prepared view crash server (unit.conc_view)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
vuvova authored and sanja-byelkin committed Oct 29, 2022
1 parent 2f42168 commit 09c4253
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 9 deletions.
17 changes: 15 additions & 2 deletions 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
Expand Down Expand Up @@ -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
#
Expand Down Expand Up @@ -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
#
22 changes: 17 additions & 5 deletions 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)
#
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 #

5 changes: 5 additions & 0 deletions sql/item.h
Expand Up @@ -2112,6 +2112,11 @@ class Item :public Value_source,
}

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.
Expand Down
29 changes: 29 additions & 0 deletions sql/item_cmpfunc.cc
Expand Up @@ -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<Item> 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.
Expand Down
1 change: 1 addition & 0 deletions sql/item_cmpfunc.h
Expand Up @@ -3193,6 +3193,7 @@ class Item_cond :public Item_bool_func
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;
Expand Down
4 changes: 2 additions & 2 deletions sql/sql_select.cc
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down

0 comments on commit 09c4253

Please sign in to comment.