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

During the 10.5->10.6 merge please use the 10.6 code on conflicts.

This is the 10.5 version of the patch (a backport of the 10.6 version).
Unlike 10.6 version, it makes changes in plugin/type_inet/sql_type_inet.*
rather than in sql/sql_type_fixedbin.h

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_inet6:
  - 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 28, 2023
1 parent dd62a28 commit f436b4a
Show file tree
Hide file tree
Showing 12 changed files with 255 additions and 64 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 @@ -2255,3 +2255,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 @@ -1660,3 +1660,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;
33 changes: 33 additions & 0 deletions plugin/type_inet/sql_type_inet.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,21 @@ class Inet6
*/
static bool fix_fields_maybe_null_on_conversion_to_inet6(Item *item);

/*
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_inet6(Item **items,
uint count)
{
for (uint i= 0; i < count; i++)
{
if (fix_fields_maybe_null_on_conversion_to_inet6(items[i]))
return true;
}
return false;
}

public:

Inet6(Item *item, bool *error, bool warn= true)
Expand Down Expand Up @@ -714,6 +729,16 @@ class Type_handler_inet6: public Type_handler
Inet6_null 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 &&
Inet6::fix_fields_maybe_null_on_conversion_to_inet6(func->arguments(), 2))
func->maybe_null= true;
return false;
}
bool Item_hybrid_func_fix_attributes(THD *thd,
const char *name,
Type_handler_hybrid_field_type *h,
Expand Down Expand Up @@ -902,6 +927,10 @@ class Type_handler_inet6: public Type_handler
bool
Item_func_between_fix_length_and_dec(Item_func_between *func) const override
{
if (!func->maybe_null &&
Inet6::fix_fields_maybe_null_on_conversion_to_inet6(func->arguments(), 3))
func->maybe_null= true;

return false;
}
longlong Item_func_between_val_int(Item_func_between *func) const override
Expand All @@ -918,6 +947,10 @@ class Type_handler_inet6: public Type_handler
Item_func_in *func)
const override
{
if (!func->maybe_null &&
Inet6::fix_fields_maybe_null_on_conversion_to_inet6(func->arguments(),
func->argument_count()))
func->maybe_null= true;
if (func->compatible_types_scalar_bisection_possible())
{
return func->value_list_convert_const_to_int(thd) ||
Expand Down
26 changes: 23 additions & 3 deletions sql/item.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2497,7 +2497,8 @@ bool DTCollation::aggregate(const DTCollation &dt, uint flags)

/******************************/
static
void my_coll_agg_error(DTCollation &c1, DTCollation &c2, const char *fname)
void my_coll_agg_error(const DTCollation &c1, const DTCollation &c2,
const char *fname)
{
my_error(ER_CANT_AGGREGATE_2COLLATIONS,MYF(0),
c1.collation->name,c1.derivation_name(),
Expand Down Expand Up @@ -2579,10 +2580,17 @@ bool Type_std_attributes::agg_item_collations(DTCollation &c, const char *fname,
}


/*
@param single_err When nargs==1, use *single_err as the second aggregated
collation when producing error message.
*/

bool Type_std_attributes::agg_item_set_converter(const DTCollation &coll,
const char *fname,
Item **args, uint nargs,
uint flags, int item_sep)
uint flags, int item_sep,
const Single_coll_err
*single_err)
{
THD *thd= current_thd;
if (thd->lex->is_ps_or_view_context_analysis())
Expand Down Expand Up @@ -2620,7 +2628,19 @@ bool Type_std_attributes::agg_item_set_converter(const DTCollation &coll,
args[0]= safe_args[0];
args[item_sep]= safe_args[1];
}
my_coll_agg_error(args, nargs, fname, item_sep);
if (nargs == 1 && single_err)
{
/*
Use *single_err to produce an error message mentioning two
collations.
*/
if (single_err->first)
my_coll_agg_error(args[0]->collation, single_err->coll, fname);
else
my_coll_agg_error(single_err->coll, args[0]->collation, fname);
}
else
my_coll_agg_error(args, nargs, fname, item_sep);
return TRUE;
}

Expand Down
11 changes: 9 additions & 2 deletions sql/item.h
Original file line number Diff line number Diff line change
Expand Up @@ -5237,10 +5237,17 @@ class Item_func_or_sum: public Item_result_field,
func_name());
return true;
}
/*
If necessary, convert both *a and *b to the collation in tmp:
*/
Single_coll_err error_for_a= {(*b)->collation, true};
Single_coll_err error_for_b= {(*a)->collation, false};
if (agg_item_set_converter(tmp, func_name(),
a, 1, MY_COLL_CMP_CONV, 1) ||
a, 1, MY_COLL_CMP_CONV, 1,
/*just for error message*/ &error_for_a) ||
agg_item_set_converter(tmp, func_name(),
b, 1, MY_COLL_CMP_CONV, 1))
b, 1, MY_COLL_CMP_CONV, 1,
/*just for error message*/ &error_for_b))
return true;
*cs= tmp.collation;
return false;
Expand Down
65 changes: 30 additions & 35 deletions sql/item_cmpfunc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -413,26 +413,6 @@ void Item_func::convert_const_compared_to_int_field(THD *thd)
}


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)
{
DTCollation tmp;
if (agg_arg_charsets_for_comparison(tmp, args, 2))
return true;
cmp->m_compare_collation= tmp.collation;
}
// 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(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 All @@ -452,6 +432,7 @@ Item_bool_rowready_func2::value_depends_on_sql_mode() const

