Commit 6ca75754 authored by unknown's avatar unknown

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".
parent 258a3d16
...@@ -605,12 +605,19 @@ Item_param::Item_param(unsigned position) : ...@@ -605,12 +605,19 @@ Item_param::Item_param(unsigned position) :
set_param_func(default_set_param_func) set_param_func(default_set_param_func)
{ {
name= (char*) "?"; 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() void Item_param::set_null()
{ {
DBUG_ENTER("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; DBUG_VOID_RETURN;
} }
...@@ -620,6 +627,7 @@ void Item_param::set_int(longlong i) ...@@ -620,6 +627,7 @@ void Item_param::set_int(longlong i)
int_value= (longlong)i; int_value= (longlong)i;
item_type= INT_ITEM; item_type= INT_ITEM;
value_is_set= 1; value_is_set= 1;
maybe_null= 0;
DBUG_PRINT("info", ("integer: %lld", int_value)); DBUG_PRINT("info", ("integer: %lld", int_value));
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
...@@ -630,6 +638,7 @@ void Item_param::set_double(double value) ...@@ -630,6 +638,7 @@ void Item_param::set_double(double value)
real_value=value; real_value=value;
item_type= REAL_ITEM; item_type= REAL_ITEM;
value_is_set= 1; value_is_set= 1;
maybe_null= 0;
DBUG_PRINT("info", ("double: %lg", real_value)); DBUG_PRINT("info", ("double: %lg", real_value));
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
...@@ -641,6 +650,7 @@ void Item_param::set_value(const char *str, uint length) ...@@ -641,6 +650,7 @@ void Item_param::set_value(const char *str, uint length)
str_value.copy(str,length,default_charset()); str_value.copy(str,length,default_charset());
item_type= STRING_ITEM; item_type= STRING_ITEM;
value_is_set= 1; value_is_set= 1;
maybe_null= 0;
DBUG_PRINT("info", ("string: %s", str_value.ptr())); DBUG_PRINT("info", ("string: %s", str_value.ptr()));
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
...@@ -665,6 +675,7 @@ void Item_param::set_time(TIME *tm, timestamp_type type) ...@@ -665,6 +675,7 @@ void Item_param::set_time(TIME *tm, timestamp_type type)
item_is_time= TRUE; item_is_time= TRUE;
item_type= STRING_ITEM; item_type= STRING_ITEM;
value_is_set= 1; value_is_set= 1;
maybe_null= 0;
} }
...@@ -673,6 +684,27 @@ void Item_param::set_longdata(const char *str, ulong length) ...@@ -673,6 +684,27 @@ void Item_param::set_longdata(const char *str, ulong length)
str_value.append(str,length); str_value.append(str,length);
long_data_supplied= 1; long_data_supplied= 1;
value_is_set= 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;
} }
......
...@@ -419,7 +419,7 @@ public: ...@@ -419,7 +419,7 @@ public:
void set_long_end(); void set_long_end();
void set_time(TIME *tm, timestamp_type type); void set_time(TIME *tm, timestamp_type type);
bool get_time(TIME *tm); bool get_time(TIME *tm);
void reset() {} void reset();
/* /*
Assign placeholder value from bind data. Assign placeholder value from bind data.
Note, that 'len' has different semantics in embedded library (as we Note, that 'len' has different semantics in embedded library (as we
......
...@@ -91,7 +91,6 @@ public: ...@@ -91,7 +91,6 @@ public:
uint last_errno; uint last_errno;
char last_error[MYSQL_ERRMSG_SIZE]; char last_error[MYSQL_ERRMSG_SIZE];
bool get_longdata_error; bool get_longdata_error;
bool long_data_used;
bool log_full_query; bool log_full_query;
#ifndef EMBEDDED_LIBRARY #ifndef EMBEDDED_LIBRARY
bool (*set_params)(Prepared_statement *st, uchar *data, uchar *data_end, 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, ...@@ -465,12 +464,11 @@ static bool insert_params_withlog(Prepared_statement *stmt, uchar *null_array,
{ {
if (is_param_null(null_array, it - begin)) 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; res= &my_null_string;
} }
else else
{ {
param->maybe_null= param->null_value= 0;
if (read_pos >= data_end) if (read_pos >= data_end)
DBUG_RETURN(1); DBUG_RETURN(1);
param->set_param_func(param, &read_pos, data_end - read_pos); 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, ...@@ -503,10 +501,9 @@ static bool insert_params(Prepared_statement *stmt, uchar *null_array,
if (!param->long_data_supplied) if (!param->long_data_supplied)
{ {
if (is_param_null(null_array, it - begin)) if (is_param_null(null_array, it - begin))
param->maybe_null= param->null_value= param->value_is_set= 1; param->set_null();
else else
{ {
param->maybe_null= param->null_value= 0;
if (read_pos >= data_end) if (read_pos >= data_end)
DBUG_RETURN(1); DBUG_RETURN(1);
param->set_param_func(param, &read_pos, data_end - read_pos); param->set_param_func(param, &read_pos, data_end - read_pos);
...@@ -562,11 +559,10 @@ static bool emb_insert_params(Prepared_statement *stmt) ...@@ -562,11 +559,10 @@ static bool emb_insert_params(Prepared_statement *stmt)
if (!param->long_data_supplied) if (!param->long_data_supplied)
{ {
if (*client_param->is_null) if (*client_param->is_null)
param->maybe_null= param->null_value= 1; param->set_null();
else else
{ {
uchar *buff= (uchar*)client_param->buffer; uchar *buff= (uchar*)client_param->buffer;
param->maybe_null= param->null_value= 0;
param->set_param_func(param, &buff, param->set_param_func(param, &buff,
client_param->length ? client_param->length ?
*client_param->length : *client_param->length :
...@@ -604,13 +600,12 @@ static bool emb_insert_params_withlog(Prepared_statement *stmt) ...@@ -604,13 +600,12 @@ static bool emb_insert_params_withlog(Prepared_statement *stmt)
{ {
if (*client_param->is_null) if (*client_param->is_null)
{ {
param->maybe_null= param->null_value= 1; param->set_null();
res= &my_null_string; res= &my_null_string;
} }
else else
{ {
uchar *buff= (uchar*)client_param->buffer; uchar *buff= (uchar*)client_param->buffer;
param->maybe_null= param->null_value= 0;
param->set_param_func(param, &buff, param->set_param_func(param, &buff,
client_param->length ? client_param->length ?
*client_param->length : *client_param->length :
...@@ -1439,6 +1434,24 @@ static void reset_stmt_for_execute(Prepared_statement *stmt) ...@@ -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. Executes previously prepared query.
If there is any parameters, then replace markers with the data supplied 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) ...@@ -1512,11 +1525,13 @@ void mysql_stmt_execute(THD *thd, char *packet, uint packet_length)
my_pthread_setprio(pthread_self(), WAIT_PRIOR); my_pthread_setprio(pthread_self(), WAIT_PRIOR);
cleanup_items(stmt->free_list); cleanup_items(stmt->free_list);
reset_stmt_params(stmt);
close_thread_tables(thd); // to close derived tables close_thread_tables(thd); // to close derived tables
thd->set_statement(&thd->stmt_backup); thd->set_statement(&thd->stmt_backup);
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
set_params_data_err: set_params_data_err:
reset_stmt_params(stmt);
thd->set_statement(&thd->stmt_backup); thd->set_statement(&thd->stmt_backup);
my_error(ER_WRONG_ARGUMENTS, MYF(0), "mysql_execute"); my_error(ER_WRONG_ARGUMENTS, MYF(0), "mysql_execute");
send_error(thd); send_error(thd);
...@@ -1552,15 +1567,12 @@ void mysql_stmt_reset(THD *thd, char *packet) ...@@ -1552,15 +1567,12 @@ void mysql_stmt_reset(THD *thd, char *packet)
stmt->get_longdata_error= 0; stmt->get_longdata_error= 0;
/* Free long data if used */ /*
if (stmt->long_data_used) Clear parameters from data which could be set by
{ mysql_stmt_send_long_data() call.
Item_param **item= stmt->param_array; */
Item_param **end= item + stmt->param_count; reset_stmt_params(stmt);
stmt->long_data_used= 0;
for (; item < end ; item++)
(**item).reset();
}
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
...@@ -1646,7 +1658,6 @@ void mysql_stmt_get_longdata(THD *thd, char *pos, ulong packet_length) ...@@ -1646,7 +1658,6 @@ void mysql_stmt_get_longdata(THD *thd, char *pos, ulong packet_length)
#else #else
param->set_longdata(thd->extra_data, thd->extra_length); param->set_longdata(thd->extra_data, thd->extra_length);
#endif #endif
stmt->long_data_used= 1;
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
} }
...@@ -1658,7 +1669,6 @@ Prepared_statement::Prepared_statement(THD *thd_arg) ...@@ -1658,7 +1669,6 @@ Prepared_statement::Prepared_statement(THD *thd_arg)
param_count(0), param_count(0),
last_errno(0), last_errno(0),
get_longdata_error(0), get_longdata_error(0),
long_data_used(0),
log_full_query(0) log_full_query(0)
{ {
*last_error= '\0'; *last_error= '\0';
......
...@@ -9101,6 +9101,152 @@ static void test_xjoin() ...@@ -9101,6 +9101,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 Read and parse arguments and MySQL options from my.cnf
*/ */
...@@ -9373,6 +9519,7 @@ int main(int argc, char **argv) ...@@ -9373,6 +9519,7 @@ int main(int argc, char **argv)
test_bind_nagative(); /* bind negative to unsigned BUG#3223 */ test_bind_nagative(); /* bind negative to unsigned BUG#3223 */
test_derived(); /* derived table with parameter BUG#3020 */ test_derived(); /* derived table with parameter BUG#3020 */
test_xjoin(); /* complex join test */ test_xjoin(); /* complex join test */
test_bug1664();
end_time= time((time_t *)0); end_time= time((time_t *)0);
total_time+= difftime(end_time, start_time); total_time+= difftime(end_time, start_time);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment