Skip to content

Commit

Permalink
MDEV-11146 SP variables of the SET data type erroneously allow values…
Browse files Browse the repository at this point in the history 
… with comma

There was a duplicate code to create TYPELIB from List<String>:
- In typelib() and mysql_prepare_create_table(), which was used to initialize
  table fields.
- create_typelib() and sp_prepare_create_field(), which was used to initialize
  SP variables.
create_typelib() was incomplete and didn't check for wrong SET values.

Fix:
- Moving the code from create_typelib() and mysql_prepare_create_field()
  to news methods Column_definition::create_interval_from_interval_list()
  and Column_definition::prepare_interval_field().
- Moving the code from calculate_interval_lengths() in sql_table.cc
  to a new method Column_definition::calculate_interval_lengths(), as it's now
  needed only in Column_definition::create_interval_from_interval_list()
- Reusing the new method Column_definition::prepare_interval_field() in both
  mysql_prepare_create_table() and sp_prepare_create_field(), instead of the
  old duplicate code pieces
- Removing global functions typelib() and create_typelib()

This patch also fixes:
MDEV-11155 Bad error message when creating a SET column with comma and non-ASCII characters
The problem was that ErrCongString() was called with a wrong "charset" parameter.
  • Loading branch information
Alexander Barkov committed Dec 6, 2016
1 parent e4ec850 commit 3c4de09
Show file tree
Hide file tree
Showing 11 changed files with 259 additions and 208 deletions.
12 changes: 12 additions & 0 deletions mysql-test/r/ctype_utf8.result
View file
Expand Up @@ -11142,3 +11142,15 @@ SET STORAGE_ENGINE=Default;
#
# End of 10.2 tests
#
#
# Start of 10.3 tests
#
#
# MDEV-11155 Bad error message when creating a SET column with comma and non-ASCII characters
#
SET NAMES utf8;
CREATE TABLE t1 (a SET('a,bü'));
ERROR 22007: Illegal set 'a,bü' value found during parsing
#
# End of 10.3 tests
#
11 changes: 11 additions & 0 deletions mysql-test/r/sp-bugs.result
View file
Expand Up @@ -281,3 +281,14 @@ end|
start transaction;
call sp();
drop procedure sp;
#
# MDEV-11146 SP variables of the SET data type erroneously allow values with comma
#
CREATE PROCEDURE p1()
BEGIN
DECLARE a SET('a','b','c','a,b');
SET a='a,b';
SELECT a, a+0;
END;
$$
ERROR 22007: Illegal set 'a,b' value found during parsing
17 changes: 17 additions & 0 deletions mysql-test/t/ctype_utf8.test
View file
Expand Up @@ -2074,3 +2074,20 @@ let $coll_pad='utf8_bin';
--echo #
--echo # End of 10.2 tests
--echo #

--echo #
--echo # Start of 10.3 tests
--echo #


--echo #
--echo # MDEV-11155 Bad error message when creating a SET column with comma and non-ASCII characters
--echo #

SET NAMES utf8;
--error ER_ILLEGAL_VALUE_FOR_TYPE
CREATE TABLE t1 (a SET('a,bü'));

--echo #
--echo # End of 10.3 tests
--echo #
14 changes: 14 additions & 0 deletions mysql-test/t/sp-bugs.test
View file
Expand Up @@ -307,3 +307,17 @@ start transaction;
call sp();
drop procedure sp;

--echo #
--echo # MDEV-11146 SP variables of the SET data type erroneously allow values with comma
--echo #

DELIMITER $$;
--error ER_ILLEGAL_VALUE_FOR_TYPE
CREATE PROCEDURE p1()
BEGIN
DECLARE a SET('a','b','c','a,b');
SET a='a,b';
SELECT a, a+0;
END;
$$
DELIMITER ;$$
83 changes: 83 additions & 0 deletions sql/field.cc
View file
Expand Up @@ -9724,6 +9724,86 @@ void Field_bit_as_char::sql_type(String &res) const
Handling of field and Create_field
*****************************************************************************/

