bk commit into 4.1 tree (cmiller:1.2624) BUG#27484

[Chad MILLER]

Below is the list of changes that have just been committed into a local
4.1 repository of cmiller. When cmiller does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-04-09 13:29:51-04:00, cmiller@stripped +7 -0
  Bug#27484: server crash with nested rows
  
  There are two possible symptoms of this problem:  1) A server 
  crash, 2) or lossy comparison that signaled equality when there 
  exists an orthogonal "row" that does differ.
  
  Now, verify that two rows that we're comparing for equality have
  the same structure, as a shortcut to being able to signal 
  inequality before testing any values.

  mysql-test/r/row.result@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +24
-0
    Verify that differing levels of nested rows raise an error and that
    IN comparisons do not cause crashes.

  mysql-test/t/row.test@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +41 -0
    Verify that differing levels of nested rows raise an error and that
    IN comparisons do not cause crashes.

  sql/item.cc@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +48 -0
    Add a function that recurses into pairs of complex items, verifying
    that both contain the same number of elements.

  sql/item.h@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +4 -0
    Add prototype and declare a flag to ensure that the recursion graph
    isn't cyclical.

  sql/item_cmpfunc.cc@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +16 -1
    Assert that comparison functions for rows receive the same number
    of columns.
    
    Additionally, fix a crash by returning if we've reported an error.

  sql/item_cmpfunc.h@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +14 -0
    Make a count of the columns available to row Items.

  sql/item_func.cc@stripped, 2007-04-09 13:29:50-04:00, cmiller@stripped +10 -0
    For function Items, verify that all arguments are isometric.

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	cmiller
# Host:	zippy.cornsilk.net
# Root:	/home/cmiller/work/mysql/my41-bug27484

--- 1.238/sql/item.cc	2007-02-14 12:21:52 -05:00
+++ 1.239/sql/item.cc	2007-04-09 13:29:50 -04:00
@@ -48,6 +48,9 @@ Item::Item():
   name= 0;
   decimals= 0; max_length= 0;
   with_subselect= 0;
+#ifndef DBUG_OFF
+  in_check_cols_deeply= FALSE;
+#endif
 
   /* Put item in free list so that we can free all items at end */
   THD *thd= current_thd;
@@ -88,6 +91,9 @@ Item::Item(THD *thd, Item *item):
 {
   next= thd->free_list;				// Put in free list
   thd->free_list= this;
+#ifndef DBUG_OFF
+  in_check_cols_deeply= FALSE;
+#endif
 }
 
 
@@ -161,6 +167,48 @@ bool Item::check_cols(uint c)
   return 0;
 }
 
