1
0
mirror of https://github.com/MariaDB/server.git synced 2025-08-09 22:24:09 +03:00

MDEV-9407 Illegal mix of collation when using GROUP_CONCAT in a VIEW

MDEV-9408 CREATE TABLE SELECT MAX(int_column) creates different columns for table vs view

There were three almost identical pieces of the code:
- Field *Item_func::tmp_table_field();
- Field *Item_sum::create_tmp_field();
- Field *create_tmp_field_from_item();
with a difference in very small details (hence the bugs):
Only Item_func::tmp_table_field() was correct, the other two were not.
Removing the two incorrect pieces of the redundant code.
Joining these three functions/methods into a single virtual method
Item::create_tmp_field().
Additionally, moving Item::make_string_field() and
Item::tmp_table_field_from_field_type() from the public into the
protected section of the class declaration, as they are now not
needed outside of Item.
This commit is contained in:
Alexander Barkov
2016-01-16 18:45:26 +04:00
parent 98b6026036
commit 7b50447aa6
16 changed files with 222 additions and 182 deletions

View File

@@ -13728,3 +13728,23 @@ SET NAMES utf8;
#
# End of MariaDB-10.1 tests
#
#
# Start of MariaDB-10.2 tests
#
#
# MDEV-9407 Illegal mix of collation when using GROUP_CONCAT in a VIEW
#
SET NAMES utf8;
CREATE TABLE t1 (col1 VARCHAR(12) CHARACTER SET utf8 COLLATE utf8_unicode_ci);
INSERT INTO t1 VALUES ('a'),('b');
CREATE VIEW v1 AS SELECT group_concat('f') AS col1;
SELECT col1 FROM v1 UNION SELECT col1 FROM t1;
col1
f
a
b
DROP VIEW v1;
DROP TABLE t1;
#
# End of MariaDB-10.2 tests
#

View File

@@ -5878,3 +5878,39 @@ DROP TABLE t1;
#
# End of 10.1 tests
#
#
# Start of 10.2 tests
#
#
# MDEV-9408 CREATE TABLE SELECT MAX(int_column) creates different columns for table vs view
#
CREATE TABLE t1 (
id int(11) NOT NULL PRIMARY KEY,
country varchar(32),
code int(11) default NULL
);
INSERT INTO t1 VALUES (1,'ITALY',100),(2,'ITALY',200),(3,'FRANCE',100), (4,'ITALY',100);
CREATE VIEW v1 AS SELECT * FROM t1;
CREATE TABLE t2 AS
SELECT code, COUNT(DISTINCT country), MAX(id) FROM t1 GROUP BY code ORDER BY MAX(id);
SHOW CREATE TABLE t2;
Table Create Table
t2 CREATE TABLE `t2` (
`code` int(11) DEFAULT NULL,
`COUNT(DISTINCT country)` bigint(21) NOT NULL,
`MAX(id)` int(11)
) ENGINE=MyISAM DEFAULT CHARSET=latin1
CREATE TABLE t3 AS
SELECT code, COUNT(DISTINCT country), MAX(id) FROM v1 GROUP BY code ORDER BY MAX(id);
SHOW CREATE TABLE t3;
Table Create Table
t3 CREATE TABLE `t3` (
`code` int(11) DEFAULT NULL,
`COUNT(DISTINCT country)` bigint(21) NOT NULL,
`MAX(id)` int(11) DEFAULT NULL
) ENGINE=MyISAM DEFAULT CHARSET=latin1
DROP VIEW v1;
DROP TABLE t1,t2,t3;
#
# End of 10.2 tests
#

View File

