Fix for remaining issues described in Bug #1664

"mysql_send_long_data() API call is completely broken".

Now we are resetting some members (long_data_supplied/null_value...) of Item_param to its 
initial state after each execution of prepared statement. We also manipulating 
Item_param::maybe_null/null_value only via Item_param::set_* setters which makes code a bit 
more robust.


sql/item.cc:
  Now we are assuming that Item_param may be NULL until we know this fact exactly.
  Added non-empty implementation of Item_param::reset() method which should be used
  for restoring Item_param state after each statment execution. (We need to clear 
  long_data_supplied flag, we also clear some other Item_param members here since it
  makes code simpler.)
sql/item.h:
  Now Item_param::reset() method really does something.
sql/sql_prepare.cc:
  Now we are calling Item_param::reset() for each parameter after execution for resetting Item_param
  to initial state. So we no longer don't need Prepared_statement::long_data_flag. We also 
  set Item_param::null_value/maybe_null value in Item_param::set_* and reset() methods 
  instead of doing it explicitly in insert_params_* functions (this by the way lowers 
  probability that we will forget to update one of such functions).
tests/client_test.c:
  Added test for Bug#1664 "mysql_send_long_data() API call is broken".

4 files changed, 211 insertions, 22 deletions

diff --git a/sql/item.cc b/sql/item.cc
index 72583ea02bb..44a05c42526 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -605,12 +605,19 @@ Item_param::Item_param(unsigned position) :
   set_param_func(default_set_param_func)
 {
   name= (char*) "?";
+  /* 
+    Since we can't say whenever this item can be NULL or cannot be NULL
+    before mysql_stmt_execute(), so we assuming that it can be NULL until
+    value is set.
+  */
+  maybe_null= 1;
 }
 
 void Item_param::set_null()
 {
   DBUG_ENTER("Item_param::set_null");
-  maybe_null= null_value= value_is_set= 1;
+  /* These are cleared after each execution by reset() method */
+  null_value= value_is_set= 1;
   DBUG_VOID_RETURN;
 }
 
@@ -620,6 +627,7 @@ void Item_param::set_int(longlong i)
   int_value= (longlong)i;
   item_type= INT_ITEM;
   value_is_set= 1;
+  maybe_null= 0;
   DBUG_PRINT("info", ("integer: %lld", int_value));
   DBUG_VOID_RETURN;
 }
@@ -630,6 +638,7 @@ void Item_param::set_double(double value)
   real_value=value;
   item_type= REAL_ITEM;
   value_is_set= 1;
+  maybe_null= 0;
   DBUG_PRINT("info", ("double: %lg", real_value));
   DBUG_VOID_RETURN;
 }
@@ -641,6 +650,7 @@ void Item_param::set_value(const char *str, uint length)
   str_value.copy(str,length,default_charset());
   item_type= STRING_ITEM;
   value_is_set= 1;
+  maybe_null= 0;
   DBUG_PRINT("info", ("string: %s", str_value.ptr()));
   DBUG_VOID_RETURN;
 }
@@ -665,6 +675,7 @@ void Item_param::set_time(TIME *tm, timestamp_type type)
   item_is_time= TRUE;
   item_type= STRING_ITEM;
   value_is_set= 1;
+  maybe_null= 0;
 }
 
 
@@ -673,6 +684,27 @@ void Item_param::set_longdata(const char *str, ulong length)
   str_value.append(str,length);
   long_data_supplied= 1;
   value_is_set= 1;
+  maybe_null= 0;
+}
+
+
+/*
+    Resets parameter after execution.
+  
+  SYNOPSIS
+     Item_param::reset()
+ 
+  NOTES
+    We clear null_value here instead of setting it in set_* methods, 
+    because we want more easily handle case for long data.
+*/
+
+void Item_param::reset()
+{  
+  str_value.set("", 0, &my_charset_bin);
+  value_is_set= long_data_supplied= 0;
+  maybe_null= 1;
+  null_value= 0;
 }
 
 
diff --git a/sql/item.h b/sql/item.h
index 0f2927b0d99..b756d824de0 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -419,7 +419,7 @@ public:
   void set_long_end();
   void set_time(TIME *tm, timestamp_type type);
   bool get_time(TIME *tm);
