Bug#19356: Assert on undefined @uservar in prepared statement execute

The executing code had a safety assertion so that it refused to free Items
that it didn't create.  However, there is a case, undefined user variables,
which would put Items into the list to be freed.

Instead, do something that is more risky in expectation that the code will 
be refactored soon, as Kostja wants to do:  Remove the assertions from 
prepare() and execute().  Put one assertion at a higher level, before 
stmt->set_params_from_vars(), which may then create new to-be-freed Items .


mysql-test/r/ps_11bugs.result:
  Create tests to prove that undefined variables work, as keys and not, and 
  that variables explicitly assigned to Null work.
mysql-test/t/ps_11bugs.test:
  Create tests to prove that undefined variables work, as keys and not, and 
  that variables explicitly assigned to Null work.
sql/sql_prepare.cc:
  Move a safety assertion up one level and higher, because there is 
  legitimately a case where thd->free_list is not NULL going into 
  Prepared_statement::{prepare,execute} methods.
  
  Kostja plans to refactor this code so that it is both safe and works.  
  (Now it works, but isn't very safe.)

1 files changed, 27 insertions, 6 deletions

diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index 32f0ca6859d..9e321411863 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -2248,6 +2248,14 @@ void mysql_stmt_execute(THD *thd, char *packet_arg, uint packet_length)
 #endif
   if (!(specialflag & SPECIAL_NO_PRIOR))
     my_pthread_setprio(pthread_self(),QUERY_PRIOR);
+
+  /*
+    If the free_list is not empty, we'll wrongly free some externally
+    allocated items when cleaning up after validation of the prepared
+    statement.
+  */
+  DBUG_ASSERT(thd->free_list == NULL);
+
   error= stmt->execute(&expanded_query,
                        test(flags & (ulong) CURSOR_TYPE_READ_ONLY));
   if (!(specialflag & SPECIAL_NO_PRIOR))
@@ -2310,6 +2318,13 @@ void mysql_sql_stmt_execute(THD *thd)
 
   DBUG_PRINT("info",("stmt: %p", stmt));
 
+  /*
+    If the free_list is not empty, we'll wrongly free some externally
+    allocated items when cleaning up after validation of the prepared
+    statement.
+  */
+  DBUG_ASSERT(thd->free_list == NULL);
+
   if (stmt->set_params_from_vars(stmt, lex->prepared_stmt_params,
                                  &expanded_query))
     goto set_params_data_err;
@@ -2788,12 +2803,12 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len)
     external changes when cleaning up after validation.
   */
   DBUG_ASSERT(thd->change_list.is_empty());
-  /*
-    If the free_list is not empty, we'll wrongly free some externally
-    allocated items when cleaning up after validation of the prepared
-    statement.
+
+  /* 
+   The only case where we should have items in the thd->free_list is
+   after stmt->set_params_from_vars(), which may in some cases create
+   Item_null objects.
   */
-  DBUG_ASSERT(thd->free_list == NULL);
 
   if (error == 0)
     error= check_prepared_statement(this, name.str != 0);
@@ -2891,7 +2906,13 @@ bool Prepared_statement::execute(String *expanded_query, bool open_cursor)
     allocated items when cleaning up after execution of this statement.
   */
   DBUG_ASSERT(thd->change_list.is_empty());
-  DBUG_ASSERT(thd->free_list == NULL);
+
+  /* 
+   The only case where we should have items in the thd->free_list is
+   after stmt->set_params_from_vars(), which may in some cases create
+   Item_null objects.
+  */
+
   thd->set_n_backup_statement(this, &stmt_backup);
   if (expanded_query->length() &&
       alloc_query(thd, (char*) expanded_query->ptr(),