@@ -646,3 +646,24 @@ SET NAMES utf8;
--echo #
--echo # End of MariaDB-10.1 tests
--echo #
--echo #
--echo # Start of MariaDB-10.2 tests
--echo #
--echo #
--echo # MDEV-9407 Illegal mix of collation when using GROUP_CONCAT in a VIEW
--echo #
SET NAMES utf8;
CREATE TABLE t1 (col1 VARCHAR(12) CHARACTER SET utf8 COLLATE utf8_unicode_ci);
INSERT INTO t1 VALUES ('a'),('b');
CREATE VIEW v1 AS SELECT group_concat('f') AS col1;
SELECT col1 FROM v1 UNION SELECT col1 FROM t1;
DROP VIEW v1;
DROP TABLE t1;
--echo #
--echo # End of MariaDB-10.2 tests
--echo #

View File

@@ -5741,3 +5741,31 @@ DROP TABLE t1;
--echo #
--echo # End of 10.1 tests
--echo #
--echo #
--echo # Start of 10.2 tests
--echo #
--echo #
--echo # MDEV-9408 CREATE TABLE SELECT MAX(int_column) creates different columns for table vs view
--echo #
CREATE TABLE t1 (
id int(11) NOT NULL PRIMARY KEY,
country varchar(32),
code int(11) default NULL
);
INSERT INTO t1 VALUES (1,'ITALY',100),(2,'ITALY',200),(3,'FRANCE',100), (4,'ITALY',100);
CREATE VIEW v1 AS SELECT * FROM t1;
CREATE TABLE t2 AS
SELECT code, COUNT(DISTINCT country), MAX(id) FROM t1 GROUP BY code ORDER BY MAX(id);
SHOW CREATE TABLE t2;
CREATE TABLE t3 AS
SELECT code, COUNT(DISTINCT country), MAX(id) FROM v1 GROUP BY code ORDER BY MAX(id);
SHOW CREATE TABLE t3;
DROP VIEW v1;
DROP TABLE t1,t2,t3;
--echo #
--echo # End of 10.2 tests
--echo #

View File

