Bug #34749 Server crash when using NAME_CONST() with an aggregate function
Submitted: 21 Feb 2008 21:10 Modified: 15 Mar 2008 21:17
Reporter: David Shrewsbury Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S1 (Critical)
Version:5.0.54a, 5.1.23-rc OS:Linux
Assigned to: Tatiana Azundris Nuernberg CPU Architecture:Any
Tags: name_const

[21 Feb 2008 21:10] David Shrewsbury
Description:
Using NAME_CONST() with a negative number AND aggregate function causes a system crash. This can be unavoidable in some cases of replication since the NAME_CONST() can be inserted by the server (as in a stored procedure).

Note that this is similar to bug #30832, but this was tested on 5.0.54a which is supposed to have the fix for that particular bug.

Backtrace follows:

(gdb) bt
#0  0x00002b3cbe72e422 in pthread_kill () from /lib/libpthread.so.0
#1  0x000000000072356f in write_core ()
#2  0x00000000005a8047 in handle_segfault ()
#3  <signal handler called>
#4  0x0000000000000000 in ?? ()
#5  0x00000000004db4f5 in Item::split_sum_func2 ()
#6  0x00000000005103b8 in Item_func::split_sum_func ()
#7  0x00000000005f0691 in setup_fields ()
#8  0x0000000000602a3b in JOIN::prepare ()
#9  0x0000000000602800 in mysql_select ()
#10 0x0000000000627f15 in handle_select ()
#11 0x00000000005bede4 in mysql_execute_command ()
#12 0x00000000005c4fcb in mysql_parse ()
#13 0x00000000005b9601 in dispatch_command ()
#14 0x00000000005b7438 in handle_one_connection ()
#15 0x00002b3cbe729317 in start_thread () from /lib/libpthread.so.0
#16 0x00002b3cbf5cbd5d in clone () from /lib/libc.so.6
#17 0x0000000000000000 in ?? ()

How to repeat:
DROP TABLE IF EXISTS t1;
CREATE TABLE t1 (a INT);

INSERT INTO t1 VALUES (1),(2),(3);

-- Must use a negative number in NAME_CONST() along with the aggregate
SELECT NAME_CONST('flag',-1) * MAX(a) FROM t1;
[26 Feb 2008 8:45] Tatiana Azundris Nuernberg
#0  Item_name_const::type (this=0x8d9c448) at item.cc:1222
#1  0x08181e17 in Item::split_sum_func2 (this=0x8d9c448, thd=0x8d642e8, ref_pointer_array=0x8d9d738, fields=@0x8d9d678, ref=0x8d9c65c, skip_registered=true) at item.cc:1308
#2  0x081b14f7 in Item_func::split_sum_func (this=0x8d9c608, thd=0x8d642e8, ref_pointer_array=0x8d9d738, fields=@0x8d9d678) at item_func.cc:342
#3  0x08270957 in setup_fields (thd=0x8d642e8, ref_pointer_array=0x8d9d738, fields=@0x8d65320, set_query_id=true, sum_func_list=0x8d9d678, allow_sum_func=true) at sql_base.cc:5124
#4  0x082a48e4 in JOIN::prepare (this=0x8d9c938, rref_pointer_array=0x8d653b0, tables_init=0x8d9c710, wild_num=0, conds_init=0x0, og_num=0, order_init=0x0, group_init=0x0, having_init=0x0, proc_param_init=0x0, select_lex_arg=0x8d6528c, unit_arg=0x8d65054) at sql_select.cc:467
#5  0x082a5723 in mysql_select (thd=0x8d642e8, rref_pointer_array=0x8d653b0, tables=0x8d9c710, wild_num=0, fields=@0x8d65320, conds=0x0, og_num=0, order=0x0, group=0x0, having=0x0, proc_param=0x0, select_options=2156153344, result=0x8d9c920, unit=0x8d65054, select_lex=0x8d6528c) at sql_select.cc:2274
#6  0x082a9ba6 in handle_select (thd=0x8d642e8, lex=0x8d64ffc, result=0x8d9c920, setup_tables_done_option=0) at sql_select.cc:257
#7  0x0823d89d in mysql_execute_command (thd=0x8d642e8) at sql_parse.cc:2736
#8  0x08245cde in mysql_parse (thd=0x8d642e8, inBuf=0x8d9c290 "SELECT NAME_CONST('flag',-1)*max(1) FROM t1", length=43, found_semicolon=0xb50db1c0) at sql_parse.cc:6174
#9  0x08248528 in dispatch_command (command=COM_QUERY, thd=0x8d642e8, packet=0x8d94231 "SELECT NAME_CONST('flag',-1)*max(1) FROM t1", packet_length=44) at sql_parse.cc:1889
#10 0x08249a81 in do_command (thd=0x8d642e8) at sql_parse.cc:1595
#11 0x0824ac5b in handle_one_connection (arg=0x8d642e8) at sql_parse.cc:1201

(gdb) print ((Item_func_neg *) value_item)->func_name()
$5 = 0x86353c7 "-"
(gdb) print ((Item_func_neg *) value_item)->val_int()
$6 = -1
(gdb) print ((Item_func_neg *) value_item)->basic_const_item()
$7 = true
(gdb) print ((Item_func_neg *) value_item)->args[0]
$8 = (class Item *) 0x8d9c370

(gdb) graph display *(((Item_func_neg *) this->value_item)->args[0]) 

(gdb) print (((Item_func_neg *) this->value_item)->args[0])->type()
$10 = INT_ITEM
[26 Feb 2008 8:46] Tatiana Azundris Nuernberg
This of course is "more fun with Item_func_neg" inasmuch as overloading like so unbreaks the crash as such:

virtual enum Type type() const { return args[0]->type() == INT_ITEM ? INT_ITEM : FUNC_ITEM; }

