Skip to content

Commit

Permalink
MDEV-32879 Server crash in my_decimal::operator= or unexpected ER_DUP…
Browse files Browse the repository at this point in the history
…_ENTRY upon comparison with INET6 and similar types

This is the 10.6 version of the patch.

Item_bool_rowready_func2, Item_func_between, Item_func_in
did not check if a not-NULL argument of an arbitrary data type
can produce a NULL value on conversion to INET6.

This caused a crash on DBUG_ASSERT() in conversion failures,
because the function returned SQL NULL for something that
has Item::maybe_null() equal to false.

Adding setting NULL-ability in such cases.

Details:

- Removing the code in Item_func::setup_args_and_comparator()
  performing character set aggregation with optional narrowing.
  This aggregation is done inside Arg_comparator::set_cmp_func_string().
  So this code was redundant

- Removing Item_func::setup_args_and_comparator() as it git simplified to
  just to two lines:
    convert_const_compared_to_int_field(thd);
    return cmp->set_cmp_func(thd, this, &args[0], &args[1], true);
  Using these lines directly in:
    - Item_bool_rowready_func2::fix_length_and_dec()
    - Item_func_nullif::fix_length_and_dec()

- Adding a new virtual method:
  - Type_handler::Item_bool_rowready_func2_fix_length_and_dec().

- Adding tests detecting if the data type conversion can return SQL NULL into
  the following methods of Type_handler_fbt:
  - Item_bool_rowready_func2_fix_length_and_dec
  - Item_func_between_fix_length_and_dec
  - Item_func_in_fix_comparator_compatible_types
  • Loading branch information
abarkov committed Nov 27, 2023
1 parent 2f467de commit 20b0ec9
Show file tree
Hide file tree
Showing 9 changed files with 196 additions and 59 deletions.
75 changes: 75 additions & 0 deletions plugin/type_inet/mysql-test/type_inet/type_inet6.result
Original file line number Diff line number Diff line change
Expand Up @@ -2283,3 +2283,78 @@ a
Warnings:
Warning 1292 Incorrect inet6 value: ''
DROP TABLE t1;
#
# MDEV-32879 Server crash in my_decimal::operator= or unexpected ER_DUP_ENTRY upon comparison with INET6 and similar types
#
CREATE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL, c DATETIME(6) NOT NULL);
INSERT INTO t1 VALUES ('','::','2000-01-01'),('','::','1900-01-01');
SELECT c + (b = a) AS f, GROUP_CONCAT(c) FROM t1 GROUP BY f;
f GROUP_CONCAT(c)
NULL 2000-01-01 00:00:00.000000,1900-01-01 00:00:00.000000
Warnings:
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
DROP TABLE t1;
CREATE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL, c DATETIME(6) NOT NULL);
INSERT INTO t1 VALUES ('','::','2000-01-01'),('','::','1900-01-01');
SELECT c + (b = a) AS f, COUNT(c) FROM t1 GROUP BY f;
f COUNT(c)
NULL 2
Warnings:
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
DROP TABLE t1;
CREATE OR REPLACE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL, c DATETIME(6) NOT NULL);
INSERT INTO t1 VALUES ('','::','2000-01-01'),('','::','1900-01-01');
SELECT c + (b = a) AS f FROM t1 ORDER BY f;
f
NULL
NULL
Warnings:
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
DROP TABLE t1;
CREATE OR REPLACE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL);
INSERT INTO t1 VALUES ('','::'),('','::');
SELECT 1.00 + (b = a) AS f FROM t1 ORDER BY f;
f
NULL
NULL
Warnings:
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
SELECT 1.00 + (b BETWEEN a AND '') AS f FROM t1 ORDER BY f;
f
NULL
NULL
Warnings:
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
SELECT 1.00 + (b IN (a,'')) AS f FROM t1 ORDER BY f;
f
NULL
NULL
Warnings:
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
Warning 1292 Incorrect inet6 value: ''
DROP TABLE t1;
26 changes: 26 additions & 0 deletions plugin/type_inet/mysql-test/type_inet/type_inet6.test
Original file line number Diff line number Diff line change
Expand Up @@ -1675,3 +1675,29 @@ CREATE OR REPLACE TABLE t1 (a INET6);
INSERT INTO t1 VALUES ('::');
SELECT * FROM t1 WHERE a IN ('','::1');
DROP TABLE t1;

--echo #
--echo # MDEV-32879 Server crash in my_decimal::operator= or unexpected ER_DUP_ENTRY upon comparison with INET6 and similar types
--echo #