+/**
+  Recurse into items to make sure that the two items have the same number
+  of columns.
+  @param  compared  The object comparing against "this" item.
+  @return  Whether there was an error (which would be reported inside).
+*/
+bool Item::check_cols_deeply(Item* compared)
+{
+  DBUG_ENTER("Item::check_cols_deeply");
+#ifndef DBUG_OFF
+  /* Be certain that this item doesn't recurse back into itself. */
+  DBUG_ASSERT(! in_check_cols_deeply);
+  in_check_cols_deeply= TRUE;
+#endif
+
+  /* It is an error if these two items have a different number of columns. */
+  if (cols() != compared->cols())
+  {
+    my_error(ER_OPERAND_COLUMNS, MYF(0), compared->cols());
+    DBUG_RETURN(TRUE);                   /* is an error */
+  }
+
+  /* Check isometry of each column. */
+  for (int c= 0; c < cols(); c++)
+  {
+    Item* i= el(c);
+    if (i == this)
+      continue;
+    
+    if (i->check_cols_deeply(compared->el(c)))  /* "TRUE" on error */
+    {
+#ifndef DBUG_OFF
+      in_check_cols_deeply= FALSE;
+#endif
+      DBUG_RETURN(TRUE);                /* error already reported. pass up. */
+    }
+  }
+#ifndef DBUG_OFF
+  in_check_cols_deeply= FALSE;
+#endif
+  DBUG_RETURN(FALSE);                   /* no error */
+}
 
 void Item::set_name(const char *str, uint length, CHARSET_INFO *cs)
 {

--- 1.197/sql/item.h	2007-01-25 21:44:32 -05:00
+++ 1.198/sql/item.h	2007-04-09 13:29:50 -04:00
@@ -323,6 +323,10 @@ public:
   virtual Item* el(uint i) { return this; }
   virtual Item** addr(uint i) { return 0; }
   virtual bool check_cols(uint c);
+#ifndef DBUG_OFF
+  bool in_check_cols_deeply;            /* Node visited in recursive search? */
+#endif
+  bool check_cols_deeply(Item* item);
   // It is not row => null inside is impossible
   virtual bool null_inside() { return 0; }
   // used in row subselects to get value of elements

--- 1.219/sql/item_cmpfunc.cc	2007-02-13 10:53:59 -05:00
+++ 1.220/sql/item_cmpfunc.cc	2007-04-09 13:29:50 -04:00
@@ -1884,6 +1884,7 @@ void cmp_item_row::store_value_by_templa
 int cmp_item_row::cmp(Item *arg)
 {
   arg->null_value= 0;
+  DBUG_ASSERT(n == arg->cols());
   if (arg->cols() != n)
   {
     my_error(ER_OPERAND_COLUMNS, MYF(0), n);
@@ -1907,6 +1908,17 @@ int cmp_item_row::cmp(Item *arg)
 int cmp_item_row::compare(cmp_item *c)
 {
   cmp_item_row *cmp= (cmp_item_row *) c;
+  /*
+    This also asserts that c is a row, because rows have min 2 cols
+    if n >= 2 then c->cols >= 2
+  */
+  if (c->cols() != n)
+  {
+    DBUG_ASSERT(FALSE);                 /* Shouldn't be possible. */
+
+    my_error(ER_OPERAND_COLUMNS, MYF(0), n);
+    return 1;
+  }
   for (uint i=0; i < n; i++)
   {
     int res;
@@ -2037,8 +2049,11 @@ void Item_func_in::fix_length_and_dec()
 	else
 	  have_null= 1;
       }
+      /* If an error inside array->set occured, array->sort could fail */
+      if (thd->net.report_error)
+        return;
       if ((array->used_count=j))
-	array->sort();
+        array->sort();
     }
   }
   else

--- 1.121/sql/item_cmpfunc.h	2007-02-13 10:53:59 -05:00
+++ 1.122/sql/item_cmpfunc.h	2007-04-09 13:29:50 -04:00
@@ -639,8 +639,18 @@ public:
   virtual int compare(cmp_item *item)= 0;
   static cmp_item* get_comparator(Item *);
   virtual cmp_item *make_same()= 0;
+  virtual uint cols()
+  {
+    return 1;
+  }
   virtual void store_value_by_template(cmp_item *tmpl, Item *item)
   {
+    DBUG_ASSERT(tmpl->cols() == item->cols());
+    if (tmpl->cols() != item->cols())
+    {
+      my_error(ER_OPERAND_COLUMNS, MYF(0), tmpl->cols());
+      return;
+    }
     store_value(item);
   }
 };
@@ -735,6 +745,10 @@ public:
   int cmp(Item *arg);
   int compare(cmp_item *arg);
   cmp_item *make_same();
+  uint cols()
+  {
+    return n;
+  }
   void store_value_by_template(cmp_item *tmpl, Item *);
 };
 

