Hi Sergei, thanks for your review. Answers inline. On Thu, Jun 04, 2015 at 04:29:24PM +0200, Sergei Golubchik wrote:
Hi, svoj!
On Jun 04, svoj@mariadb.org wrote:
revision-id: 91259f903c2ffac7c7c8fc72385a572eace4fa89 parent(s): 9bb9bde42dbf6f936faadeaa34cde838bf6b63dd committer: Sergey Vojtovich branch nick: mariadb timestamp: 2015-06-04 16:04:05 +0400 message:
MDEV-7505 - Too large scale in DECIMAL dynamic column getter crashes mysqld
Server may crash if sanity checks of COLUMN_GET() fail.
COLUMN_GET() description generator expects parent CAST item, which may not have been created due to failure of sanity checks. Then further attempt to report an error may crash the server.
Fixed COLUMN_GET() description generator to handle such case.
--- mysql-test/r/dyncol.result | 6 ++++++ mysql-test/t/AAA.test | 1 +
Mistakenly committed file ^^^^ Thanks! I'll remove it in next commit.
mysql-test/t/dyncol.test | 6 ++++++ sql/item_strfunc.cc | 7 +++++++ 4 files changed, 20 insertions(+)
diff --git a/mysql-test/t/AAA.test b/mysql-test/t/AAA.test new file mode 100644 index 0000000..371db62 --- /dev/null +++ b/mysql-test/t/AAA.test @@ -0,0 +1 @@ +SELECT COLUMN_GET(`x`, 'y' AS DECIMAL(5,34)); diff --git a/sql/item_strfunc.cc b/sql/item_strfunc.cc index eedf149..7ad4c06 100644 --- a/sql/item_strfunc.cc +++ b/sql/item_strfunc.cc @@ -4467,6 +4467,13 @@ bool Item_dyncol_get::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
void Item_dyncol_get::print(String *str, enum_query_type query_type)
How do you get to Item_dyncol_get::print if cast couldn't be created? The comment below says "see create_func_dyncol_get". This function looks like
if (!(res= new (thd->mem_root) Item_dyncol_get(str, num))) return res; // Return NULL return create_func_cast(thd, res, cast_type, c_len, c_dec, cs);
and to get Item_dyncol_get without a parent, one needs create_func_cast to return 'res' without wrapping it in a CAST(...). But it never does that.
Trace is as following: Item_dyncol_get::print() item_name() wrong_precision_error() create_func_cast() create_func_dyncol_get()
{ + /* Parent cast doesn't exist yet. Only print dynamic column name. */ + if (!str->length())
Not safe, if you wrap your COLUMN_GET() in, say, CONCAT():
CONCAT(COLUMN_GET(`x`, 'y' AS DECIMAL(5,34)))
then str->length() won't be 0, despite the error in CAST.
Safe, because item_name() does str->length(0). Frankly speaking I'd gladly replace this my hack, but I couldn't come up with something more solid. Thanks, Sergey