CREATE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL, c DATETIME(6) NOT NULL);
INSERT INTO t1 VALUES ('','::','2000-01-01'),('','::','1900-01-01');
SELECT c + (b = a) AS f, GROUP_CONCAT(c) FROM t1 GROUP BY f;
DROP TABLE t1;

CREATE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL, c DATETIME(6) NOT NULL);
INSERT INTO t1 VALUES ('','::','2000-01-01'),('','::','1900-01-01');
SELECT c + (b = a) AS f, COUNT(c) FROM t1 GROUP BY f;
DROP TABLE t1;

CREATE OR REPLACE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL, c DATETIME(6) NOT NULL);
INSERT INTO t1 VALUES ('','::','2000-01-01'),('','::','1900-01-01');
SELECT c + (b = a) AS f FROM t1 ORDER BY f;
DROP TABLE t1;

CREATE OR REPLACE TABLE t1 (a CHAR(36) NOT NULL, b INET6 NOT NULL);
INSERT INTO t1 VALUES ('','::'),('','::');
SELECT 1.00 + (b = a) AS f FROM t1 ORDER BY f;
SELECT 1.00 + (b BETWEEN a AND '') AS f FROM t1 ORDER BY f;
SELECT 1.00 + (b IN (a,'')) AS f FROM t1 ORDER BY f;
DROP TABLE t1;
67 changes: 21 additions & 46 deletions sql/item_cmpfunc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -457,40 +457,6 @@ void Item_bool_func::raise_note_if_key_become_unused(THD *thd, const Item_args &
}


bool Item_func::setup_args_and_comparator(THD *thd, Arg_comparator *cmp)
{
DBUG_ASSERT(arg_count >= 2); // Item_func_nullif has arg_count == 3

if (args[0]->cmp_type() == STRING_RESULT &&
args[1]->cmp_type() == STRING_RESULT)
{
CHARSET_INFO *tmp;
/*
Use charset narrowing only for equalities, as that would allow
to construct ref access.
Non-equality comparisons with constants work without charset narrowing,
the constant gets converted.
Non-equality comparisons with non-constants would need narrowing to
enable range optimizer to handle e.g.
t1.mb3key_col <= const_table.mb4_col
But this doesn't look important.
*/
bool allow_narrowing= MY_TEST(functype()==Item_func::EQ_FUNC ||
functype()==Item_func::EQUAL_FUNC);

if (agg_arg_charsets_for_comparison(&tmp, &args[0], &args[1],
allow_narrowing))
return true;
cmp->m_compare_collation= tmp;
}
// Convert constants when compared to int/year field
DBUG_ASSERT(functype() != LIKE_FUNC);
convert_const_compared_to_int_field(thd);

return cmp->set_cmp_func(thd, this, &args[0], &args[1], true);
}


/*
Comparison operators remove arguments' dependency on PAD_CHAR_TO_FULL_LENGTH
in case of PAD SPACE comparison collations: trailing spaces do not affect
Expand Down Expand Up @@ -520,8 +486,15 @@ bool Item_bool_rowready_func2::fix_length_and_dec()
if (!args[0] || !args[1])
return FALSE;
Item_args old_args(args[0], args[1]);
if (setup_args_and_comparator(thd, &cmp))
convert_const_compared_to_int_field(thd);
Type_handler_hybrid_field_type tmp;
if (tmp.aggregate_for_comparison(func_name_cstring(), args, 2, false) ||
tmp.type_handler()->Item_bool_rowready_func2_fix_length_and_dec(thd,
this))
{
DBUG_ASSERT(thd->is_error());
return true;
}
raise_note_if_key_become_unused(thd, old_args);
return false;
}
Expand All @@ -541,21 +514,14 @@ bool Item_bool_rowready_func2::fix_length_and_dec()
*/

int Arg_comparator::set_cmp_func(THD *thd, Item_func_or_sum *owner_arg,
const Type_handler *compare_handler,
Item **a1, Item **a2)
{
owner= owner_arg;
set_null= set_null && owner_arg;
a= a1;
b= a2;
Item *tmp_args[2]= {*a1, *a2};
Type_handler_hybrid_field_type tmp;
if (tmp.aggregate_for_comparison(owner_arg->func_name_cstring(), tmp_args, 2,
false))
{
DBUG_ASSERT(thd->is_error());
return 1;
}
m_compare_handler= tmp.type_handler();
m_compare_handler= compare_handler;
return m_compare_handler->set_comparator_func(thd, this);
}

