Skip to content

M
MariaDB
Commit 1a36caf0 authored by Alexander Barkov's avatar Alexander Barkov
Browse files
Options

MDEV-8729 Wrong result for SELECT..WHERE HEX(enum_column)='61' AND enum_column='a '

parent e0df1160
Hide whitespace changes
Inline Side-by-side
Showing with 377 additions and 173 deletions
mysql-test/r/type_enum.result
View file @ 1a36caf0
......@@ -2082,3 +2082,55 @@ DROP TABLE t1;
#
# End of 10.0 tests
#
#
# Start of 10.1 tests
#
#
# MDEV-8729 Wrong result for SELECT..WHERE HEX(enum_column)='61' AND enum_column='a '
#
CREATE TABLE t1 (a ENUM('a','A') CHARACTER SET latin1 COLLATE latin1_bin);
INSERT INTO t1 VALUES ('a'),('A');
SELECT * FROM t1 WHERE a='a ';
a
a
SELECT * FROM t1 WHERE HEX(a)='61';
a
a
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
a
a
# Can't propagate the equality into HEX(a), because binary collations still ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t1 ALL NULL NULL NULL NULL 2 100.00 Using where
Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` = 'a ') and (hex(`test`.`t1`.`a`) = '61'))
DROP TABLE t1;
CREATE TABLE t1 (a ENUM('a','a ') CHARACTER SET BINARY);
INSERT INTO t1 VALUES ('a'),('a ');
SELECT * FROM t1 WHERE a='a ';
a
a
SELECT * FROM t1 WHERE HEX(a)='61';
a
a
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
a
a
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
a
# Ok to propagate the equality into HEX(a), because "CHARACTER SET BINARY" does not ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t1 ALL NULL NULL NULL NULL 2 100.00 Using where
Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where (`test`.`t1`.`a` = 'a')
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE NULL NULL NULL NULL NULL NULL NULL NULL Impossible WHERE
Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where 0
DROP TABLE t1;
#
# End of 10.1 tests
#
......
mysql-test/r/type_set.result
View file @ 1a36caf0
......@@ -263,3 +263,55 @@ DROP TABLE t1;
#
# End of 10.0 tests
#
#
# Start of 10.1 tests
#
#
# MDEV-8729 Wrong result for SELECT..WHERE HEX(enum_column)='61' AND enum_column='a '
#
CREATE TABLE t1 (a SET('a','A') CHARACTER SET latin1 COLLATE latin1_bin);
INSERT INTO t1 VALUES ('a'),('A');
SELECT * FROM t1 WHERE a='a ';
a
a
SELECT * FROM t1 WHERE HEX(a)='61';
a
a
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
a
a
# Can't propagate the equality into HEX(a), because binary collations still ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t1 ALL NULL NULL NULL NULL 2 100.00 Using where
Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where ((`test`.`t1`.`a` = 'a ') and (hex(`test`.`t1`.`a`) = '61'))
DROP TABLE t1;
CREATE TABLE t1 (a SET('a','a ') CHARACTER SET BINARY);
INSERT INTO t1 VALUES ('a'),('a ');
SELECT * FROM t1 WHERE a='a ';
a
a
SELECT * FROM t1 WHERE HEX(a)='61';
a
a
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
a
a
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
a
# Ok to propagate the equality into HEX(a), because "CHARACTER SET BINARY" does not ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t1 ALL NULL NULL NULL NULL 2 100.00 Using where
Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where (`test`.`t1`.`a` = 'a')
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
id select_type table type possible_keys key key_len ref rows filtered Extra
1 SIMPLE NULL NULL NULL NULL NULL NULL NULL NULL Impossible WHERE
Warnings:
Note 1003 select `test`.`t1`.`a` AS `a` from `test`.`t1` where 0
DROP TABLE t1;
#
# End of 10.1 tests
#
mysql-test/t/type_enum.test
View file @ 1a36caf0
......@@ -357,3 +357,34 @@ DROP TABLE t1;
--echo #
--echo # End of 10.0 tests
--echo #
--echo #
--echo # Start of 10.1 tests
--echo #
--echo #
--echo # MDEV-8729 Wrong result for SELECT..WHERE HEX(enum_column)='61' AND enum_column='a '
--echo #
CREATE TABLE t1 (a ENUM('a','A') CHARACTER SET latin1 COLLATE latin1_bin);
INSERT INTO t1 VALUES ('a'),('A');
SELECT * FROM t1 WHERE a='a ';
SELECT * FROM t1 WHERE HEX(a)='61';
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
--echo # Can't propagate the equality into HEX(a), because binary collations still ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
DROP TABLE t1;
CREATE TABLE t1 (a ENUM('a','a ') CHARACTER SET BINARY);
INSERT INTO t1 VALUES ('a'),('a ');
SELECT * FROM t1 WHERE a='a ';
SELECT * FROM t1 WHERE HEX(a)='61';
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
--echo # Ok to propagate the equality into HEX(a), because "CHARACTER SET BINARY" does not ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
DROP TABLE t1;
--echo #
--echo # End of 10.1 tests
--echo #
......
mysql-test/t/type_set.test
View file @ 1a36caf0
......@@ -185,3 +185,35 @@ DROP TABLE t1;
--echo #
--echo # End of 10.0 tests
--echo #
--echo #
--echo # Start of 10.1 tests
--echo #
--echo #
--echo # MDEV-8729 Wrong result for SELECT..WHERE HEX(enum_column)='61' AND enum_column='a '
--echo #
CREATE TABLE t1 (a SET('a','A') CHARACTER SET latin1 COLLATE latin1_bin);
INSERT INTO t1 VALUES ('a'),('A');
SELECT * FROM t1 WHERE a='a ';
SELECT * FROM t1 WHERE HEX(a)='61';
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
--echo # Can't propagate the equality into HEX(a), because binary collations still ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
DROP TABLE t1;
CREATE TABLE t1 (a SET('a','a ') CHARACTER SET BINARY);
INSERT INTO t1 VALUES ('a'),('a ');
SELECT * FROM t1 WHERE a='a ';
SELECT * FROM t1 WHERE HEX(a)='61';
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
--echo # Ok to propagate the equality into HEX(a), because "CHARACTER SET BINARY" does not ignore trailing spaces
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a';
EXPLAIN EXTENDED SELECT * FROM t1 WHERE HEX(a)='61' AND a='a ';
DROP TABLE t1;
--echo #
--echo # End of 10.1 tests
--echo #
sql/field.cc
View file @ 1a36caf0
......@@ -1261,6 +1261,61 @@ bool Field::test_if_equality_guarantees_uniqueness(const Item *item) const
}
/**
Check whether a field item can be substituted for an equal item
@details
The function checks whether a substitution of a field item for
an equal item is valid.
@param arg *arg != NULL <-> the field is in the context
where substitution for an equal item is valid
@note
The following statement is not always true:
@n
x=y => F(x)=F(x/y).
@n
This means substitution of an item for an equal item not always
yields an equavalent condition. Here's an example:
@code
'a'='a '
(LENGTH('a')=1) != (LENGTH('a ')=2)
@endcode
Such a substitution is surely valid if either the substituted
field is not of a STRING type or if it is an argument of
a comparison predicate.
@retval
TRUE substitution is valid
@retval
FALSE otherwise
*/
bool Field::can_be_substituted_to_equal_item(const Context &ctx,
const Item_equal *item_equal)
{
DBUG_ASSERT(item_equal->compare_type() != STRING_RESULT);
DBUG_ASSERT(cmp_type() != STRING_RESULT);
switch (ctx.subst_constraint()) {
case ANY_SUBST:
/*
Disable const propagation for items used in different comparison contexts.
This must be done because, for example, Item_hex_string->val_int() is not
the same as (Item_hex_string->val_str() in BINARY column)->val_int().
We cannot simply disable the replacement in a particular context (
e.g. <bin_col> = <int_col> AND <bin_col> = <hex_string>) since
Items don't know the context they are in and there are functions like
IF (<hex_string>, 'yes', 'no').
*/
return ctx.compare_type() == item_equal->compare_type();
case IDENTITY_SUBST:
return true;
}
return false;
}
/*
This handles all numeric and BIT data types.
*/
......@@ -1303,7 +1358,7 @@ Field_num::Field_num(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
}
void Field_num::prepend_zeros(String *value)
void Field_num::prepend_zeros(String *value) const
{
int diff;
if ((diff= (int) (field_length - value->length())) > 0)
......@@ -1317,6 +1372,36 @@ void Field_num::prepend_zeros(String *value)
}
}
/**
Convert a numeric value to a zero-filled string
@param[in] thd current thread
@param[in] item the item to convert
This function converts a numeric value to a string. In this conversion
the zero-fill flag of the field is taken into account.
This is required so the resulting string value can be used instead of
the field reference when propagating equalities.
*/
Item *Field_num::convert_zerofill_number_to_string(THD *thd, Item *item) const
{
char buff[MAX_FIELD_WIDTH],*pos;
String tmp(buff,sizeof(buff),Field_num::charset()), *res;
res= item->val_str(&tmp);
if (item->is_null())
return new (thd->mem_root) Item_null(thd);
else
{
prepend_zeros(res);
pos= (char *) sql_strmake (res->ptr(), res->length());
return new (thd->mem_root) Item_string(thd, pos, res->length(), Field_num::charset());
}
}
/**
Test if given number is a int.
......@@ -1884,6 +1969,23 @@ bool Field_str::test_if_equality_guarantees_uniqueness(const Item *item) const
}
bool Field_str::can_be_substituted_to_equal_item(const Context &ctx,
const Item_equal *item_equal)
{
DBUG_ASSERT(item_equal->compare_type() == STRING_RESULT);
switch (ctx.subst_constraint()) {
case ANY_SUBST:
return ctx.compare_type() == item_equal->compare_type() &&
(ctx.compare_type() != STRING_RESULT ||
ctx.compare_collation() == item_equal->compare_collation());
case IDENTITY_SUBST:
return ((charset()->state & MY_CS_BINSORT) &&
(charset()->state & MY_CS_NOPAD));
}
return false;
}
void Field_num::make_field(Send_field *field)
{
Field::make_field(field);
......
sql/field.h
View file @ 1a36caf0
......@@ -41,6 +41,7 @@ class Column_statistics;
class Column_statistics_collected;
class Item_func;
class Item_bool_func;
class Item_equal;
enum enum_check_fields
{
......@@ -50,6 +51,77 @@ enum enum_check_fields
};
/*
Common declarations for Field and Item
*/
class Value_source
{
public:
/*
The enumeration Subst_constraint is currently used only in implementations
of the virtual function subst_argument_checker.
*/
enum Subst_constraint
{
ANY_SUBST, /* Any substitution for a field is allowed */
IDENTITY_SUBST /* Substitution for a field is allowed if any two
different values of the field type are not equal */
};
/*
Item context attributes.
Comparison functions pass their attributes to propagate_equal_fields().
For exmple, for string comparison, the collation of the comparison
operation is important inside propagate_equal_fields().
*/
class Context
{
/*
Which type of propagation is allowed:
- ANY_SUBST (loose equality, according to the collation), or
- IDENTITY_SUBST (strict binary equality).
*/
Subst_constraint m_subst_constraint;
/*
Comparison type.
Impostant only when ANY_SUBSTS.
*/
Item_result m_compare_type;
/*
Collation of the comparison operation.
Important only when ANY_SUBST.
*/
CHARSET_INFO *m_compare_collation;
public:
Context(Subst_constraint subst, Item_result type, CHARSET_INFO *cs)
:m_subst_constraint(subst),
m_compare_type(type),
m_compare_collation(cs) { }
Subst_constraint subst_constraint() const { return m_subst_constraint; }
Item_result compare_type() const
{
DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
return m_compare_type;
}
CHARSET_INFO *compare_collation() const
{
DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
return m_compare_collation;
}
};
class Context_identity: public Context
{ // Use this to request only exact value, no invariants.
public:
Context_identity()
:Context(IDENTITY_SUBST, STRING_RESULT, &my_charset_bin) { }
};
class Context_boolean: public Context
{ // Use this when an item is [a part of] a boolean expression
public:
Context_boolean() :Context(ANY_SUBST, INT_RESULT, &my_charset_bin) { }
};
};
enum Derivation
{
DERIVATION_IGNORABLE= 6,
......@@ -61,6 +133,7 @@ enum Derivation
DERIVATION_EXPLICIT= 0
};
#define STORAGE_TYPE_MASK 7
#define COLUMN_FORMAT_MASK 7
#define COLUMN_FORMAT_SHIFT 3
......@@ -287,11 +360,18 @@ class Virtual_column_info: public Sql_alloc
}
};
class Field
class Field: public Value_source
{
Field(const Item &); /* Prevent use of these */
void operator=(Field &);
public:
virtual bool can_be_substituted_to_equal_item(const Context &ctx,
const Item_equal *item);
virtual Item *get_equal_const_item(THD *thd, const Context &ctx,
Item_field *field_item, Item *const_item)
{
return const_item;
}
static void *operator new(size_t size, MEM_ROOT *mem_root) throw ()
{ return alloc_root(mem_root, size); }
static void *operator new(size_t size) throw ()
......@@ -1126,7 +1206,11 @@ class Field
class Field_num :public Field {
protected:
Item *convert_zerofill_number_to_string(THD *thd, Item *item) const;
public:
Item *get_equal_const_item(THD *thd, const Context &ctx,
Item_field *field_item, Item *const_item);
const uint8 dec;
bool zerofill,unsigned_flag; // Purify cannot handle bit fields
Field_num(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
......@@ -1137,7 +1221,7 @@ class Field_num :public Field {
enum Derivation derivation(void) const { return DERIVATION_NUMERIC; }
uint repertoire(void) const { return MY_REPERTOIRE_NUMERIC; }
CHARSET_INFO *charset(void) const { return &my_charset_numeric; }
void prepend_zeros(String *value);
void prepend_zeros(String *value) const;
void add_zerofill_and_unsigned(String &res) const;
friend class Create_field;
void make_field(Send_field *);
......@@ -1172,6 +1256,8 @@ class Field_str :public Field {
CHARSET_INFO *field_charset;
enum Derivation field_derivation;
public:
bool can_be_substituted_to_equal_item(const Context &ctx,
const Item_equal *item_equal);
Field_str(uchar *ptr_arg,uint32 len_arg, uchar *null_ptr_arg,
uchar null_bit_arg, utype unireg_check_arg,
const char *field_name_arg, CHARSET_INFO *charset);
......
sql/item.cc
View file @ 1a36caf0
......@@ -5330,93 +5330,6 @@ Item_equal *Item_field::find_item_equal(COND_EQUAL *cond_equal)
}
/**
Check whether a field item can be substituted for an equal item
@details
The function checks whether a substitution of a field item for
an equal item is valid.
@param arg *arg != NULL <-> the field is in the context
where substitution for an equal item is valid
@note
The following statement is not always true:
@n
x=y => F(x)=F(x/y).
@n
This means substitution of an item for an equal item not always
yields an equavalent condition. Here's an example:
@code
'a'='a '
(LENGTH('a')=1) != (LENGTH('a ')=2)
@endcode
Such a substitution is surely valid if either the substituted
field is not of a STRING type or if it is an argument of
a comparison predicate.
@retval
TRUE substitution is valid
@retval
FALSE otherwise
*/
bool Item_field::can_be_substituted_to_equal_item(const Context &ctx,
const Item_equal *item_equal)
{
switch (ctx.subst_constraint()) {
case ANY_SUBST:
/*
Disable const propagation for items used in different comparison contexts.
This must be done because, for example, Item_hex_string->val_int() is not
the same as (Item_hex_string->val_str() in BINARY column)->val_int().
We cannot simply disable the replacement in a particular context (
e.g. <bin_col> = <int_col> AND <bin_col> = <hex_string>) since
Items don't know the context they are in and there are functions like
IF (<hex_string>, 'yes', 'no').
*/
return
ctx.compare_type() == item_equal->compare_type() &&
(ctx.compare_type() != STRING_RESULT ||
ctx.compare_collation() == item_equal->compare_collation());
case IDENTITY_SUBST:
return field->cmp_type() != STRING_RESULT ||
((field->charset()->state & MY_CS_BINSORT) &&
(field->charset()->state & MY_CS_NOPAD));
}
return false;
}
/**
Convert a numeric value to a zero-filled string
@param[in,out] item the item to operate on
@param field The field that this value is equated to
This function converts a numeric value to a string. In this conversion
the zero-fill flag of the field is taken into account.
This is required so the resulting string value can be used instead of
the field reference when propagating equalities.
*/
static void convert_zerofill_number_to_string(THD *thd, Item **item, Field_num *field)
{
char buff[MAX_FIELD_WIDTH],*pos;
String tmp(buff,sizeof(buff), field->charset()), *res;
res= (*item)->val_str(&tmp);
if ((*item)->is_null())
*item= new (thd->mem_root) Item_null(thd);
else
{
field->prepend_zeros(res);
pos= (char *) sql_strmake (res->ptr(), res->length());
*item= new (thd->mem_root) Item_string(thd, pos, res->length(), field->charset());
}
}
/**
Set a pointer to the multiple equality the field reference belongs to
(if any).
......@@ -5445,32 +5358,34 @@ Item *Item_field::propagate_equal_fields(THD *thd,
const Context &ctx,
COND_EQUAL *arg)
{
if (no_const_subst)
if (no_const_subst || !(item_equal= find_item_equal(arg)))
return this;
item_equal= find_item_equal(arg);
Item *item= 0;
if (item_equal)
if (!field->can_be_substituted_to_equal_item(ctx, item_equal))
{
if (!can_be_substituted_to_equal_item(ctx, item_equal))
{
item_equal= NULL;
return this;
}
item= item_equal->get_const();
item_equal= NULL;
return this;
}
if (!item)
item= this;
else if (field && (field->flags & ZEROFILL_FLAG) && IS_NUM(field->type()))
Item *item= item_equal->get_const();
return item ? field->get_equal_const_item(thd, ctx, this, item) : this;
}
Item *Field_num::get_equal_const_item(THD *thd, const Context &ctx,
Item_field *field_item,
Item *const_item)
{
DBUG_ASSERT(const_item->const_item());
if ((flags & ZEROFILL_FLAG) && IS_NUM(type()))
{
if (ctx.subst_constraint() == IDENTITY_SUBST)
convert_zerofill_number_to_string(thd, &item, (Field_num *)field);
return convert_zerofill_number_to_string(thd, const_item);
else
{
DBUG_ASSERT(ctx.compare_type() != STRING_RESULT);
item= this;
return field_item;
}
}
return item;
return const_item;
}
......
sql/item.h
View file @ 1a36caf0
......@@ -602,7 +602,7 @@ class Type_std_attributes
};
class Item: public Type_std_attributes
class Item: public Value_source, public Type_std_attributes
{
Item(const Item &); /* Prevent use of these */
void operator=(Item &);
......@@ -1417,70 +1417,6 @@ class Item: public Type_std_attributes
return FALSE;
}
/*
The enumeration Subst_constraint is currently used only in implementations
of the virtual function subst_argument_checker.
*/
enum Subst_constraint
{
ANY_SUBST, /* Any substitution for a field is allowed */
IDENTITY_SUBST /* Substitution for a field is allowed if any two
different values of the field type are not equal */
};
/*
Item context attributes.
Comparison functions pass their attributes to propagate_equal_fields().
For exmple, for string comparison, the collation of the comparison
operation is important inside propagate_equal_fields().
*/
class Context
{
/*
Which type of propagation is allowed:
- ANY_SUBST (loose equality, according to the collation), or
- IDENTITY_SUBST (strict binary equality).
*/
Subst_constraint m_subst_constraint;
/*
Comparison type.
Impostant only when ANY_SUBSTS.
*/
Item_result m_compare_type;
/*
Collation of the comparison operation.
Important only when ANY_SUBST.
*/
CHARSET_INFO *m_compare_collation;
public:
Context(Subst_constraint subst, Item_result type, CHARSET_INFO *cs)
:m_subst_constraint(subst),
m_compare_type(type),
m_compare_collation(cs) { }
Subst_constraint subst_constraint() const { return m_subst_constraint; }
Item_result compare_type() const
{
DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
return m_compare_type;
}
CHARSET_INFO *compare_collation() const
{
DBUG_ASSERT(m_subst_constraint == ANY_SUBST);
return m_compare_collation;
}
};
class Context_identity: public Context
{ // Use this to request only exact value, no invariants.
public:
Context_identity()
:Context(IDENTITY_SUBST, STRING_RESULT, &my_charset_bin) { }
};
class Context_boolean: public Context
{ // Use this when an item is [a part of] a boolean expression
public:
Context_boolean() :Context(ANY_SUBST, INT_RESULT, &my_charset_bin) { }
};
virtual Item* propagate_equal_fields(THD*, const Context &, COND_EQUAL *)
{
return this;
......@@ -2312,8 +2248,6 @@ class Item_ident_for_show :public Item
class Item_field :public Item_ident
{
bool can_be_substituted_to_equal_item(const Context &ctx,
const Item_equal *item);
protected:
void set_field(Field *field);
public:
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7