List:Commits« Previous MessageNext Message »
From:Chad MILLER Date:October 2 2006 4:54pm
Subject:bk commit into 5.0 tree (cmiller:1.2275) BUG#19536
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 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, 2006-10-02 12:54:26-04:00, cmiller@stripped +3 -0
  Bug#19536: 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@stripped, 2006-10-02 12:54:24-04:00, cmiller@stripped +33 -0
    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@stripped, 2006-10-02 12:54:24-04:00, cmiller@stripped +34 -0
    Create tests to prove that undefined variables work, as keys and not, and 
    that variables explicitly assigned to Null work.

  sql/sql_prepare.cc@stripped, 2006-10-02 12:54:24-04:00, cmiller@stripped +34 -0
    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.)

# 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/bug19356/my50-bug19356

--- 1.4/mysql-test/r/ps_11bugs.result	2006-10-02 12:54:31 -04:00
+++ 1.5/mysql-test/r/ps_11bugs.result	2006-10-02 12:54:31 -04:00
@@ -130,3 +130,36 @@ prepare st_18492 from 'select * from t1 
 execute st_18492;
 a
 drop table t1;
+create table t1 (a int, b varchar(4));
+create table t2 (a int, b varchar(4), primary key(a));
+prepare stmt1 from 'insert into t1 (a, b) values (?, ?)';
+prepare stmt2 from 'insert into t2 (a, b) values (?, ?)';
+set @intarg= 11;
+set @varchararg= '2222';
+execute stmt1 using @intarg, @varchararg;
+execute stmt2 using @intarg, @varchararg;
+set @intarg= 12;
+execute stmt1 using @intarg, @UNDEFINED;
+execute stmt2 using @intarg, @UNDEFINED;
+set @intarg= 13;
+execute stmt1 using @UNDEFINED, @varchararg;
+execute stmt2 using @UNDEFINED, @varchararg;
+ERROR 23000: Column 'a' cannot be null
+set @intarg= 14;
+set @nullarg= Null;
+execute stmt1 using @UNDEFINED, @nullarg;
+execute stmt2 using @nullarg, @varchararg;
+ERROR 23000: Column 'a' cannot be null
+select * from t1;
+a	b
+11	2222
+12	NULL
+NULL	2222
+NULL	NULL
+select * from t2;
+a	b
+11	2222
+12	NULL
+drop table t1;
+drop table t2;
+End of 5.0 tests.

--- 1.6/mysql-test/t/ps_11bugs.test	2006-10-02 12:54:31 -04:00
+++ 1.7/mysql-test/t/ps_11bugs.test	2006-10-02 12:54:31 -04:00
@@ -144,3 +144,37 @@ prepare st_18492 from 'select * from t1 
 execute st_18492;
 
 drop table t1;
+
+#
+# Bug#19356: Assertion failure with undefined @uservar in prepared statement execution
+# 
+create table t1 (a int, b varchar(4));
+create table t2 (a int, b varchar(4), primary key(a));
+
+prepare stmt1 from 'insert into t1 (a, b) values (?, ?)';
+prepare stmt2 from 'insert into t2 (a, b) values (?, ?)';
+
+set @intarg= 11;
+set @varchararg= '2222';
+execute stmt1 using @intarg, @varchararg;
+execute stmt2 using @intarg, @varchararg;
+set @intarg= 12;
+execute stmt1 using @intarg, @UNDEFINED;
+execute stmt2 using @intarg, @UNDEFINED;
+set @intarg= 13;
+execute stmt1 using @UNDEFINED, @varchararg;
+--error 1048
+execute stmt2 using @UNDEFINED, @varchararg;
+set @intarg= 14;
+set @nullarg= Null;
+execute stmt1 using @UNDEFINED, @nullarg;
+--error 1048
+execute stmt2 using @nullarg, @varchararg;
+
+select * from t1;
+select * from t2;
+
+drop table t1;
+drop table t2;
+
+--echo End of 5.0 tests.

--- 1.182/sql/sql_prepare.cc	2006-10-02 12:54:31 -04:00
+++ 1.183/sql/sql_prepare.cc	2006-10-02 12:54:31 -04:00
@@ -2310,6 +2310,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 +2795,28 @@ bool Prepared_statement::prepare(const c
     external changes when cleaning up after validation.
   */
   DBUG_ASSERT(thd->change_list.is_empty());
+
+  /* 
+   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.
+
+   See Bug#19356.  Kostja says that this should be refactored.  An
+   assertion should be possible near here, but only before 
+   stmt->set_params_from_vars() .  It should not be possible to 
+   create new Item objects between setting parameters and here,
+   because we could free too much.
+  
+   See similar case in ::execute().
+  */
+#if 0
   /*
     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);
+#endif
 
   if (error == 0)
     error= check_prepared_statement(this, name.str != 0);
@@ -2891,7 +2914,18 @@ bool Prepared_statement::execute(String 
     allocated items when cleaning up after execution of this statement.
   */
   DBUG_ASSERT(thd->change_list.is_empty());
+
+  /* 
+   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.
+
+   See Bug#19356 and similar case in ::prepare().
+  */
+#if 0
   DBUG_ASSERT(thd->free_list == NULL);
+#endif
+
   thd->set_n_backup_statement(this, &stmt_backup);
   if (expanded_query->length() &&
       alloc_query(thd, (char*) expanded_query->ptr(),
Thread
bk commit into 5.0 tree (cmiller:1.2275) BUG#19536Chad MILLER2 Oct