bool Column_definition::create_interval_from_interval_list(MEM_ROOT *mem_root,
bool reuse_interval_list_values)
{
DBUG_ENTER("Column_definition::create_interval_from_interval_list");
DBUG_ASSERT(!interval);
if (!(interval= (TYPELIB*) alloc_root(mem_root, sizeof(TYPELIB))))
DBUG_RETURN(true); // EOM

List_iterator<String> it(interval_list);
StringBuffer<64> conv;
char comma_buf[5]; /* 5 bytes for 'filename' charset */
DBUG_ASSERT(sizeof(comma_buf) >= charset->mbmaxlen);
int comma_length= charset->cset->wc_mb(charset, ',',
(uchar*) comma_buf,
(uchar*) comma_buf +
sizeof(comma_buf));
DBUG_ASSERT(comma_length >= 0 && comma_length <= (int) sizeof(comma_buf));

if (!multi_alloc_root(mem_root,
&interval->type_names,
sizeof(char*) * (interval_list.elements + 1),
&interval->type_lengths,
sizeof(uint) * (interval_list.elements + 1),
NullS))
goto err; // EOM

interval->name= "";
interval->count= interval_list.elements;

for (uint i= 0; i < interval->count; i++)
{
uint32 dummy;
String *tmp= it++;
LEX_CSTRING value;
if (String::needs_conversion(tmp->length(), tmp->charset(),
charset, &dummy))
{
uint cnv_errs;
conv.copy(tmp->ptr(), tmp->length(), tmp->charset(), charset, &cnv_errs);
value.str= strmake_root(mem_root, conv.ptr(), conv.length());
value.length= conv.length();
}
else
{
value.str= reuse_interval_list_values ? tmp->ptr() :
strmake_root(mem_root,
tmp->ptr(),
tmp->length());
value.length= tmp->length();
}
if (!value.str)
goto err; // EOM

// Strip trailing spaces.
value.length= charset->cset->lengthsp(charset, value.str, value.length);
((char*) value.str)[value.length]= '\0';

if (sql_type == MYSQL_TYPE_SET)
{
if (charset->coll->instr(charset, value.str, value.length,
comma_buf, comma_length, NULL, 0))
{
ErrConvString err(tmp);
my_error(ER_ILLEGAL_VALUE_FOR_TYPE, MYF(0), "set", err.ptr());
goto err;
}
}
interval->type_names[i]= value.str;
interval->type_lengths[i]= value.length;
}
interval->type_names[interval->count]= 0; // End marker
interval->type_lengths[interval->count]= 0;
interval_list.empty(); // Don't need interval_list anymore
DBUG_RETURN(false);
err:
interval= NULL; // Avoid having both non-empty interval_list and interval
DBUG_RETURN(true);
}