mysql> SELECT NAME_CONST('flag',-1) * MAX(a) FROM t1;
+--------------------------------+
| NAME_CONST('flag',-1) * MAX(a) |
+--------------------------------+
|                             -3 | 
+--------------------------------+
[26 Feb 2008 8:55] Tatiana Azundris Nuernberg
(note that above is just an illustration, not the actual fix as for instance doesn't hotfix REAL_ITEM. fix in proper place.)
[27 Feb 2008 19:01] Tatiana Azundris Nuernberg
Fixing this in the same place as Bug#30832 in the name of consistency is a nice idea, but (rather expectedly) affects execution plans. It is still interesting in that we get neg(1) rather than -1 as an integer const in the first place or, to rephrase, that the item identifies as FUNC and the func-type as UNKNOWN (implying at face value that to identify neg() you'd look at the function's name(!!) instead, which I really won't comment on, which in turn might lead one to conclude that since nobody introduced FUNC_NEG identification, no such rewriting is taking place).

Hence, this is looks like a good idea (to the extent that it fixes in the same place that the fix  for #30832 does), but ends up too broad:

class Item_func_neg :public Item_func_num1
{
  ...
  virtual enum Type type() const { return args[0]->basic_const_item() ? args[0]->type() : Item_func_num1::type(); }
  ...

On the other hand, this only fixes the offending item-type "NAME_CONST" by making it transparent where possible (that is, returns the underlying type of a item_func_neg()). Which gives us, wait for it, this:

Item::Type Item_name_const::type() const
{
  ...
  /*
    valid_args guarantees value_item->basic_const_item(); if type is
    FUNC_ITEM, then we have a fudged item_func_neg() on our hands
    and return the underlying type.
  */
  return valid_args ?
           (value_item->type() == FUNC_ITEM) ?
             (((Item_func *) value_item)->key_item()->type() :
             value_item->type()) :
           NULL_ITEM;
}
[27 Feb 2008 19:32] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43104

ChangeSet@1.2608, 2008-02-27 20:31:50+01:00, tnurnberg@mysql.com +3 -0
  Bug#34749: Server crash when using NAME_CONST() with an aggregate function
  
  NAME_CONST('whatever', -1) * MAX(whatever) bombed since -1 was
  not seen as constant, but as FUNCTION_UNARY_MINUS(constant)
  while we are at the same time pretending it was a basic const
  item. This confused the aggregate handlers in exciting ways.
  
  We now make NAME_CONST() behave more consistently in that when
  it is transparent with regard to basic_const_item(), it also
  is with regard to type().
[28 Feb 2008 12:37] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43139

ChangeSet@1.2595, 2008-02-28 13:35:56+01:00, tnurnberg@mysql.com +5 -0
  Bug#34749: Server crash when using NAME_CONST() with an aggregate function
  
  NAME_CONST('whatever', -1) * MAX(whatever) bombed since -1 was
  not seen as constant, but as FUNCTION_UNARY_MINUS(constant)
  while we are at the same time pretending it was a basic const
  item. This confused the aggregate handlers in exciting ways.
  We now make NAME_CONST() behave more consistently.
[28 Feb 2008 13:01] Tatiana Azundris Nuernberg
REVIEWERS! : )

There are three ChangeSets attached. This is not evolution,
it is choice.

1 - Keep original patch for Bug#30832. Least intrusive code-wise,
    but a tad messy in terms of style.

2 - Rewrite Bug#30832 to apply more narrowly (only to NAME_CONST(),
    not to all unary minus). More elegant than 1, but uses funcname()
    to identify unary minus ("My func has no name!"  "How does it smell?"
    "AWFUL!!!")

3 - Like 2, but uses new identifier NEG_FUNC (as it should be). As a
    result also teaches ndb about this type because otherwise, we'll
    lose some rewrites, and execution plans will be affected.  Most
    stylish, but also most intrusive.

See all and decide which you'd like!  ; )
http://crazy.codetroop.com/randimg/?valfritt.jpg
[28 Feb 2008 13:42] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/43149

ChangeSet@1.2595, 2008-02-28 13:54:58+01:00, tnurnberg@mysql.com +7 -0
  Bug#34749: Server crash when using NAME_CONST() with an aggregate function
  
  NAME_CONST('whatever', -1) * MAX(whatever) bombed since -1 was
  not seen as constant, but as FUNCTION_UNARY_MINUS(constant)
  while we are at the same time pretending it was a basic const
  item. This confused the aggregate handlers in exciting ways.
  We now make NAME_CONST() behave more consistently.
[8 Mar 2008 0:18] Sergey Petrunya
Ok to push patch #3.

Tatjana, thanks for going extra kilometer to present all the options.
[10 Mar 2008 6:42] Tatiana Azundris Nuernberg
autopushing to 5.0.58, 5.1.24, 6.0.5 in opt
[13 Mar 2008 19:29] Bugs System
Pushed into 6.0.5-alpha
[13 Mar 2008 19:36] Bugs System
Pushed into 5.1.24-rc
[13 Mar 2008 19:43] Bugs System
Pushed into 5.0.60
[15 Mar 2008 21:17] Jon Stephens
Documented bugfix in the 5.0.60, 5.1.24, and 6.0.5 changelogs, as follows:

        Using NAME_CONST() with a negative number and an aggregate function
        caused MySQL to crash. This could also have a negative impact on
        replication.
[2 Apr 2008 20:15] Jon Stephens
Also noted in the 5.1.23-ndb-6.3.11 changelog.
[15 Apr 2008 8:55] Valeriy Kravchuk
Bug #36084 was marked as a duplicate of this one.
[8 Jul 2011 11:20] MySQL Verification Team
Similar bug seems to have come back to haunt use.
Oracle defect #12661433