Commit 972879f4 authored by Oleg Smirnov's avatar Oleg Smirnov

MDEV-33010 Crash when pushing condition with CHARSET()/COERCIBILITY() into derived table

Based on the current logic, objects of classes Item_func_charset and
Item_func_coercibility (responsible for CHARSET() and COERCIBILITY()
functions) are always considered constant.
However, SQL syntax allows their use in a non-constant manner, such as
CHARSET(t1.a), COERCIBILITY(t1.a).

In these cases, the `used_tables()` parameter corresponds to table names
in the function parameters, creating an inconsistency: the item is marked
as constant but accesses tables. This leads to crashes when
conditions with CHARSET()/COERCIBILITY() are pushed into derived tables.

This commit addresses the issue by setting `used_tables()` to 0 for
`Item_func_charset` and `Item_func_coercibility`. Additionally, the items
now store the return values during the preparation phase and return
them during the execution phase. This ensures that the items do not call
its arguments methods during the execution and are truly constant.

Reviewer: Alexander Barkov <bar@mariadb.com>
parent 384ec03e
#
# MDEV-33010: Crash when pushing condition with CHARSET()/COERCIBILITY()
# into derived table
#
CREATE TABLE t1 (c1 BIGINT, KEY (c1)) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1);
CREATE TABLE t2 (c2 DOUBLE UNSIGNED);
INSERT INTO t2 VALUES (1);
SET optimizer_switch='derived_merge=off';
EXPLAIN EXTENDED
SELECT dt1_c1 FROM
(SELECT c1 AS dt1_c1 FROM t1) AS dt1
JOIN
(SELECT 1 AS dt2_c2 FROM t2) AS dt2
ON CHARSET(dt2_c2) BETWEEN dt1_c1 AND dt1_c1;
id select_type table type possible_keys key key_len ref rows filtered Extra
1 PRIMARY <derived3> system NULL NULL NULL NULL 1 100.00
1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where
3 DERIVED t2 system NULL NULL NULL NULL 1 100.00
2 DERIVED t1 ref c1 c1 9 const 1 100.00 Using where; Using index
Warnings:
Warning 1292 Truncated incorrect DECIMAL value: 'binary'
Note 1003 /* select#1 */ select `dt1`.`dt1_c1` AS `dt1_c1` from (/* select#2 */ select `test`.`t1`.`c1` AS `dt1_c1` from `test`.`t1` where <cache>(charset(1)) between `test`.`t1`.`c1` and `test`.`t1`.`c1`) `dt1` where <cache>(charset(1)) between `dt1`.`dt1_c1` and `dt1`.`dt1_c1`
EXPLAIN EXTENDED
SELECT dt1_c1 FROM
(SELECT c1 AS dt1_c1 FROM t1) AS dt1
JOIN
(SELECT 1 AS dt2_c2 FROM t2) AS dt2
ON COERCIBILITY(dt2_c2) BETWEEN dt1_c1 AND dt1_c1;
id select_type table type possible_keys key key_len ref rows filtered Extra
1 PRIMARY <derived3> system NULL NULL NULL NULL 1 100.00
1 PRIMARY <derived2> ALL NULL NULL NULL NULL 2 100.00 Using where
3 DERIVED t2 system NULL NULL NULL NULL 1 100.00
2 DERIVED t1 ref c1 c1 9 const 1 100.00 Using where; Using index
Warnings:
Note 1003 /* select#1 */ select `dt1`.`dt1_c1` AS `dt1_c1` from (/* select#2 */ select `test`.`t1`.`c1` AS `dt1_c1` from `test`.`t1` where <cache>(coercibility(1)) between `test`.`t1`.`c1` and `test`.`t1`.`c1`) `dt1` where <cache>(coercibility(1)) between `dt1`.`dt1_c1` and `dt1`.`dt1_c1`
SET optimizer_switch=DEFAULT;
DROP TABLE t1, t2;
# End of 10.4 tests
--source include/have_innodb.inc
--echo #
--echo # MDEV-33010: Crash when pushing condition with CHARSET()/COERCIBILITY()
--echo # into derived table
--echo #
CREATE TABLE t1 (c1 BIGINT, KEY (c1)) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1);
CREATE TABLE t2 (c2 DOUBLE UNSIGNED);
INSERT INTO t2 VALUES (1);
SET optimizer_switch='derived_merge=off';
EXPLAIN EXTENDED
SELECT dt1_c1 FROM
(SELECT c1 AS dt1_c1 FROM t1) AS dt1
JOIN
(SELECT 1 AS dt2_c2 FROM t2) AS dt2
ON CHARSET(dt2_c2) BETWEEN dt1_c1 AND dt1_c1;
EXPLAIN EXTENDED
SELECT dt1_c1 FROM
(SELECT c1 AS dt1_c1 FROM t1) AS dt1
JOIN
(SELECT 1 AS dt2_c2 FROM t2) AS dt2
ON COERCIBILITY(dt2_c2) BETWEEN dt1_c1 AND dt1_c1;
SET optimizer_switch=DEFAULT;
DROP TABLE t1, t2;
--echo # End of 10.4 tests
......@@ -787,18 +787,12 @@ DROP TABLE t1;
#
# MDEV-17830 Server crashes in Item_null_result::field_type upon SELECT with CHARSET(date) and ROLLUP
#
# Note, different MariaDB versions can return different results
# in the two rows (such as "latin1" vs "binary"). This is wrong.
# Both lines should return equal values.
# The point in this test is to make sure it does not crash.
# As this is a minor issue, bad result will be fixed
# in a later version, presumably in 10.4.
CREATE TABLE t (d DATE) ENGINE=MyISAM;
INSERT INTO t VALUES ('2018-12-12');
SELECT CHARSET(d) AS f FROM t GROUP BY d WITH ROLLUP;
f
binary
latin1
binary
DROP TABLE t;
#
# MDEV-14041 Server crashes in String::length on queries with functions and ROLLUP
......
......@@ -433,13 +433,6 @@ DROP TABLE t1;
--echo # MDEV-17830 Server crashes in Item_null_result::field_type upon SELECT with CHARSET(date) and ROLLUP
--echo #
--echo # Note, different MariaDB versions can return different results
--echo # in the two rows (such as "latin1" vs "binary"). This is wrong.
--echo # Both lines should return equal values.
--echo # The point in this test is to make sure it does not crash.
--echo # As this is a minor issue, bad result will be fixed
--echo # in a later version, presumably in 10.4.
CREATE TABLE t (d DATE) ENGINE=MyISAM;
INSERT INTO t VALUES ('2018-12-12');
SELECT CHARSET(d) AS f FROM t GROUP BY d WITH ROLLUP;
......
......@@ -3143,11 +3143,27 @@ longlong Item_func_char_length::val_int()
}
bool Item_func_coercibility::fix_length_and_dec()
{
max_length=10;
maybe_null= 0;
/*
Since this is a const item which doesn't use tables (see used_tables()),
we don't want to access the function arguments during execution.
That's why we store the derivation here during the preparation phase
and only return it later at the execution phase
*/
DBUG_ASSERT(args[0]->is_fixed());
m_cached_collation_derivation= (longlong) args[0]->collation.derivation;
return false;
}
longlong Item_func_coercibility::val_int()
{
DBUG_ASSERT(fixed == 1);
null_value= 0;
return (longlong) args[0]->collation.derivation;
return m_cached_collation_derivation;
}
......
......@@ -2285,13 +2285,15 @@ class Item_func_char_length :public Item_long_func_length
class Item_func_coercibility :public Item_long_func
{
longlong m_cached_collation_derivation;
bool check_arguments() const override
{ return args[0]->check_type_can_return_str(func_name()); }
public:
Item_func_coercibility(THD *thd, Item *a): Item_long_func(thd, a) {}
longlong val_int() override;
const char *func_name() const override { return "coercibility"; }
bool fix_length_and_dec() override { max_length=10; maybe_null= 0; return FALSE; }
bool fix_length_and_dec() override;
bool eval_not_null_tables(void *) override
{
not_null_tables_cache= 0;
......@@ -2306,6 +2308,7 @@ class Item_func_coercibility :public Item_long_func
bool const_item() const override { return true; }
Item *do_get_copy(THD *thd) const override
{ return get_item_copy<Item_func_coercibility>(thd, this); }
table_map used_tables() const override { return 0; }
};
......
......@@ -3625,9 +3625,8 @@ String *Item_func_charset::val_str(String *str)
DBUG_ASSERT(fixed == 1);
uint dummy_errors;
CHARSET_INFO *cs= args[0]->charset_for_protocol();
null_value= 0;
str->copy(cs->csname, (uint) strlen(cs->csname),
str->copy(m_cached_charset_info.str, m_cached_charset_info.length,
&my_charset_latin1, collation.collation, &dummy_errors);
return str;
}
......
......@@ -1692,6 +1692,8 @@ class Item_func_expr_str_metadata :public Item_str_func
class Item_func_charset :public Item_func_expr_str_metadata
{
LEX_CSTRING m_cached_charset_info;
public:
Item_func_charset(THD *thd, Item *a)
:Item_func_expr_str_metadata(thd, a) { }
......@@ -1699,6 +1701,23 @@ class Item_func_charset :public Item_func_expr_str_metadata
const char *func_name() const override { return "charset"; }
Item *do_get_copy(THD *thd) const override
{ return get_item_copy<Item_func_charset>(thd, this); }
table_map used_tables() const override { return 0; }
bool fix_length_and_dec() override
{
if (Item_func_expr_str_metadata::fix_length_and_dec())
return true;
/*
Since this is a const item which doesn't use tables (see used_tables()),
we don't want to access the function arguments during execution.
That's why we store the charset here during the preparation phase
and only return it later at the execution phase
*/
DBUG_ASSERT(args[0]->is_fixed());
m_cached_charset_info.str= args[0]->charset_for_protocol()->csname;
m_cached_charset_info.length=
strlen(args[0]->charset_for_protocol()->csname);
return false;
}
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment