A fix and a test case for Bug#10736 "mysql_stmt_attr_set

CURSOR_TYPE_READ_ONLY select within select".
The bug was caused by the reset of thd->mem_root to thd->main_mem_root in 
Item_subselect::exec, which in turn triggered too early free_root() for
data which was needed on subsequent fetches from a cursor.
This reset also caused a memory leak in stored procedures, as 
subsequent executions of instructions containing a subselect
were allocating memory in thd->main_mem_root, which is not freed
until the end of the entire SP, instead of the per-call mem_root,
which is freed in the end of execution of the instruction.

sql/item_subselect.cc

@@ -194,15 +194,8 @@ bool Item_subselect::fix_fields(THD *thd_param, TABLE_LIST *tables, Item **ref)
 bool Item_subselect::exec()
 {
   int res;
-  MEM_ROOT *old_root= thd->mem_root;
 
-  /*
-    As this is execution, all objects should be allocated through the main
-    mem root
-  */
-  thd->mem_root= &thd->main_mem_root;
   res= engine->exec();
-  thd->mem_root= old_root;
 
   if (engine_changed)
   {

sql/opt_range.cc

@@ -325,7 +325,7 @@ typedef struct st_qsel_param {
   TABLE *table;
   KEY_PART *key_parts,*key_parts_end;
   KEY_PART *key[MAX_KEY]; /* First key parts of keys used in the query */
-  MEM_ROOT *mem_root;
+  MEM_ROOT *mem_root, *old_root;
   table_map prev_tables,read_tables,current_table;
   uint baseflag, max_key_part, range_count;
 
@@ -1665,7 +1665,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use,
   keys_to_use.intersect(head->keys_in_use_for_query);
   if (!keys_to_use.is_clear_all())
   {
-    MEM_ROOT *old_root,alloc;
+    MEM_ROOT alloc;
     SEL_TREE *tree= NULL;
     KEY_PART *key_parts;
     KEY *key_info;
@@ -1680,6 +1680,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use,
     param.table=head;
     param.keys=0;
     param.mem_root= &alloc;
+    param.old_root= thd->mem_root;
     param.needed_reg= &needed_reg;
     param.imerge_cost_buff_size= 0;
 
@@ -1695,7 +1696,6 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use,
       DBUG_RETURN(0);				// Can't use range
     }
     key_parts= param.key_parts;
-    old_root= thd->mem_root;
     thd->mem_root= &alloc;
 
     /*
@@ -1845,7 +1845,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use,
       }
     }
 
-    thd->mem_root= old_root;
+    thd->mem_root= param.old_root;
 
     /* If we got a read plan, create a quick select from it. */
     if (best_trp)
@@ -1860,7 +1860,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use,
 
   free_mem:
     free_root(&alloc,MYF(0));			// Return memory & allocator
-    thd->mem_root= old_root;
+    thd->mem_root= param.old_root;
     thd->no_errors=0;
   }
 
@@ -3695,24 +3695,38 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
 {
   uint maybe_null=(uint) field->real_maybe_null();
   bool optimize_range;
-  SEL_ARG *tree;
+  SEL_ARG *tree= 0;
+  MEM_ROOT *alloc= param->mem_root;
   char *str;
   DBUG_ENTER("get_mm_leaf");
 
+  /*
+    We need to restore the runtime mem_root of the thread in this
+    function becuase it evaluates the value of its argument, while
+    the argument can be any, e.g. a subselect. The subselect
+    items, in turn, assume that all the memory allocated during
+    the evaluation has the same life span as the item itself.
+    TODO: opt_range.cc should not reset thd->mem_root at all.
+  */
+  param->thd->mem_root= param->old_root;
   if (!value)					// IS NULL or IS NOT NULL
   {
     if (field->table->maybe_null)		// Can't use a key on this
-      DBUG_RETURN(0);
+      goto end;
     if (!maybe_null)				// Not null field
-      DBUG_RETURN(type == Item_func::ISNULL_FUNC ? &null_element : 0);
-    if (!(tree=new SEL_ARG(field,is_null_string,is_null_string)))
-      DBUG_RETURN(0);		// out of memory
+    {
+      if (type == Item_func::ISNULL_FUNC)
+        tree= &null_element;
+      goto end;
+    }
+    if (!(tree= new (alloc) SEL_ARG(field,is_null_string,is_null_string)))
+      goto end;                                 // out of memory
     if (type == Item_func::ISNOTNULL_FUNC)
     {
       tree->min_flag=NEAR_MIN;		    /* IS NOT NULL ->  X > NULL */
       tree->max_flag=NO_MAX_RANGE;
     }
-    DBUG_RETURN(tree);
+    goto end;
   }
 
   /*
@@ -3732,7 +3746,7 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
       key_part->image_type == Field::itRAW &&
       ((Field_str*)field)->charset() != conf_func->compare_collation() &&
       !(conf_func->compare_collation()->state & MY_CS_BINSORT))
-    DBUG_RETURN(0);
+    goto end;
 
   optimize_range= field->optimize_range(param->real_keynr[key_part->key],
                                         key_part->part);
@@ -3746,9 +3760,12 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
     uint field_length= field->pack_length()+maybe_null;
 
     if (!optimize_range)
-      DBUG_RETURN(0);				// Can't optimize this
+      goto end;
     if (!(res= value->val_str(&tmp)))
-      DBUG_RETURN(&null_element);
+    {
+      tree= &null_element;
+      goto end;
+    }
 
     /*
       TODO:
@@ -3761,7 +3778,7 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
       res= &tmp;
     }
     if (field->cmp_type() != STRING_RESULT)
-      DBUG_RETURN(0);				// Can only optimize strings
+      goto end;                                 // Can only optimize strings
 
     offset=maybe_null;
     length=key_part->store_length;
@@ -3786,8 +3803,8 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
 	field_length= length;
     }
     length+=offset;
-    if (!(min_str= (char*) alloc_root(param->mem_root, length*2)))
-      DBUG_RETURN(0);
+    if (!(min_str= (char*) alloc_root(alloc, length*2)))
+      goto end;
 
     max_str=min_str+length;
     if (maybe_null)
@@ -3802,20 +3819,21 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
 			      min_str+offset, max_str+offset,
 			      &min_length, &max_length);
     if (like_error)				// Can't optimize with LIKE
-      DBUG_RETURN(0);
+      goto end;
 
     if (offset != maybe_null)			// BLOB or VARCHAR
     {
       int2store(min_str+maybe_null,min_length);
       int2store(max_str+maybe_null,max_length);
     }
-    DBUG_RETURN(new SEL_ARG(field,min_str,max_str));
+    tree= new (alloc) SEL_ARG(field, min_str, max_str);
+    goto end;
   }
 
   if (!optimize_range &&
       type != Item_func::EQ_FUNC &&
       type != Item_func::EQUAL_FUNC)
-    DBUG_RETURN(0);				// Can't optimize this
+    goto end;                                   // Can't optimize this
 
   /*
     We can't always use indexes when comparing a string index to a number
@@ -3824,21 +3842,22 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
   if (field->result_type() == STRING_RESULT &&
       value->result_type() != STRING_RESULT &&
       field->cmp_type() != value->result_type())
-    DBUG_RETURN(0);
+    goto end;
 
   if (value->save_in_field_no_warnings(field, 1) < 0)
   {
     /* This happens when we try to insert a NULL field in a not null column */
-    DBUG_RETURN(&null_element);			// cmp with NULL is never TRUE
+    tree= &null_element;                        // cmp with NULL is never TRUE
+    goto end;
   }
-  str= (char*) alloc_root(param->mem_root, key_part->store_length+1);
+  str= (char*) alloc_root(alloc, key_part->store_length+1);
   if (!str)
-    DBUG_RETURN(0);
+    goto end;
   if (maybe_null)
     *str= (char) field->is_real_null();		// Set to 1 if null
   field->get_key_image(str+maybe_null, key_part->length, key_part->image_type);
-  if (!(tree=new SEL_ARG(field,str,str)))
-    DBUG_RETURN(0);		// out of memory
+  if (!(tree= new (alloc) SEL_ARG(field, str, str)))
+    goto end;                                   // out of memory
 
   /*
     Check if we are comparing an UNSIGNED integer with a negative constant.
@@ -3862,10 +3881,13 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
       if (type == Item_func::LT_FUNC || type == Item_func::LE_FUNC)
       {
         tree->type= SEL_ARG::IMPOSSIBLE;
-        DBUG_RETURN(tree);
+        goto end;
       }
       if (type == Item_func::GT_FUNC || type == Item_func::GE_FUNC)
-        DBUG_RETURN(0);
+      {
+        tree= 0;
+        goto end;
+      }
     }
   }
 
@@ -3928,6 +3950,9 @@ get_mm_leaf(PARAM *param, COND *conf_func, Field *field, KEY_PART *key_part,
   default:
     break;
   }
+
+end:
+  param->thd->mem_root= alloc;
   DBUG_RETURN(tree);
 }
 

tests/mysql_client_test.c

@@ -13332,6 +13332,64 @@ static void test_bug9992()
   mysql_close(mysql1);
 }
 
+
+/* Bug#10736: cursors and subqueries, memroot management */
+
+static void test_bug10736()
+{
+  MYSQL_STMT *stmt;
+  MYSQL_BIND bind[1];
+  char a[21];
+  int rc;
+  const char *stmt_text;
+  int i= 0;
+  ulong type;
+
+  myheader("test_bug10736");
+
+  mysql_query(mysql, "drop table if exists t1");
+  mysql_query(mysql, "create table t1 (id integer not null primary key,"
+                                      "name VARCHAR(20) NOT NULL)");
+  rc= mysql_query(mysql, "insert into t1 (id, name) values "
+                         "(1, 'aaa'), (2, 'bbb'), (3, 'ccc')");
+  myquery(rc);
+
+  stmt= mysql_stmt_init(mysql);
+
+  type= (ulong) CURSOR_TYPE_READ_ONLY;
+  rc= mysql_stmt_attr_set(stmt, STMT_ATTR_CURSOR_TYPE, (void*) &type);
+  check_execute(stmt, rc);
+  stmt_text= "select name from t1 where name=(select name from t1 where id=2)";
+  rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text));
+  check_execute(stmt, rc);
+
+  bzero(bind, sizeof(bind));
+  bind[0].buffer_type= MYSQL_TYPE_STRING;
+  bind[0].buffer= (void*) a;
+  bind[0].buffer_length= sizeof(a);
+  mysql_stmt_bind_result(stmt, bind);
+
+  for (i= 0; i < 3; i++)
+  {
+    int row_no= 0;
+    rc= mysql_stmt_execute(stmt);
+    check_execute(stmt, rc);
+    while ((rc= mysql_stmt_fetch(stmt)) == 0)
+    {
+      if (!opt_silent)
+        printf("%d: %s\n", row_no, a);
+      ++row_no;
+    }
+    DIE_UNLESS(rc == MYSQL_NO_DATA);
+  }
+  rc= mysql_stmt_close(stmt);
+  DIE_UNLESS(rc == 0);
+
+  rc= mysql_query(mysql, "drop table t1");
+  myquery(rc);
+}
+
+
 /*
   Read and parse arguments and MySQL options from my.cnf
 */
@@ -13567,6 +13625,7 @@ static struct my_tests_st my_tests[]= {
   { "test_bug10729", test_bug10729 },
   { "test_bug11111", test_bug11111 },
   { "test_bug9992", test_bug9992 },
+  { "test_bug10736", test_bug10736 },
   { 0, 0 }
 };