Expand Down Expand Up @@ -606,6 +572,14 @@ bool Arg_comparator::set_cmp_func_string(THD *thd)
We must set cmp_collation here as we may be called from for an automatic
generated item, like in natural join.
Allow reinterpted superset as subset.
Use charset narrowing only for equalities, as that would allow
to construct ref access.
Non-equality comparisons with constants work without charset narrowing,
the constant gets converted.
Non-equality comparisons with non-constants would need narrowing to
enable range optimizer to handle e.g.
t1.mb3key_col <= const_table.mb4_col
But this doesn't look important.
*/
bool allow_narrowing= false;
if (owner->type() == Item::FUNC_ITEM)
Expand Down Expand Up @@ -2816,8 +2790,9 @@ Item_func_nullif::fix_length_and_dec()
fix_char_length(args[2]->max_char_length());
set_maybe_null();
m_arg0= args[0];
if (setup_args_and_comparator(thd, &cmp))
return TRUE;
convert_const_compared_to_int_field(thd);
if (cmp.set_cmp_func(thd, this, &args[0], &args[1], true/*set_null*/))
return true;
/*
A special code for EXECUTE..PREPARE.
Expand Down
28 changes: 25 additions & 3 deletions sql/item_cmpfunc.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class Arg_comparator: public Sql_alloc
// when one of arguments is NULL.

int set_cmp_func(THD *thd, Item_func_or_sum *owner_arg,
const Type_handler *compare_handler,
Item **a1, Item **a2);

int compare_not_null_values(longlong val1, longlong val2)
Expand Down Expand Up @@ -95,11 +96,24 @@ class Arg_comparator: public Sql_alloc
bool set_cmp_func_decimal(THD *thd);

inline int set_cmp_func(THD *thd, Item_func_or_sum *owner_arg,
Item **a1, Item **a2, bool set_null_arg)
const Type_handler *compare_handler,
Item **a1, Item **a2, bool set_null_arg)
{
set_null= set_null_arg;
return set_cmp_func(thd, owner_arg, a1, a2);
return set_cmp_func(thd, owner_arg, compare_handler, a1, a2);
}
int set_cmp_func(THD *thd, Item_func_or_sum *owner_arg,
Item **a1, Item **a2, bool set_null_arg)
{
Item *tmp_args[2]= { *a1, *a2 };
Type_handler_hybrid_field_type tmp;
if (tmp.aggregate_for_comparison(owner_arg->func_name_cstring(),
tmp_args, 2, false))
return 1;
return set_cmp_func(thd, owner_arg, tmp.type_handler(),
a1, a2, set_null_arg);
}

inline int compare() { return (this->*func)(); }

int compare_string(); // compare args[0] & args[1]
Expand Down Expand Up @@ -562,9 +576,17 @@ class Item_bool_rowready_func2 :public Item_bool_func2_with_rev
return this;
}
bool fix_length_and_dec() override;
bool fix_length_and_dec_generic(THD *thd,
const Type_handler *compare_handler)
{
DBUG_ASSERT(args == tmp_arg);
return cmp.set_cmp_func(thd, this, compare_handler,
tmp_arg, tmp_arg + 1, true/*set_null*/);
}
int set_cmp_func(THD *thd)
{
return cmp.set_cmp_func(thd, this, tmp_arg, tmp_arg + 1, true);
DBUG_ASSERT(args == tmp_arg);
return cmp.set_cmp_func(thd, this, tmp_arg, tmp_arg + 1, true/*set_null*/);
}
CHARSET_INFO *compare_collation() const override
{ return cmp.compare_collation(); }
Expand Down
9 changes: 0 additions & 9 deletions sql/item_func.h
Original file line number Diff line number Diff line change
Expand Up @@ -388,15 +388,6 @@ class Item_func :public Item_func_or_sum
}
}
void convert_const_compared_to_int_field(THD *thd);
/**
Prepare arguments and setup a comparator.
Used in Item_func_xxx with two arguments and a comparator,
e.g. Item_bool_func2 and Item_func_nullif.
args[0] or args[1] can be modified:
- converted to character set and collation of the operation
- or replaced to an Item_int_with_ref
*/
bool setup_args_and_comparator(THD *thd, Arg_comparator *cmp);
Item_func *get_item_func() override { return this; }
bool is_simplified_cond_processor(void *arg) override
{ return const_item() && !val_int(); }
Expand Down
7 changes: 6 additions & 1 deletion sql/item_sum.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1288,9 +1288,14 @@ void Item_sum_min_max::setup_hybrid(THD *thd, Item *item, Item *value_arg)
/* Don't cache value, as it will change */
if (!item->const_item())
arg_cache->set_used_tables(RAND_TABLE_BIT);
DBUG_ASSERT(item->type_handler_for_comparison() ==
value->type_handler_for_comparison());
DBUG_ASSERT(item->type_handler_for_comparison() ==
arg_cache->type_handler_for_comparison());
cmp= new (thd->mem_root) Arg_comparator();
if (cmp)
cmp->set_cmp_func(thd, this, (Item**)&arg_cache, (Item**)&value, FALSE);
cmp->set_cmp_func(thd, this, item->type_handler_for_comparison(),
(Item**)&arg_cache, (Item**)&value, FALSE);
DBUG_VOID_RETURN;
}

