diff options
| -rw-r--r-- | sql/item.cc | 13 | ||||
| -rw-r--r-- | sql/item.h | 39 | ||||
| -rw-r--r-- | tests/client_test.c | 90 |
3 files changed, 136 insertions, 6 deletions
diff --git a/sql/item.cc b/sql/item.cc index f3a13411fe3..7db1a448e55 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -753,9 +753,10 @@ void Item_param::reset() str_value.free(); else str_value.length(0); + str_value_ptr.length(0); /* - We must prevent all charset conversions unless data of str_value - has been written to the binary log. + We must prevent all charset conversions untill data has been written + to the binary log. */ str_value.set_charset(&my_charset_bin); state= NO_VALUE; @@ -866,7 +867,7 @@ String *Item_param::val_str(String* str) switch (state) { case STRING_VALUE: case LONG_DATA_VALUE: - return &str_value; + return &str_value_ptr; case REAL_VALUE: str->set(value.real, NOT_FIXED_DEC, &my_charset_bin); return str; @@ -980,6 +981,12 @@ bool Item_param::convert_str_value(THD *thd) } max_length= str_value.length(); decimals= 0; + /* + str_value_ptr is returned from val_str(). It must be not alloced + to prevent it's modification by val_str() invoker. + */ + str_value_ptr.set(str_value.ptr(), str_value.length(), + str_value.charset()); } return rc; } diff --git a/sql/item.h b/sql/item.h index ccf8e8685d0..885a34dce81 100644 --- a/sql/item.h +++ b/sql/item.h @@ -160,6 +160,31 @@ public: /* valXXX methods must return NULL or 0 or 0.0 if null_value is set. */ virtual double val()=0; virtual longlong val_int()=0; + /* + Return string representation of this item object. + + The argument to val_str() is an allocated buffer this or any + nested Item object can use to store return value of this method. + This buffer should only be used if the item itself doesn't have an + own String buffer. In case when the item maintains it's own string + buffer, it's preferrable to return it instead to minimize number of + mallocs/memcpys. + The caller of this method can modify returned string, but only in + case when it was allocated on heap, (is_alloced() is true). This + allows the caller to efficiently use a buffer allocated by a child + without having to allocate a buffer of it's own. The buffer, given + to val_str() as agrument, belongs to the caller and is later used + by the caller at it's own choosing. + A few implications from the above: + - unless you return a string object which only points to your buffer + but doesn't manages it you should be ready that it will be + modified. + - even for not allocated strings (is_alloced() == false) the caller + can change charset (see Item_func_{typecast/binary}. XXX: is this + a bug? + - still you should try to minimize data copying and return internal + object whenever possible. + */ virtual String *val_str(String*)=0; virtual Field *get_tmp_table_field() { return 0; } virtual Field *tmp_table_field(TABLE *t_arg) { return 0; } @@ -390,6 +415,9 @@ public: void print(String *str) { str->append("NULL", 4); } }; + +/* Item represents one placeholder ('?') of prepared statement */ + class Item_param :public Item { public: @@ -399,6 +427,17 @@ public: STRING_VALUE, TIME_VALUE, LONG_DATA_VALUE } state; + /* + A buffer for string and long data values. Historically all allocated + values returned from val_str() were treated as eligible to + modification. I. e. in some cases Item_func_concat can append it's + second argument to return value of the first one. Because of that we + can't return the original buffer holding string data from val_str(), + and have to have one buffer for data and another just pointing to + the data. This is the latter one and it's returned from val_str(). + Can not be declared inside the union as it's not a POD type. + */ + String str_value_ptr; union { longlong integer; diff --git a/tests/client_test.c b/tests/client_test.c index 725d794c4a5..1f80ecc9481 100644 --- a/tests/client_test.c +++ b/tests/client_test.c @@ -9670,12 +9670,12 @@ static void test_union_param() /* bind parameters */ bind[0].buffer_type= FIELD_TYPE_STRING; - bind[0].buffer= &my_val; + bind[0].buffer= (char*) &my_val; bind[0].buffer_length= 4; bind[0].length= &my_length; bind[0].is_null= (char*)&my_null; bind[1].buffer_type= FIELD_TYPE_STRING; - bind[1].buffer= &my_val; + bind[1].buffer= (char*) &my_val; bind[1].buffer_length= 4; bind[1].length= &my_length; bind[1].is_null= (char*)&my_null; @@ -9872,7 +9872,90 @@ static void test_ps_i18n() mysql_stmt_close(stmt); stmt_text= "DROP TABLE t1"; - mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + myquery(rc); + stmt_text= "SET NAMES DEFAULT"; + rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + myquery(rc); +} + + +static void test_bug3796() +{ + MYSQL_STMT *stmt; + MYSQL_BIND bind[1]; + const char *concat_arg0= "concat_with_"; + const int OUT_BUFF_SIZE= 30; + char out_buff[OUT_BUFF_SIZE]; + char canonical_buff[OUT_BUFF_SIZE]; + ulong out_length; + const char *stmt_text; + int rc; + + myheader("test_bug3796"); + + /* Create and fill test table */ + stmt_text= "DROP TABLE IF EXISTS t1"; + rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + myquery(rc); + + stmt_text= "CREATE TABLE t1 (a INT, b VARCHAR(30))"; + rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + myquery(rc); + + stmt_text= "INSERT INTO t1 VALUES(1,'ONE'), (2,'TWO')"; + rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + myquery(rc); + + /* Create statement handle and prepare it with select */ + stmt = mysql_stmt_init(mysql); + stmt_text= "SELECT concat(?, b) FROM t1"; + + rc= mysql_stmt_prepare(stmt, stmt_text, strlen(stmt_text)); + check_execute(stmt, rc); + + /* Bind input buffers */ + bzero(bind, sizeof(bind)); + + bind[0].buffer_type= MYSQL_TYPE_STRING; + bind[0].buffer= (char*) concat_arg0; + bind[0].buffer_length= strlen(concat_arg0); + + mysql_stmt_bind_param(stmt, bind); + + /* Execute the select statement */ + rc= mysql_stmt_execute(stmt); + check_execute(stmt, rc); + + bind[0].buffer= (char*) out_buff; + bind[0].buffer_length= OUT_BUFF_SIZE; + bind[0].length= &out_length; + + mysql_stmt_bind_result(stmt, bind); + + rc= mysql_stmt_fetch(stmt); + printf("Concat result: '%s'\n", out_buff); + check_execute(stmt, rc); + strcpy(canonical_buff, concat_arg0); + strcat(canonical_buff, "ONE"); + assert(strlen(canonical_buff) == out_length && + strncmp(out_buff, canonical_buff, out_length) == 0); + + rc= mysql_stmt_fetch(stmt); + check_execute(stmt, rc); + strcpy(canonical_buff + strlen(concat_arg0), "TWO"); + assert(strlen(canonical_buff) == out_length && + strncmp(out_buff, canonical_buff, out_length) == 0); + printf("Concat result: '%s'\n", out_buff); + + rc= mysql_stmt_fetch(stmt); + assert(rc == MYSQL_NO_DATA); + + mysql_stmt_close(stmt); + + stmt_text= "DROP TABLE IF EXISTS t1"; + rc= mysql_real_query(mysql, stmt_text, strlen(stmt_text)); + myquery(rc); } /* @@ -10164,6 +10247,7 @@ int main(int argc, char **argv) test_order_param(); /* ORDER BY with parameters in select list (Bug #3686 */ test_ps_i18n(); /* test for i18n support in binary protocol */ + test_bug3796(); /* test for select concat(?, <string>) */ end_time= time((time_t *)0); total_time+= difftime(end_time, start_time); |