-  void reset() {}
+  void reset();
   /*
     Assign placeholder value from bind data.
     Note, that 'len' has different semantics in embedded library (as we
diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc
index ff650f9c66a..0ef7dbcf594 100644
--- a/sql/sql_prepare.cc
+++ b/sql/sql_prepare.cc
@@ -91,7 +91,6 @@ public:
   uint last_errno;
   char last_error[MYSQL_ERRMSG_SIZE];
   bool get_longdata_error;
-  bool long_data_used;
   bool log_full_query;
 #ifndef EMBEDDED_LIBRARY
   bool (*set_params)(Prepared_statement *st, uchar *data, uchar *data_end,
@@ -465,12 +464,11 @@ static bool insert_params_withlog(Prepared_statement *stmt, uchar *null_array,
     {
       if (is_param_null(null_array, it - begin))
       {
-        param->maybe_null= param->null_value= param->value_is_set= 1;
+        param->set_null();
         res= &my_null_string;
       }
       else
       {
-        param->maybe_null= param->null_value= 0;
         if (read_pos >= data_end)
           DBUG_RETURN(1);
         param->set_param_func(param, &read_pos, data_end - read_pos);
@@ -503,10 +501,9 @@ static bool insert_params(Prepared_statement *stmt, uchar *null_array,
     if (!param->long_data_supplied)
     {
       if (is_param_null(null_array, it - begin))
-        param->maybe_null= param->null_value= param->value_is_set= 1;
+        param->set_null();
       else
       {
-        param->maybe_null= param->null_value= 0;
         if (read_pos >= data_end)
           DBUG_RETURN(1);
         param->set_param_func(param, &read_pos, data_end - read_pos);
@@ -562,11 +559,10 @@ static bool emb_insert_params(Prepared_statement *stmt)
     if (!param->long_data_supplied)
     {
       if (*client_param->is_null)
-        param->maybe_null= param->null_value= 1;
+        param->set_null();
       else
       {
 	uchar *buff= (uchar*)client_param->buffer;
-        param->maybe_null= param->null_value= 0;
         param->set_param_func(param, &buff,
                               client_param->length ? 
                               *client_param->length : 
@@ -604,13 +600,12 @@ static bool emb_insert_params_withlog(Prepared_statement *stmt)
     {
       if (*client_param->is_null)
       {
-        param->maybe_null= param->null_value= 1;
+        param->set_null();
         res= &my_null_string;
       }
       else
       {
 	uchar *buff= (uchar*)client_param->buffer;
-        param->maybe_null= param->null_value= 0;
         param->set_param_func(param, &buff,
                               client_param->length ? 
                               *client_param->length : 
@@ -1439,6 +1434,24 @@ static void reset_stmt_for_execute(Prepared_statement *stmt)
   }
 }
 
+
+/* 
+    Clears parameters from data left from previous execution or long data
+    
+  SYNOPSIS
+    reset_stmt_params()
+      stmt - prepared statement for which parameters should be reset
+*/
+
+static void reset_stmt_params(Prepared_statement *stmt)
+{
+  Item_param **item= stmt->param_array;
+  Item_param **end= item + stmt->param_count;
+  for (;item < end ; ++item)
+    (**item).reset();
+}
+
+
 /*
   Executes previously prepared query.
   If there is any parameters, then replace markers with the data supplied
@@ -1512,11 +1525,13 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
     my_pthread_setprio(pthread_self(), WAIT_PRIOR);
 
   cleanup_items(stmt->free_list);
+  reset_stmt_params(stmt);
   close_thread_tables(thd); // to close derived tables
   thd->set_statement(&thd->stmt_backup);
   DBUG_VOID_RETURN;
 
 set_params_data_err:
+  reset_stmt_params(stmt);
   thd->set_statement(&thd->stmt_backup);
   my_error(ER_WRONG_ARGUMENTS, MYF(0), "mysql_execute");
   send_error(thd);
@@ -1552,15 +1567,12 @@ void mysql_stmt_reset(THD *thd, char *packet)
 
   stmt->get_longdata_error= 0;
 
-  /* Free long data if used */
-  if (stmt->long_data_used)
-  {
-    Item_param **item= stmt->param_array;
-    Item_param **end= item + stmt->param_count;
-    stmt->long_data_used= 0;
-    for (; item < end ; item++)
-      (**item).reset();
-  }
+  /* 
+    Clear parameters from data which could be set by 
+    mysql_stmt_send_long_data() call.
+  */
+  reset_stmt_params(stmt);
+  
   DBUG_VOID_RETURN;
 }
 
@@ -1646,7 +1658,6 @@ void mysql_stmt_get_longdata(THD *thd, char *pos, ulong packet_length)
 #else
   param->set_longdata(thd->extra_data, thd->extra_length);
 #endif
-  stmt->long_data_used= 1;
   DBUG_VOID_RETURN;
 }
 
@@ -1658,7 +1669,6 @@ Prepared_statement::Prepared_statement(THD *thd_arg)
   param_count(0),
   last_errno(0),
   get_longdata_error(0),