Expand Down
8 changes: 8 additions & 0 deletions sql/sql_type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5679,6 +5679,14 @@ Type_handler_string_result::Item_func_hybrid_field_type_get_date(

/***************************************************************************/

bool Type_handler::Item_bool_rowready_func2_fix_length_and_dec(THD *thd,
Item_bool_rowready_func2 *func) const
{
return func->fix_length_and_dec_generic(thd, this);
}

/***************************************************************************/

bool Type_handler_numeric::
Item_func_between_fix_length_and_dec(Item_func_between *func) const
{
Expand Down
3 changes: 3 additions & 0 deletions sql/sql_type.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ class Item_hybrid_func;
class Item_func_min_max;
class Item_func_hybrid_field_type;
class Item_bool_func2;
class Item_bool_rowready_func2;
class Item_func_between;
class Item_func_in;
class Item_func_round;
Expand Down Expand Up @@ -4271,6 +4272,8 @@ class Type_handler
}
virtual bool Item_eq_value(THD *thd, const Type_cmp_attributes *attr,
Item *a, Item *b) const= 0;
virtual bool Item_bool_rowready_func2_fix_length_and_dec(THD *thd,
Item_bool_rowready_func2 *func) const;
virtual bool Item_hybrid_func_fix_attributes(THD *thd,
const LEX_CSTRING &name,
Type_handler_hybrid_field_type *,
Expand Down
32 changes: 32 additions & 0 deletions sql/sql_type_fixedbin.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,21 @@ class Type_handler_fbt: public Type_handler
return Fbt_null(item, false).is_null();
}

/*
Check at fix_fields() time if any of the items can return a nullable
value on conversion to Fbt.
*/
static bool fix_fields_maybe_null_on_conversion_to_fbt(Item **items,
uint count)
{
for (uint i= 0; i < count; i++)
{
if (Fbt::fix_fields_maybe_null_on_conversion_to_fbt(items[i]))
return true;
}
return false;
}

public:

Fbt(Item *item, bool *error, bool warn= true)
Expand Down Expand Up @@ -1532,6 +1547,16 @@ class Type_handler_fbt: public Type_handler
Fbt_null na(a), nb(b);
return !na.is_null() && !nb.is_null() && !na.cmp(nb);
}
bool Item_bool_rowready_func2_fix_length_and_dec(THD *thd,
Item_bool_rowready_func2 *func) const override
{
if (Type_handler::Item_bool_rowready_func2_fix_length_and_dec(thd, func))
return true;
if (!func->maybe_null() &&
Fbt::fix_fields_maybe_null_on_conversion_to_fbt(func->arguments(), 2))
func->set_maybe_null();
return false;
}
bool Item_hybrid_func_fix_attributes(THD *thd, const LEX_CSTRING &name,
Type_handler_hybrid_field_type *h,
Type_all_attributes *attr,
Expand Down Expand Up @@ -1713,6 +1738,9 @@ class Type_handler_fbt: public Type_handler

bool Item_func_between_fix_length_and_dec(Item_func_between *func) const override
{
if (!func->maybe_null() &&
Fbt::fix_fields_maybe_null_on_conversion_to_fbt(func->arguments(), 3))
func->set_maybe_null();
return false;
}
longlong Item_func_between_val_int(Item_func_between *func) const override
Expand All @@ -1735,6 +1763,10 @@ class Type_handler_fbt: public Type_handler
Item_func_in *func)
const override
{
if (!func->maybe_null() &&
Fbt::fix_fields_maybe_null_on_conversion_to_fbt(func->arguments(),
func->argument_count()))
func->set_maybe_null();
if (func->compatible_types_scalar_bisection_possible())
{
return func->value_list_convert_const_to_int(thd) ||
Expand Down

0 comments on commit 20b0ec9

Please sign in to comment.