--- 1.273/sql/item_func.cc	2007-02-24 18:10:03 -05:00
+++ 1.274/sql/item_func.cc	2007-04-09 13:29:50 -04:00
@@ -159,7 +159,17 @@ Item_func::fix_fields(THD *thd, TABLE_LI
 
       if (allowed_arg_cols)
       {
+        /*
+          allowed_arg_cols could have been set by the function, rather than
+          being fetched from the 1st column.
+        */
         if (item->check_cols(allowed_arg_cols))
+          return 1;
+        /*
+          Don't compare the item with itself.  All args to a function must have
+          the same structure.
+        */
+        if (arg != args && item->check_cols_deeply(*args))
           return 1;
       }
       else

--- 1.18/mysql-test/r/row.result	2004-05-13 16:46:58 -04:00
+++ 1.19/mysql-test/r/row.result	2007-04-09 13:29:50 -04:00
@@ -170,3 +170,27 @@ ROW(2,10) <=> ROW(3,4)
 SELECT ROW(NULL,10) <=> ROW(3,NULL);
 ROW(NULL,10) <=> ROW(3,NULL)
 0
+select row(1,row(2,3))   IN (row(1,row(2,3))   ,row(1,1));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3))   IN (row(1,row(2,3))   ,row(1,1), row(1,row(2,3)));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3))   IN (row(1,row(2,3))   ,row(1,row(2,2,2)));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3,4)) IN (row(1,row(2,3,4)) ,row(1,row(2,2)));
+ERROR 21000: Operand should contain 3 column(s)
+select row(1,row(2,3))   IN (row(1,1),          row(1,row(2,3)));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3))   IN (row(1,row(2,2,2)), row(1,row(2,3)));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3,4)) IN (row(1,row(2,2)),   row(1,row(2,3,4)));
+ERROR 21000: Operand should contain 3 column(s)
+select row(1,row(2,3)) IN (row(1,row(2,3)), (select 1,1));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3)) IN (row(1,row(2,3)), (select 1,1), row(1,row(2,4)));
+ERROR 21000: Operand should contain 2 column(s)
+select row(1,row(2,3)) IN ((select 1,1), row(1,row(2,3)));
+ERROR 21000: Operand should contain 2 column(s)
+select row(2,1) IN (row(21,2), row(row(1,1,3),0) );
+ERROR 21000: Operand should contain 1 column(s)
+select row(2,1) IN (row(row(1,1,3),0), row(21,2));
+ERROR 21000: Operand should contain 1 column(s)

--- 1.17/mysql-test/t/row.test	2005-07-27 20:21:46 -04:00
+++ 1.18/mysql-test/t/row.test	2007-04-09 13:29:50 -04:00
@@ -83,4 +83,45 @@ drop table t1;
 SELECT ROW(2,10) <=> ROW(3,4);
 SELECT ROW(NULL,10) <=> ROW(3,NULL);
 
+
+#
+# BUG #27484, row() in "IN()"
+# Nested row() in "IN()" could cause a crash if a row was compared to a
+# different type or a row of different length.
+
+# qsort / compare
+-- error 1241
+select row(1,row(2,3))   IN (row(1,row(2,3))   ,row(1,1));
+-- error 1241
+select row(1,row(2,3))   IN (row(1,row(2,3))   ,row(1,1), row(1,row(2,3)));
+-- error 1241
+select row(1,row(2,3))   IN (row(1,row(2,3))   ,row(1,row(2,2,2)));
+-- error 1241
+select row(1,row(2,3,4)) IN (row(1,row(2,3,4)) ,row(1,row(2,2)));
+
+# the following were not affected, but should be tested
+-- error 1241
+select row(1,row(2,3))   IN (row(1,1),          row(1,row(2,3)));
+-- error 1241
+select row(1,row(2,3))   IN (row(1,row(2,2,2)), row(1,row(2,3)));
+-- error 1241
+select row(1,row(2,3,4)) IN (row(1,row(2,2)),   row(1,row(2,3,4)));
+
+
+#using subselect in nested row() and IN(), invalid statements were accepted with no error
+--error 1241
+select row(1,row(2,3)) IN (row(1,row(2,3)), (select 1,1));
+--error 1241
+select row(1,row(2,3)) IN (row(1,row(2,3)), (select 1,1), row(1,row(2,4)));
+--error 1241
+select row(1,row(2,3)) IN ((select 1,1), row(1,row(2,3)));
+
+#debug asserts happened with nested row() / run the below with a debug server
+
+-- error 1241
+select row(2,1) IN (row(21,2), row(row(1,1,3),0) );
+-- error 1241
+select row(2,1) IN (row(row(1,1,3),0), row(21,2));
+
+
 # End of 4.1 tests