-  long_data_used(0),
   log_full_query(0)
 {
   *last_error= '\0';
diff --git a/tests/client_test.c b/tests/client_test.c
index 7d7b21bd4bf..475e3dc8cce 100644
--- a/tests/client_test.c
+++ b/tests/client_test.c
@@ -9102,6 +9102,152 @@ static void test_xjoin()
 
 
 /*
+  This tests for various mysql_send_long_data bugs described in #1664
+*/
+
+static void test_bug1664()
+{
+    MYSQL_STMT *stmt;
+    int        rc, int_data;
+    const char *data;
+    const char *str_data= "Simple string";
+    MYSQL_BIND bind[2];
+    const char *query= "INSERT INTO test_long_data(col2, col1) VALUES(?,?)";
+    
+    myheader("test_bug1664");
+    
+    rc= mysql_query(mysql,"DROP TABLE IF EXISTS test_long_data");
+    myquery(rc);
+    
+    rc= mysql_query(mysql,"CREATE TABLE test_long_data(col1 int, col2 long varchar)");
+    myquery(rc);
+    
+    stmt= mysql_stmt_init(mysql);
+    mystmt_init(stmt);
+    rc= mysql_stmt_prepare(stmt, query, strlen(query));
+    mystmt(stmt, rc);
+    
+    verify_param_count(stmt, 2);
+   
+    bzero(&bind, sizeof(bind));
+    
+    bind[0].buffer_type= FIELD_TYPE_STRING;
+    bind[0].buffer= (char *)str_data;
+    bind[0].buffer_length= strlen(str_data);
+
+    bind[1].buffer= (char *)&int_data;
+    bind[1].buffer_type= FIELD_TYPE_LONG;
+    
+    rc= mysql_stmt_bind_param(stmt,bind);
+    mystmt(stmt, rc);
+    
+    int_data= 1;
+
+    /* 
+      Let us supply empty long_data. This should work and should 
+      not break following execution.
+    */
+    data= "";
+    rc= mysql_stmt_send_long_data(stmt,0,data,strlen(data));
+    mystmt(stmt,rc);
+
+    rc= mysql_stmt_execute(stmt);
+    fprintf(stdout," mysql_execute() returned %d\n",rc);
+    mystmt(stmt,rc);
+    
+    verify_col_data("test_long_data","col1","1");
+    verify_col_data("test_long_data","col2","");
+
+    rc= mysql_query(mysql,"DELETE FROM test_long_data");
+    myquery(rc);
+    
+    /* This should pass OK */
+    data= (char *)"Data";
+    rc= mysql_stmt_send_long_data(stmt, 0, data, strlen(data));
+    mystmt(stmt, rc);
+
+    rc= mysql_stmt_execute(stmt);
+    fprintf(stdout," mysql_execute() returned %d\n",rc);
+    mystmt(stmt, rc);
+    
+    verify_col_data("test_long_data", "col1", "1");
+    verify_col_data("test_long_data", "col2", "Data");
+
+    /* clean up */
+    rc= mysql_query(mysql, "DELETE FROM test_long_data");
+    myquery(rc);
+
+    /*
+      Now we are changing int parameter and don't do anything
+      with first parameter. Second mysql_execute() should run
+      OK treating this first parameter as string parameter.
+    */
+    
+    int_data= 2;
+    /* execute */
+    rc = mysql_stmt_execute(stmt);
+    fprintf(stdout, " mysql_execute() returned %d\n", rc);
+    mystmt(stmt, rc);
+  
+    verify_col_data("test_long_data", "col1", "2");
+    verify_col_data("test_long_data", "col2", str_data);
+
+    /* clean up */
+    rc= mysql_query(mysql, "DELETE FROM test_long_data");
+    myquery(rc);
+    
+    /*
+      Now we are sending other long data. It should not be 
+      concatened to previous.
+    */
+
+    data= (char *)"SomeOtherData";
+    rc= mysql_stmt_send_long_data(stmt, 0, data, strlen(data));
+    mystmt(stmt, rc);
+    
+    rc= mysql_stmt_execute(stmt);
+    fprintf(stdout, " mysql_execute() returned %d\n", rc);
+    mystmt(stmt, rc);
+
+    verify_col_data("test_long_data", "col1", "2");
+    verify_col_data("test_long_data", "col2", "SomeOtherData");
+    
+    mysql_stmt_close(stmt);
+
+    /* clean up */
+    rc= mysql_query(mysql, "DELETE FROM test_long_data");
+    myquery(rc);
+    
+    /* Now let us test how mysql_stmt_reset works. */
+    stmt= mysql_stmt_init(mysql);
+    mystmt_init(stmt);
+    rc= mysql_stmt_prepare(stmt, query, strlen(query));
+    mystmt(stmt, rc);
+    rc= mysql_bind_param(stmt, bind);
+    mystmt(stmt, rc);
+    
+    data= (char *)"SomeData";
+    rc= mysql_stmt_send_long_data(stmt, 0, data, strlen(data));
+    mystmt(stmt, rc);
+
+    rc= mysql_stmt_reset(stmt);
+    mystmt(stmt, rc);
+
+    rc= mysql_stmt_execute(stmt);
+    fprintf(stdout," mysql_execute() returned %d\n",rc);
+    mystmt(stmt, rc);
+    
+    verify_col_data("test_long_data", "col1", "2");
+    verify_col_data("test_long_data", "col2", str_data);
+
+    mysql_stmt_close(stmt);
+    
+    /* Final clean up */
+    rc= mysql_query(mysql, "DROP TABLE test_long_data");
+    myquery(rc);
+}
+
+/*
   Read and parse arguments and MySQL options from my.cnf
 */
 
@@ -9373,6 +9519,7 @@ int main(int argc, char **argv)
     test_bind_nagative();   /* bind negative to unsigned BUG#3223 */
     test_derived();	    /* derived table with parameter BUG#3020 */
     test_xjoin();	    /* complex join test */
+    test_bug1664();
 
     end_time= time((time_t *)0);
     total_time+= difftime(end_time, start_time);