bool Item_bool_rowready_func2::fix_length_and_dec()
{
THD *thd= current_thd;
max_length= 1; // Function returns 0 or 1

/*
Expand All @@ -460,7 +441,16 @@ bool Item_bool_rowready_func2::fix_length_and_dec()
*/
if (!args[0] || !args[1])
return FALSE;
return setup_args_and_comparator(current_thd, &cmp);
convert_const_compared_to_int_field(thd);
Type_handler_hybrid_field_type tmp;
if (tmp.aggregate_for_comparison(func_name(), args, 2, false) ||
tmp.type_handler()->Item_bool_rowready_func2_fix_length_and_dec(thd,
this))
{
DBUG_ASSERT(thd->is_error());
return true;
}
return false;
}


Expand All @@ -477,27 +467,22 @@ bool Item_bool_rowready_func2::fix_length_and_dec()
items, holding the cached converted value of the original (constant) item.
*/

int Arg_comparator::set_cmp_func(Item_func_or_sum *owner_arg,
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(), tmp_args, 2, false))
{
DBUG_ASSERT(current_thd->is_error());
return 1;
}
m_compare_handler= tmp.type_handler();
m_compare_handler= compare_handler;
return m_compare_handler->set_comparator_func(this);
}


bool Arg_comparator::set_cmp_func_for_row_arguments()
{
THD *thd= current_thd;
uint n= (*a)->cols();
if (n != (*b)->cols())
{
Expand All @@ -514,8 +499,8 @@ bool Arg_comparator::set_cmp_func_for_row_arguments()
my_error(ER_OPERAND_COLUMNS, MYF(0), (*a)->element_index(i)->cols());
return true;
}
if (comparators[i].set_cmp_func(owner, (*a)->addr(i),
(*b)->addr(i), set_null))
if (comparators[i].set_cmp_func(thd, owner, (*a)->addr(i),
(*b)->addr(i), set_null))
return true;
}
return false;
Expand All @@ -541,7 +526,16 @@ bool Arg_comparator::set_cmp_func_string()
{
/*
We must set cmp_collation here as we may be called from for an automatic
generated item, like in natural join
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.
*/
if (owner->agg_arg_charsets_for_comparison(&m_compare_collation, a, b))
return true;
Expand Down Expand Up @@ -2748,8 +2742,9 @@ Item_func_nullif::fix_length_and_dec()
fix_char_length(args[2]->max_char_length());
maybe_null=1;
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
Loading

0 comments on commit f436b4a

Please sign in to comment.