maria-developers team mailing list archive

[Sergey Vojtovich <svoj@xxxxxxxxxxx>]

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@xxxxxxxxxxx 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