@@ -656,6 +656,14 @@ protected:
SEL_TREE *get_mm_tree_for_const(RANGE_OPT_PARAM *param);
virtual Field *make_string_field(TABLE *table);
Field *tmp_table_field_from_field_type(TABLE *table,
bool fixed_length,
bool set_blob_packlength);
Field *create_tmp_field(bool group, TABLE *table,
uint convert_blob_length,
uint convert_int_length);
public:
/*
Cache val_str() into the own buffer, e.g. to evaluate constant
@@ -722,7 +730,6 @@ public:
void init_make_field(Send_field *tmp_field,enum enum_field_types type);
virtual void cleanup();
virtual void make_field(THD *thd, Send_field *field);
virtual Field *make_string_field(TABLE *table);
virtual bool fix_fields(THD *, Item **);
/*
Fix after some tables has been pulled out. Basically re-calculate all
@@ -1046,8 +1053,6 @@ public:
int save_str_value_in_field(Field *field, String *result);
virtual Field *get_tmp_table_field() { return 0; }
/* This is also used to create fields in CREATE ... SELECT: */
virtual Field *tmp_table_field(TABLE *t_arg) { return 0; }
virtual Field *create_field_for_create_select(THD *thd, TABLE *table);
virtual Field *create_field_for_schema(THD *thd, TABLE *table);
virtual const char *full_name() const { return name ? name : "???"; }
@@ -1655,9 +1660,18 @@ public:
// used in row subselects to get value of elements
virtual void bring_value() {}
Field *tmp_table_field_from_field_type(TABLE *table,
bool fixed_length,
bool set_blob_packlength);
virtual Field *create_tmp_field(bool group, TABLE *table,
uint convert_blob_length)
{
/*
Values with MY_INT32_NUM_DECIMAL_DIGITS digits may or may not fit into
Field_long : make them Field_longlong.
*/
return create_tmp_field(false, table,
convert_blob_length,
MY_INT32_NUM_DECIMAL_DIGITS - 2);
}
virtual Item_field *field_for_view_update() { return 0; }
virtual Item *neg_transformer(THD *thd) { return NULL; }
@@ -2252,7 +2266,6 @@ public:
{}
~Item_result_field() {} /* Required with gcc 2.95 */
Field *get_tmp_table_field() { return result_field; }
Field *tmp_table_field(TABLE *t_arg) { return result_field; }
/*
This implementation of used_tables() used by Item_avg_field and
Item_variance_field which work when only temporary table left, so theu
@@ -3440,8 +3453,6 @@ public:
{ return val_real_from_date(); }
my_decimal *val_decimal(my_decimal *decimal_value)
{ return val_decimal_from_date(decimal_value); }
Field *tmp_table_field(TABLE *table)
{ return tmp_table_field_from_field_type(table, false, false); }
int save_in_field(Field *field, bool no_conversions)
{ return save_date_in_field(field); }
};
@@ -3947,7 +3958,6 @@ public:
enum_field_types field_type() const { return (*ref)->field_type(); }
Field *get_tmp_table_field()
{ return result_field ? result_field : (*ref)->get_tmp_table_field(); }
Field *tmp_table_field(TABLE *t_arg) { return 0; }
Item *get_tmp_table_item(THD *thd);
table_map used_tables() const;
void update_used_tables();

View File

@@ -2300,11 +2300,6 @@ uint Item_func_case_abbreviation2::decimal_precision2(Item **args) const
}
Field *Item_func_ifnull::tmp_table_field(TABLE *table)
{
return tmp_table_field_from_field_type(table, false, false);
}
double
Item_func_ifnull::real_op()
{

View File

@@ -940,7 +940,9 @@ public:
maybe_null= args[1]->maybe_null;
}
const char *func_name() const { return "ifnull"; }
Field *tmp_table_field(TABLE *table);
Field *create_field_for_create_select(THD *thd, TABLE *table)
{ return tmp_table_field_from_field_type(table, false, false); }
table_map not_null_tables() const { return 0; }
uint decimal_precision() const
{

View File

@@ -509,43 +509,6 @@ bool Item_func::eq(const Item *item, bool binary_cmp) const
}
Field *Item_func::tmp_table_field(TABLE *table)
{
Field *field= NULL;
MEM_ROOT *mem_root= table->in_use->mem_root;
switch (result_type()) {
case INT_RESULT:
if (max_char_length() > MY_INT32_NUM_DECIMAL_DIGITS)
field= new (mem_root)
Field_longlong(max_char_length(), maybe_null, name,
unsigned_flag);
else
field= new (mem_root)
Field_long(max_char_length(), maybe_null, name,
unsigned_flag);
break;
case REAL_RESULT:
field= new (mem_root)
Field_double(max_char_length(), maybe_null, name, decimals);
break;
case STRING_RESULT:
return make_string_field(table);
case DECIMAL_RESULT:
field= Field_new_decimal::create_from_item(mem_root, this);
break;
case ROW_RESULT:
case TIME_RESULT:
// This case should never be chosen
DBUG_ASSERT(0);
field= 0;
break;
}
if (field)
field->init(table);
return field;
}
/*
bool Item_func::is_expensive_processor(uchar *arg)
{
@@ -2908,10 +2871,10 @@ void Item_func_min_max::fix_length_and_dec()
collation.set_numeric();
fix_char_length(float_length(decimals));
/*
Set type to DOUBLE, as Item_func::tmp_table_field() does not
Set type to DOUBLE, as Item_func::create_tmp_field() does not
distinguish between DOUBLE and FLOAT and always creates Field_double.
Perhaps we should eventually change this to use agg_field_type() here,
and fix Item_func::tmp_table_field() to create Field_float when possible.
and fix Item_func::create_tmp_field() to create Field_float when possible.
*/
set_handler_by_field_type(MYSQL_TYPE_DOUBLE);
break;
@@ -6801,16 +6764,6 @@ longlong Item_func_found_rows::val_int()
}
Field *
Item_func_sp::tmp_table_field(TABLE *t_arg)
{
DBUG_ENTER("Item_func_sp::tmp_table_field");
DBUG_ASSERT(sp_result_field);
DBUG_RETURN(sp_result_field);
}
/**
@brief Checks if requested access to function can be granted to user.
If function isn't found yet, it searches function first.

View File

@@ -171,12 +171,11 @@ public:
}
void signal_divide_by_null();
friend class udf_handler;
Field *tmp_table_field() { return result_field; }
Field *tmp_table_field(TABLE *t_arg);
Field *create_field_for_create_select(THD *thd, TABLE *table)
{
DBUG_ASSERT(thd == table->in_use);
return result_type() != STRING_RESULT ?
tmp_table_field(table) :
create_tmp_field(false, table, 0, MY_INT32_NUM_DECIMAL_DIGITS) :
tmp_table_field_from_field_type(table, false, false);
}
Item *get_tmp_table_item(THD *thd);
@@ -1767,7 +1766,7 @@ public:
Field *create_field_for_create_select(THD *thd, TABLE *table)
{
return result_type() != STRING_RESULT ?
tmp_table_field(table) :
create_tmp_field(false, table, 0, MY_INT32_NUM_DECIMAL_DIGITS) :
tmp_table_field_from_field_type(table, false, true);
}
table_map used_tables() const
@@ -2136,8 +2135,12 @@ public:
enum enum_field_types field_type() const;
Field *tmp_table_field(TABLE *t_arg);
Field *create_field_for_create_select(THD *thd, TABLE *table)
{
return result_type() != STRING_RESULT ?
sp_result_field :
tmp_table_field_from_field_type(table, false, false);
}
void make_field(THD *thd, Send_field *tmp_field);
Item_result result_type() const;

View File

@@ -41,7 +41,7 @@
#include "opt_range.h"
Field *Item_geometry_func::tmp_table_field(TABLE *t_arg)
Field *Item_geometry_func::create_field_for_create_select(THD *thd, TABLE *t_arg)
{
Field *result;
if ((result= new Field_geom(max_length, maybe_null, name, t_arg->s,

View File

@@ -40,7 +40,7 @@ public:
Item_geometry_func(THD *thd, List<Item> &list): Item_str_func(thd, list) {}
void fix_length_and_dec();
enum_field_types field_type() const { return MYSQL_TYPE_GEOMETRY; }
Field *tmp_table_field(TABLE *t_arg);
Field *create_field_for_create_select(THD *thd, TABLE *table);
};
class Item_func_geometry_from_text: public Item_geometry_func

View File

@@ -496,44 +496,6 @@ Item *Item_sum::get_tmp_table_item(THD *thd)
}
Field *Item_sum::create_tmp_field(bool group, TABLE *table,
uint convert_blob_length)
{
Field *UNINIT_VAR(field);
MEM_ROOT *mem_root= table->in_use->mem_root;
switch (result_type()) {
case REAL_RESULT:
field= new (mem_root)
Field_double(max_length, maybe_null, name, decimals, TRUE);
break;
case INT_RESULT:
field= new (mem_root)
Field_longlong(max_length, maybe_null, name, unsigned_flag);
break;
case STRING_RESULT:
if (max_length/collation.collation->mbmaxlen <= 255 ||
convert_blob_length > Field_varstring::MAX_SIZE ||
!convert_blob_length)
return make_string_field(table);
field= new (mem_root) Field_varstring(convert_blob_length, maybe_null,
name, table->s, collation.collation);
break;
case DECIMAL_RESULT:
field= Field_new_decimal::create_from_item(mem_root, this);
break;
case ROW_RESULT:
case TIME_RESULT:
// This case should never be choosen
DBUG_ASSERT(0);
return 0;
}
if (field)
field->init(table);
return field;
}
void Item_sum::update_used_tables ()
{
if (!Item_sum::const_item())

View File

@@ -481,8 +481,12 @@ public:
}
virtual void make_unique() { force_copy_fields= TRUE; }
Item *get_tmp_table_item(THD *thd);
virtual Field *create_tmp_field(bool group, TABLE *table,
uint convert_blob_length);
Field *create_tmp_field(bool group, TABLE *table,
uint convert_blob_length)
{
return Item::create_tmp_field(group, table, convert_blob_length,
MY_INT32_NUM_DECIMAL_DIGITS);
}
virtual bool collect_outer_ref_processor(uchar *param);
bool init_sum_func_check(THD *thd);
bool check_sum_func(THD *thd, Item **ref);
@@ -1093,7 +1097,6 @@ public:
}
table_map used_tables() const { return (table_map) 1L; }
Field *get_tmp_table_field() { DBUG_ASSERT(0); return NULL; }
Field *tmp_table_field(TABLE *) { DBUG_ASSERT(0); return NULL; }
void set_result_field(Field *) { DBUG_ASSERT(0); }
void save_in_result_field(bool no_conversions) { DBUG_ASSERT(0); }
};
@@ -1483,6 +1486,8 @@ class Item_func_group_concat : public Item_sum
friend int dump_leaf_key(void* key_arg,
element_count count __attribute__((unused)),
void* item_arg);
protected:
virtual Field *make_string_field(TABLE *table);
public:
Item_func_group_concat(THD *thd, Name_resolution_context *context_arg,
@@ -1497,7 +1502,6 @@ public:
const char *func_name() const { return "group_concat"; }
virtual Item_result result_type () const { return STRING_RESULT; }
virtual Item_result cmp_type () const { return STRING_RESULT; }
virtual Field *make_string_field(TABLE *table);
enum_field_types field_type() const
{
if (too_big_for_varchar())

View File

@@ -502,7 +502,7 @@ public:
bool get_date(MYSQL_TIME *res, ulonglong fuzzy_date) { DBUG_ASSERT(0); return 1; }
my_decimal *val_decimal(my_decimal *decimal_value)
{ return val_decimal_from_date(decimal_value); }
Field *tmp_table_field(TABLE *table)
Field *create_field_for_create_select(THD *thd, TABLE *table)
{ return tmp_table_field_from_field_type(table, false, false); }
int save_in_field(Field *field, bool no_conversions)
{ return save_date_in_field(field); }

View File

@@ -3892,7 +3892,7 @@ void select_insert::abort_result_set() {
Field *Item::create_field_for_create_select(THD *thd, TABLE *table)
{
Field *def_field, *tmp_field;
return create_tmp_field(thd, table, this, type(),
return ::create_tmp_field(thd, table, this, type(),
(Item ***) 0, &tmp_field, &def_field, 0, 0, 0, 0, 0);
}

View File

@@ -15681,6 +15681,73 @@ Field *create_tmp_field_from_field(THD *thd, Field *org_field,
return new_field;
}
Field *Item::create_tmp_field(bool group, TABLE *table,
uint convert_blob_length,
uint convert_int_length)
{
Field *UNINIT_VAR(new_field);
MEM_ROOT *mem_root= table->in_use->mem_root;
switch (cmp_type()) {
case REAL_RESULT:
new_field= new (mem_root)
Field_double(max_length, maybe_null, name, decimals, TRUE);
break;
case INT_RESULT:
/*
Select an integer type with the minimal fit precision.
convert_int_length is sign inclusive, don't consider the sign.
*/
if (max_char_length() > convert_int_length)
new_field= new (mem_root)
Field_longlong(max_char_length(), maybe_null, name, unsigned_flag);
else
new_field= new (mem_root)
Field_long(max_char_length(), maybe_null, name, unsigned_flag);
break;
case TIME_RESULT:
new_field= tmp_table_field_from_field_type(table, true, false);
break;
case STRING_RESULT:
DBUG_ASSERT(collation.collation);
/*
GEOMETRY fields have STRING_RESULT result type.
To preserve type they needed to be handled separately.
*/
if (field_type() == MYSQL_TYPE_GEOMETRY)
new_field= tmp_table_field_from_field_type(table, true, false);
/*
Make sure that the blob fits into a Field_varstring which has
2-byte lenght.
*/
else if (max_length / collation.collation->mbmaxlen > 255 &&
convert_blob_length <= Field_varstring::MAX_SIZE &&
convert_blob_length)
new_field= new (mem_root)
Field_varstring(convert_blob_length, maybe_null,
name, table->s, collation.collation);
else
new_field= make_string_field(table);
new_field->set_derivation(collation.derivation);
break;
case DECIMAL_RESULT:
new_field= Field_new_decimal::create_from_item(mem_root, this);
break;
case ROW_RESULT:
// This case should never be choosen
DBUG_ASSERT(0);
new_field= 0;
break;
}
if (new_field)
new_field->init(table);
return new_field;
}
/**
Create field for temporary table using type of given item.
@@ -15709,69 +15776,9 @@ static Field *create_tmp_field_from_item(THD *thd, Item *item, TABLE *table,
Item ***copy_func, bool modify_item,
uint convert_blob_length)
{
bool maybe_null= item->maybe_null;
Field *UNINIT_VAR(new_field);
MEM_ROOT *mem_root= thd->mem_root;
switch (item->result_type()) {
case REAL_RESULT:
new_field= new (mem_root)
Field_double(item->max_length, maybe_null,
item->name, item->decimals, TRUE);
break;
case INT_RESULT:
/*
Select an integer type with the minimal fit precision.
MY_INT32_NUM_DECIMAL_DIGITS is sign inclusive, don't consider the sign.
Values with MY_INT32_NUM_DECIMAL_DIGITS digits may or may not fit into
Field_long : make them Field_longlong.
*/
if (item->max_length >= (MY_INT32_NUM_DECIMAL_DIGITS - 1))
new_field=new (mem_root)
Field_longlong(item->max_length, maybe_null,
item->name, item->unsigned_flag);
else
new_field=new (mem_root)
Field_long(item->max_length, maybe_null, item->name,
item->unsigned_flag);
break;
case STRING_RESULT:
DBUG_ASSERT(item->collation.collation);
/*
DATE/TIME and GEOMETRY fields have STRING_RESULT result type.
To preserve type they needed to be handled separately.
*/
if (item->cmp_type() == TIME_RESULT ||
item->field_type() == MYSQL_TYPE_GEOMETRY)
new_field= item->tmp_table_field_from_field_type(table, true, false);
/*
Make sure that the blob fits into a Field_varstring which has
2-byte lenght.
*/
else if (item->max_length/item->collation.collation->mbmaxlen > 255 &&
convert_blob_length <= Field_varstring::MAX_SIZE &&
convert_blob_length)
new_field= new (mem_root)
Field_varstring(convert_blob_length, maybe_null,
item->name, table->s,
item->collation.collation);
else
new_field= item->make_string_field(table);
new_field->set_derivation(item->collation.derivation);
break;
case DECIMAL_RESULT:
new_field= Field_new_decimal::create_from_item(mem_root, item);
break;
case ROW_RESULT:
default:
// This case should never be choosen
DBUG_ASSERT(0);
new_field= 0;
break;
}
if (new_field)
new_field->init(table);
DBUG_ASSERT(thd == table->in_use);
new_field= item->Item::create_tmp_field(false, table, convert_blob_length);
if (copy_func && item->real_item()->is_result_field())
*((*copy_func)++) = item; // Save for copy_funcs
@@ -15866,8 +15873,7 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type,
switch (type) {
case Item::SUM_FUNC_ITEM:
{
Item_sum *item_sum=(Item_sum*) item;
result= item_sum->create_tmp_field(group, table, convert_blob_length);
result= item->create_tmp_field(group, table, convert_blob_length);
if (!result)
my_error(ER_OUT_OF_RESOURCES, MYF(ME_FATALERROR));
return result;