/**
Convert create_field::length from number of characters to number of bytes.
*/
Expand Down Expand Up @@ -10540,6 +10620,9 @@ Column_definition::Column_definition(THD *thd, Field *old_field,
interval= ((Field_enum*) old_field)->typelib;
else
interval=0;

interval_list.empty(); // prepare_interval_field() needs this

char_length= length;

/*
Expand Down
96 changes: 96 additions & 0 deletions sql/field.h
View file
Expand Up @@ -3700,6 +3700,44 @@ Field *make_field(TABLE_SHARE *share, MEM_ROOT *mem_root,
*/
class Column_definition: public Sql_alloc
{
/**
Create "interval" from "interval_list".
@param mem_root - memory root to create the TYPELIB
instance and its values on
@param reuse_interval_list_values - determines if TYPELIB can reuse strings
from interval_list, or should always
allocate a copy on mem_root, even if
character set conversion is not needed
@retval false on success
@retval true on error (bad values, or EOM)
*/
bool create_interval_from_interval_list(MEM_ROOT *mem_root,
bool reuse_interval_list_values);

/*
Calculate TYPELIB (set or enum) max and total lengths
@param cs charset+collation pair of the interval
@param max_length length of the longest item
@param tot_length sum of the item lengths
After this method call:
- ENUM uses max_length
- SET uses tot_length.
*/
void calculate_interval_lengths(uint32 *max_length, uint32 *tot_length)
{
const char **pos;
uint *len;
*max_length= *tot_length= 0;
for (pos= interval->type_names, len= interval->type_lengths;
*pos ; pos++, len++)
{
size_t length= charset->cset->numchars(charset, *pos, *pos + *len);
*tot_length+= length;
set_if_bigger(*max_length, (uint32)length);
}
}
public:
const char *field_name;
LEX_STRING comment; // Comment for field
Expand Down Expand Up @@ -3749,7 +3787,65 @@ class Column_definition: public Sql_alloc

Column_definition(THD *thd, Field *field, Field *orig_field);
void create_length_to_internal_length(void);
/**
Prepare a SET/ENUM field.
Create "interval" from "interval_list" if needed, and adjust "length".
@param mem_root - Memory root to allocate TYPELIB and
its values on
@param reuse_interval_list_values - determines if TYPELIB can reuse value
buffers from interval_list, or should
always allocate a copy on mem_root,
even if character set conversion
is not needed
*/
bool prepare_interval_field(MEM_ROOT *mem_root,
bool reuse_interval_list_values)
{
DBUG_ENTER("Column_definition::prepare_interval_field");
DBUG_ASSERT(sql_type == MYSQL_TYPE_ENUM || sql_type == MYSQL_TYPE_SET);
/*
Interval values are either in "interval" or in "interval_list",
but not in both at the same time, and are not empty at the same time.
- Values are in "interval_list" when we're coming from the parser
in CREATE TABLE or in CREATE {FUNCTION|PROCEDURE}.
- Values are in "interval" when we're in ALTER TABLE.
In a corner case with an empty set like SET(''):
- after the parser we have interval_list.elements==1
- in ALTER TABLE we have a non-NULL interval with interval->count==1,
with interval->type_names[0]=="" and interval->type_lengths[0]==0.
So the assert is still valid for this corner case.
ENUM and SET with no values at all (e.g. ENUM(), SET()) are not possible,
as the parser requires at least one element, so for a ENUM or SET field it
should never happen that both internal_list.elements and interval are 0.
*/
DBUG_ASSERT((interval == NULL) == (interval_list.elements > 0));

/*
Create typelib from interval_list, and if necessary
convert strings from client character set to the
column character set.
*/
if (interval_list.elements &&
create_interval_from_interval_list(mem_root,
reuse_interval_list_values))
DBUG_RETURN(true);

uint32 field_length, dummy;
if (sql_type == MYSQL_TYPE_SET)
{
calculate_interval_lengths(&dummy, &field_length);
length= field_length + (interval->count - 1);
}
else /* MYSQL_TYPE_ENUM */
{
calculate_interval_lengths(&field_length, &dummy);
length= field_length;
}
set_if_smaller(length, MAX_FIELD_WIDTH - 1);
DBUG_RETURN(false);
}
bool check(THD *thd);

bool stored_in_db() const { return !vcol_info || vcol_info->stored_in_db; }
Expand Down
59 changes: 2 additions & 57 deletions sql/sp_head.cc
View file
Expand Up @@ -747,58 +747,6 @@ sp_head::set_stmt_end(THD *thd)
}


static TYPELIB *
create_typelib(MEM_ROOT *mem_root, Column_definition *field_def, List<String> *src)
{
TYPELIB *result= NULL;
CHARSET_INFO *cs= field_def->charset;
DBUG_ENTER("create_typelib");

if (src->elements)
{
result= (TYPELIB*) alloc_root(mem_root, sizeof(TYPELIB));
result->count= src->elements;
result->name= "";
if (!(result->type_names=(const char **)
alloc_root(mem_root,(sizeof(char *)+sizeof(int))*(result->count+1))))
DBUG_RETURN(0);
result->type_lengths= (uint*)(result->type_names + result->count+1);
List_iterator<String> it(*src);
String conv;
for (uint i=0; i < result->count; i++)
{
uint32 dummy;
uint length;
String *tmp= it++;

if (String::needs_conversion(tmp->length(), tmp->charset(),
cs, &dummy))
{
uint cnv_errs;
conv.copy(tmp->ptr(), tmp->length(), tmp->charset(), cs, &cnv_errs);

length= conv.length();
result->type_names[i]= (char*) strmake_root(mem_root, conv.ptr(),
length);
}
else
{
length= tmp->length();
result->type_names[i]= strmake_root(mem_root, tmp->ptr(), length);
}

// Strip trailing spaces.
length= cs->cset->lengthsp(cs, result->type_names[i], length);
result->type_lengths[i]= length;
((uchar *)result->type_names[i])[length]= '\0';
}
result->type_names[result->count]= 0;
result->type_lengths[result->count]= 0;
}
DBUG_RETURN(result);
}


sp_head::~sp_head()
{
LEX *lex;
Expand Down Expand Up @@ -2362,11 +2310,8 @@ sp_head::fill_field_definition(THD *thd, LEX *lex,
if (field_def->check(thd))
return TRUE;

if (field_def->interval_list.elements)
field_def->interval= create_typelib(mem_root, field_def,
&field_def->interval_list);

sp_prepare_create_field(thd, field_def);
if (sp_prepare_create_field(thd, mem_root, field_def))
return true;

if (prepare_create_field(field_def, &unused1, HA_CAN_GEOMETRY))
{
Expand Down

0 comments on commit 3c4de09

Please sign in to comment.