From dd62a285b88958dae9b9f3e49fdd0c55f6eced72 Mon Sep 17 00:00:00 2001 From: Anel Husakovic Date: Mon, 20 Nov 2023 16:22:07 +0100 Subject: [PATCH 01/11] MDEV-31611: mariadb-setpermission - Can't use string as an ARRAY ref while strict refs in use Reviewer: <> --- scripts/mysql_setpermission.sh | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/mysql_setpermission.sh b/scripts/mysql_setpermission.sh index b3c9c27ca880a..aa165a5e74257 100644 --- a/scripts/mysql_setpermission.sh +++ b/scripts/mysql_setpermission.sh @@ -68,7 +68,7 @@ usage() if ($opt_help); # the help function if ($opt_host =~ s/:(\d+)$//) { - $opt_port = $1; + $opt_port = $1; } if ($opt_host eq '') @@ -98,7 +98,7 @@ my $prefix= 'mysql'; if (eval {DBI->install_driver("MariaDB")}) { $dsn ="DBI:MariaDB:;"; $prefix= 'mariadb'; -} +} else { $dsn = "DBI:mysql:;"; } @@ -226,11 +226,11 @@ sub setpwd { $pass = "PASSWORD(". $dbh->quote($pass) . ")"; } - my $uh= "$user@$host"; + my $uh= $user."@".$host; my $sth = $dbh->prepare("set password for $uh =$pass") || die $dbh->errstr; $sth->execute || die $dbh->errstr; $sth->finish; - print "The password is set for user $user.\n\n"; + print "The password is set for user $uh.\n\n"; } From 20b0ec9aae38faaa4d249f67315cc5394eaea22e Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 27 Nov 2023 09:56:21 +0400 Subject: [PATCH 02/11] MDEV-32879 Server crash in my_decimal::operator= or unexpected ER_DUP_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 --- .../mysql-test/type_inet/type_inet6.result | 75 +++++++++++++++++++ .../mysql-test/type_inet/type_inet6.test | 26 +++++++ sql/item_cmpfunc.cc | 67 ++++++----------- sql/item_cmpfunc.h | 28 ++++++- sql/item_func.h | 9 --- sql/item_sum.cc | 7 +- sql/sql_type.cc | 8 ++ sql/sql_type.h | 3 + sql/sql_type_fixedbin.h | 32 ++++++++ 9 files changed, 196 insertions(+), 59 deletions(-) diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6.result b/plugin/type_inet/mysql-test/type_inet/type_inet6.result index 9c601697e4f4a..38af2c16076ab 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6.result +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6.result @@ -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; diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6.test b/plugin/type_inet/mysql-test/type_inet/type_inet6.test index 771d8fbc347d7..32638573825f3 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6.test +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6.test @@ -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; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index d63db33edb351..48215c607689b 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -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 @@ -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; } @@ -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); } @@ -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) @@ -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. diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index ae0efdd6ae14f..2e8a28cdd174a 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -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) @@ -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] @@ -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(); } diff --git a/sql/item_func.h b/sql/item_func.h index a1712745a89db..1390fab014fd9 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -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(); } diff --git a/sql/item_sum.cc b/sql/item_sum.cc index a26adb17e28b2..b3e600824de43 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -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; } diff --git a/sql/sql_type.cc b/sql/sql_type.cc index 2c12f1b7139f5..fec7f0edb9f63 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -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 { diff --git a/sql/sql_type.h b/sql/sql_type.h index fc270915e5bd3..11ca46517fccd 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -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; @@ -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 *, diff --git a/sql/sql_type_fixedbin.h b/sql/sql_type_fixedbin.h index 9bd23a949fa5e..1ebf559eabb46 100644 --- a/sql/sql_type_fixedbin.h +++ b/sql/sql_type_fixedbin.h @@ -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) @@ -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, @@ -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 @@ -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) || From f436b4a523df603ca3f245e7b523e30bbe06f12b Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Mon, 27 Nov 2023 09:56:21 +0400 Subject: [PATCH 03/11] MDEV-32879 Server crash in my_decimal::operator= or unexpected ER_DUP_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 --- .../mysql-test/type_inet/type_inet6.result | 75 +++++++++++++++++++ .../mysql-test/type_inet/type_inet6.test | 26 +++++++ plugin/type_inet/sql_type_inet.h | 33 ++++++++ sql/item.cc | 26 ++++++- sql/item.h | 11 ++- sql/item_cmpfunc.cc | 65 ++++++++-------- sql/item_cmpfunc.h | 35 +++++++-- sql/item_func.h | 10 --- sql/item_sum.cc | 9 ++- sql/sql_select.cc | 10 +-- sql/sql_type.cc | 8 ++ sql/sql_type.h | 11 ++- 12 files changed, 255 insertions(+), 64 deletions(-) diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6.result b/plugin/type_inet/mysql-test/type_inet/type_inet6.result index 9c43725c733dd..7484a10962c98 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6.result +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6.result @@ -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; diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6.test b/plugin/type_inet/mysql-test/type_inet/type_inet6.test index b0dffb098f2b5..abe9071962ff5 100644 --- a/plugin/type_inet/mysql-test/type_inet/type_inet6.test +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6.test @@ -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; diff --git a/plugin/type_inet/sql_type_inet.h b/plugin/type_inet/sql_type_inet.h index 80d8544e6c941..a83dddb4c26e1 100644 --- a/plugin/type_inet/sql_type_inet.h +++ b/plugin/type_inet/sql_type_inet.h @@ -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) @@ -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, @@ -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 @@ -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) || diff --git a/sql/item.cc b/sql/item.cc index 1853f7b560fcb..44965a1feb97d 100644 --- a/sql/item.cc +++ b/sql/item.cc @@ -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(), @@ -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()) @@ -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; } diff --git a/sql/item.h b/sql/item.h index e7125cccd14ec..166797760539d 100644 --- a/sql/item.h +++ b/sql/item.h @@ -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; diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 4a2de58e7488a..324aceafd41c9 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -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 @@ -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 /* @@ -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; } @@ -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()) { @@ -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; @@ -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; @@ -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. diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 7cbc1236a033f..3929b4c7d3051 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -56,7 +56,9 @@ class Arg_comparator: public Sql_alloc Item *a_cache, *b_cache; // Cached values of a and b items // when one of arguments is NULL. - int set_cmp_func(Item_func_or_sum *owner_arg, Item **a1, Item **a2); + 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) { @@ -93,12 +95,25 @@ class Arg_comparator: public Sql_alloc bool set_cmp_func_real(); bool set_cmp_func_decimal(); - inline int set_cmp_func(Item_func_or_sum *owner_arg, - Item **a1, Item **a2, bool set_null_arg) + inline int set_cmp_func(THD *thd, Item_func_or_sum *owner_arg, + const Type_handler *compare_handler, + Item **a1, Item **a2, bool set_null_arg) { set_null= set_null_arg; - return set_cmp_func(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(), + 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] @@ -533,9 +548,17 @@ class Item_bool_rowready_func2 :public Item_bool_func2_with_rev return this; } bool fix_length_and_dec(); - int set_cmp_func() + 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(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 { return cmp.compare_collation(); } const Type_handler *compare_type_handler() const diff --git a/sql/item_func.h b/sql/item_func.h index bd04af926b0af..ef020fa9054ae 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -395,16 +395,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); - bool with_sum_func() const { return m_with_sum_func; } With_sum_func_cache* get_with_sum_func_cache() { return this; } Item_func *get_item_func() { return this; } diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 46942c0c78575..584fd21ca4ae3 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -1277,9 +1277,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); - cmp= new Arg_comparator(); + 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(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; } diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 62766cdb9cfbe..21132651b070e 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -15405,7 +15405,7 @@ static bool check_row_equality(THD *thd, const Arg_comparator *comparators, { Item_func_eq *eq_item; if (!(eq_item= new (thd->mem_root) Item_func_eq(thd, left_item, right_item)) || - eq_item->set_cmp_func()) + eq_item->set_cmp_func(thd)) return FALSE; eq_item->quick_fix_field(); eq_list->push_back(eq_item, thd->mem_root); @@ -16155,7 +16155,7 @@ Item *eliminate_item_equal(THD *thd, COND *cond, COND_EQUAL *upper_levels, Don't produce equality if const is equal to item_const. */ Item_func_eq *func= new (thd->mem_root) Item_func_eq(thd, item_const, upper_const); - func->set_cmp_func(); + func->set_cmp_func(thd); func->quick_fix_field(); if (func->val_int()) item= 0; @@ -16203,7 +16203,7 @@ Item *eliminate_item_equal(THD *thd, COND *cond, COND_EQUAL *upper_levels, field_item->remove_item_direct_ref(), head_item->remove_item_direct_ref()); - if (!eq_item || eq_item->set_cmp_func()) + if (!eq_item || eq_item->set_cmp_func(thd)) return 0; eq_item->quick_fix_field(); } @@ -16621,7 +16621,7 @@ change_cond_ref_to_const(THD *thd, I_List *save_list, So make sure to use set_cmp_func() only for non-LIKE operators. */ if (functype != Item_func::LIKE_FUNC) - ((Item_bool_rowready_func2*) func)->set_cmp_func(); + ((Item_bool_rowready_func2*) func)->set_cmp_func(thd); } } else if (can_change_cond_ref_to_const(func, left_item, right_item, @@ -16646,7 +16646,7 @@ change_cond_ref_to_const(THD *thd, I_List *save_list, save_list->push_back(tmp2); } if (functype != Item_func::LIKE_FUNC) - ((Item_bool_rowready_func2*) func)->set_cmp_func(); + ((Item_bool_rowready_func2*) func)->set_cmp_func(thd); } } } diff --git a/sql/sql_type.cc b/sql/sql_type.cc index 6bdc96ac401bb..e7058f6b4ed17 100644 --- a/sql/sql_type.cc +++ b/sql/sql_type.cc @@ -5675,6 +5675,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 { diff --git a/sql/sql_type.h b/sql/sql_type.h index aa89c8850af42..ee3eb454ead61 100644 --- a/sql/sql_type.h +++ b/sql/sql_type.h @@ -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; @@ -3233,9 +3234,15 @@ class Type_std_attributes: public Type_numeric_attributes bool agg_item_collations(DTCollation &c, const char *name, Item **items, uint nitems, uint flags, int item_sep); + struct Single_coll_err + { + const DTCollation& coll; + bool first; + }; bool 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_item_err= NULL); /* Collect arguments' character sets together. @@ -4232,6 +4239,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 char *name, Type_handler_hybrid_field_type *, From 569da6a7bab034fc9768af88d8dbc2a8e2944764 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 28 Nov 2023 15:50:41 +0200 Subject: [PATCH 04/11] MDEV-32899 InnoDB is holding shared dict_sys.latch while waiting for FOREIGN KEY child table lock on DDL lock_table_children(): A new function to lock all child tables of a table. We will only hold dict_sys.latch while traversing dict_table_t::referenced_set. To prevent a race condition with std::set::erase() we will copy the pointers to the child tables to a local vector. Once we have acquired references to all child tables, we can safely release dict_sys.latch, wait for the locks, and finally release the references. This fixes up commit 2ca112346438611ab7800b70bea6af1fd1169308 (MDEV-26217) and commit c3c53926c467c95386ae98d61ada87294bd61478 (MDEV-26554). --- storage/innobase/handler/ha_innodb.cc | 29 +++------------------- storage/innobase/handler/handler0alter.cc | 11 +-------- storage/innobase/include/lock0lock.h | 7 ++++++ storage/innobase/lock/lock0lock.cc | 30 +++++++++++++++++++++++ 4 files changed, 41 insertions(+), 36 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index d5cc2311a2b2a..ea4f85229b4cd 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -13561,14 +13561,7 @@ int ha_innobase::delete_table(const char *name) /* FOREIGN KEY constraints cannot exist on partitioned tables. */; #endif else - { - dict_sys.freeze(SRW_LOCK_CALL); - for (const dict_foreign_t* f : table->referenced_set) - if (dict_table_t* child= f->foreign_table) - if ((err= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS) - break; - dict_sys.unfreeze(); - } + err= lock_table_children(table, trx); } dict_table_t *table_stats= nullptr, *index_stats= nullptr; @@ -13966,14 +13959,7 @@ int ha_innobase::truncate() dict_table_t *table_stats = nullptr, *index_stats = nullptr; MDL_ticket *mdl_table = nullptr, *mdl_index = nullptr; - dberr_t error= DB_SUCCESS; - - dict_sys.freeze(SRW_LOCK_CALL); - for (const dict_foreign_t *f : ib_table->referenced_set) - if (dict_table_t *child= f->foreign_table) - if ((error= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS) - break; - dict_sys.unfreeze(); + dberr_t error= lock_table_children(ib_table, trx); if (error == DB_SUCCESS) error= lock_table_for_trx(ib_table, trx, LOCK_X); @@ -14164,16 +14150,7 @@ ha_innobase::rename_table( /* There is no need to lock any FOREIGN KEY child tables. */ } else if (dict_table_t *table = dict_table_open_on_name( norm_from, false, DICT_ERR_IGNORE_FK_NOKEY)) { - dict_sys.freeze(SRW_LOCK_CALL); - for (const dict_foreign_t* f : table->referenced_set) { - if (dict_table_t* child = f->foreign_table) { - error = lock_table_for_trx(child, trx, LOCK_X); - if (error != DB_SUCCESS) { - break; - } - } - } - dict_sys.unfreeze(); + error = lock_table_children(table, trx); if (error == DB_SUCCESS) { error = lock_table_for_trx(table, trx, LOCK_X); } diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index 092aa54737dce..9ce277362e441 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -11200,16 +11200,7 @@ ha_innobase::commit_inplace_alter_table( fts_optimize_remove_table(ctx->old_table); } - dict_sys.freeze(SRW_LOCK_CALL); - for (auto f : ctx->old_table->referenced_set) { - if (dict_table_t* child = f->foreign_table) { - error = lock_table_for_trx(child, trx, LOCK_X); - if (error != DB_SUCCESS) { - break; - } - } - } - dict_sys.unfreeze(); + error = lock_table_children(ctx->old_table, trx); if (ctx->new_table->fts) { ut_ad(!ctx->new_table->fts->add_wq); diff --git a/storage/innobase/include/lock0lock.h b/storage/innobase/include/lock0lock.h index 59ee7f551b4a1..655378599245b 100644 --- a/storage/innobase/include/lock0lock.h +++ b/storage/innobase/include/lock0lock.h @@ -438,6 +438,13 @@ dberr_t lock_table_for_trx(dict_table_t *table, trx_t *trx, lock_mode mode, bool no_wait= false) MY_ATTRIBUTE((nonnull, warn_unused_result)); +/** Lock the child tables of a table. +@param table parent table +@param trx transaction +@return error code */ +dberr_t lock_table_children(dict_table_t *table, trx_t *trx) + MY_ATTRIBUTE((nonnull, warn_unused_result)); + /** Exclusively lock the data dictionary tables. @param trx dictionary transaction @return error code diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index df51ceb16d818..150984157829b 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -3976,6 +3976,36 @@ dberr_t lock_table_for_trx(dict_table_t *table, trx_t *trx, lock_mode mode, return err; } +/** Lock the child tables of a table. +@param table parent table +@param trx transaction +@return error code */ +dberr_t lock_table_children(dict_table_t *table, trx_t *trx) +{ + dict_sys.freeze(SRW_LOCK_CALL); + std::vector children; + + for (auto f : table->referenced_set) + if (dict_table_t *child= f->foreign_table) + { + child->acquire(); + children.emplace_back(child); + } + dict_sys.unfreeze(); + + dberr_t err= DB_SUCCESS; + + for (auto child : children) + if ((err= lock_table_for_trx(child, trx, LOCK_X)) != DB_SUCCESS) + break; + + for (auto child : children) + child->release(); + + return err; +} + + /** Exclusively lock the data dictionary tables. @param trx dictionary transaction @return error code From 387b92df97e70680641ad0bcaed83b44373f13c5 Mon Sep 17 00:00:00 2001 From: Monty Date: Tue, 28 Nov 2023 15:31:02 +0200 Subject: [PATCH 05/11] Remove deprication from mariadbd --debug --debug is supported by allmost all our other binaries and we should keep it also in the server to keep option names similar. --- mysql-test/suite/innodb/r/innodb-blob.result | 6 ------ mysql-test/suite/innodb/r/innodb-index-debug.result | 2 -- mysql-test/suite/innodb/r/innodb-table-online.result | 6 ------ mysql-test/suite/innodb/r/page_reorganize.result | 2 -- .../suite/innodb_fts/r/ft_result_cache_limit.result | 2 -- mysql-test/suite/innodb_gis/r/check_rtree.result | 2 -- mysql-test/suite/innodb_gis/r/rollback.result | 2 -- mysql-test/suite/innodb_gis/r/rtree_compress.result | 2 -- mysql-test/suite/innodb_gis/r/rtree_create_inplace.result | 2 -- mysql-test/suite/rpl/r/rpl_semisync_ali_issues.result | 8 -------- mysql-test/suite/sys_vars/r/debug_basic.result | 2 -- mysql-test/suite/sys_vars/r/sysvars_debug.result | 2 +- sql/sys_vars.cc | 5 ++--- 13 files changed, 3 insertions(+), 40 deletions(-) diff --git a/mysql-test/suite/innodb/r/innodb-blob.result b/mysql-test/suite/innodb/r/innodb-blob.result index fdfbfe3e6832b..bbb071ccb3e3c 100644 --- a/mysql-test/suite/innodb/r/innodb-blob.result +++ b/mysql-test/suite/innodb/r/innodb-blob.result @@ -20,8 +20,6 @@ a RIGHT(b,20) 2 bbbbbbbbbbbbbbbbbbbb connection default; SET DEBUG='+d,row_ins_extern_checkpoint'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead SET DEBUG_SYNC='before_row_ins_extern_latch SIGNAL rec_not_blob WAIT_FOR crash'; ROLLBACK; BEGIN; @@ -88,8 +86,6 @@ BEGIN; INSERT INTO t2 VALUES (347); connection default; SET DEBUG='+d,row_upd_extern_checkpoint'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead SET DEBUG_SYNC='before_row_upd_extern SIGNAL have_latch WAIT_FOR crash'; UPDATE t3 SET c=REPEAT('i',3000) WHERE a=2; connection con2; @@ -126,8 +122,6 @@ BEGIN; INSERT INTO t2 VALUES (33101); connection default; SET DEBUG='+d,row_upd_extern_checkpoint'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead SET DEBUG_SYNC='after_row_upd_extern SIGNAL have_latch WAIT_FOR crash'; UPDATE t3 SET c=REPEAT('j',3000) WHERE a=2; connection con2; diff --git a/mysql-test/suite/innodb/r/innodb-index-debug.result b/mysql-test/suite/innodb/r/innodb-index-debug.result index bcd41392adbf3..8db3757dc9552 100644 --- a/mysql-test/suite/innodb/r/innodb-index-debug.result +++ b/mysql-test/suite/innodb/r/innodb-index-debug.result @@ -88,8 +88,6 @@ ALTER TABLE t1 FORCE, ADD COLUMN k4 int; connection default; SET DEBUG_SYNC= 'now WAIT_FOR opened'; SET debug = '+d,row_log_tmpfile_fail'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead INSERT INTO t1 select NULL,'aaa','bbb' from t480; INSERT INTO t1 select NULL,'aaaa','bbbb' from t480; SET DEBUG_SYNC= 'now SIGNAL flushed'; diff --git a/mysql-test/suite/innodb/r/innodb-table-online.result b/mysql-test/suite/innodb/r/innodb-table-online.result index 8078daad6336f..8f71ea83b9c1b 100644 --- a/mysql-test/suite/innodb/r/innodb-table-online.result +++ b/mysql-test/suite/innodb/r/innodb-table-online.result @@ -40,16 +40,10 @@ SET DEBUG_DBUG = '+d,innodb_OOM_prepare_inplace_alter'; ALTER TABLE t1 ROW_FORMAT=REDUNDANT, ALGORITHM=INPLACE, LOCK=NONE; ERROR HY000: Out of memory. SET SESSION DEBUG = @saved_debug_dbug; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead SET SESSION DEBUG = '+d,innodb_OOM_inplace_alter'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead ALTER TABLE t1 ROW_FORMAT=REDUNDANT, ALGORITHM=INPLACE, LOCK=NONE; ERROR HY000: Out of memory. SET SESSION DEBUG = @saved_debug_dbug; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead ALTER TABLE t1 ROW_FORMAT=REDUNDANT, ALGORITHM=INPLACE, LOCK=NONE; connection default; SHOW CREATE TABLE t1; diff --git a/mysql-test/suite/innodb/r/page_reorganize.result b/mysql-test/suite/innodb/r/page_reorganize.result index 20e1600bd0de1..fe85926e2b8c4 100644 --- a/mysql-test/suite/innodb/r/page_reorganize.result +++ b/mysql-test/suite/innodb/r/page_reorganize.result @@ -16,8 +16,6 @@ SET @save_dbug = @@debug_dbug; SET DEBUG_DBUG = '+d,do_page_reorganize,do_lock_reverse_page_reorganize'; insert into t1(f2) values (repeat('+', 100)); SET DEBUG = @save_dbug; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead commit; connection con1; f1 diff --git a/mysql-test/suite/innodb_fts/r/ft_result_cache_limit.result b/mysql-test/suite/innodb_fts/r/ft_result_cache_limit.result index 2dbdd5a04bc6c..74d60410d5841 100644 --- a/mysql-test/suite/innodb_fts/r/ft_result_cache_limit.result +++ b/mysql-test/suite/innodb_fts/r/ft_result_cache_limit.result @@ -20,8 +20,6 @@ END// CALL populate_t1; SET autocommit=1; SET SESSION debug="+d,fts_instrument_result_cache_limit"; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead ALTER TABLE t1 ADD FULLTEXT INDEX `text_content_idx` (`text_content`); SELECT FTS_DOC_ID, text_content FROM t1 diff --git a/mysql-test/suite/innodb_gis/r/check_rtree.result b/mysql-test/suite/innodb_gis/r/check_rtree.result index fe60a628fde52..0d7b25b6b7258 100644 --- a/mysql-test/suite/innodb_gis/r/check_rtree.result +++ b/mysql-test/suite/innodb_gis/r/check_rtree.result @@ -1,7 +1,5 @@ create table t1 (i int, g geometry not null, spatial index (g))engine=innodb; SET SESSION debug="+d,rtree_test_check_count"; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead insert into t1 values (1, POINT(1,1)); insert into t1 values (1, POINT(1.5,1.5)); insert into t1 values (1, POINT(3,3)); diff --git a/mysql-test/suite/innodb_gis/r/rollback.result b/mysql-test/suite/innodb_gis/r/rollback.result index 8688690b66a7b..9e0db79667e90 100644 --- a/mysql-test/suite/innodb_gis/r/rollback.result +++ b/mysql-test/suite/innodb_gis/r/rollback.result @@ -408,8 +408,6 @@ update t1 set a=point(5,5), b=point(5,5), c=5 where i < 3; ERROR 23000: Duplicate entry '5' for key 'c' rollback; set session debug="+d,row_mysql_crash_if_error"; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead update t1 set a=point(5,5), b=point(5,5), c=5 where i < 3; ERROR HY000: Lost connection to MySQL server during query insert into t1 values(5, point(5,5), point(5,5), 5); diff --git a/mysql-test/suite/innodb_gis/r/rtree_compress.result b/mysql-test/suite/innodb_gis/r/rtree_compress.result index a88f8b9fa9b67..360cf8ab5efe7 100644 --- a/mysql-test/suite/innodb_gis/r/rtree_compress.result +++ b/mysql-test/suite/innodb_gis/r/rtree_compress.result @@ -42,8 +42,6 @@ count(*) 0 SET @saved_dbug = @@SESSION.debug_dbug; SET DEBUG='+d,page_copy_rec_list_start_compress_fail'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead delete from t1; select count(*) from t1 where MBRWithin(t1.c2, @g1); count(*) diff --git a/mysql-test/suite/innodb_gis/r/rtree_create_inplace.result b/mysql-test/suite/innodb_gis/r/rtree_create_inplace.result index a8898f5c98f38..5279eea8aa82f 100644 --- a/mysql-test/suite/innodb_gis/r/rtree_create_inplace.result +++ b/mysql-test/suite/innodb_gis/r/rtree_create_inplace.result @@ -31,7 +31,5 @@ COUNT(*) 0 ALTER TABLE t1 DROP INDEX idx, ADD SPATIAL INDEX idx3(c2); SET SESSION debug="+d,row_merge_instrument_log_check_flush"; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead ALTER TABLE t1 DROP INDEX idx3, ADD SPATIAL INDEX idx4(c2), ADD SPATIAL INDEX idx5(c3); DROP TABLE t1; diff --git a/mysql-test/suite/rpl/r/rpl_semisync_ali_issues.result b/mysql-test/suite/rpl/r/rpl_semisync_ali_issues.result index 9607e8a799814..94ec9ceae8613 100644 --- a/mysql-test/suite/rpl/r/rpl_semisync_ali_issues.result +++ b/mysql-test/suite/rpl/r/rpl_semisync_ali_issues.result @@ -266,16 +266,12 @@ Variable_name Value Rpl_semi_sync_master_clients 1 # Test failure of select error . SET GLOBAL debug = 'd,rpl_semisync_simulate_select_error'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead INSERT INTO t1 VALUES(3); connection slave; connection con1; # Test failure of pthread_create SET GLOBAL rpl_semi_sync_master_enabled = 0; SET GLOBAL debug = 'd,rpl_semisync_simulate_create_thread_failure'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead SET GLOBAL rpl_semi_sync_master_enabled= ON; # Test failure of pthread_join SET GLOBAL rpl_semi_sync_master_enabled= OFF; @@ -283,8 +279,6 @@ SET GLOBAL rpl_semi_sync_master_enabled= OFF; # Failure on registering semisync slave # SET GLOBAL debug= 'd,rpl_semisync_simulate_add_slave_failure'; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead SET GLOBAL rpl_semi_sync_master_enabled= ON; connection slave; STOP SLAVE IO_THREAD; @@ -293,8 +287,6 @@ START SLAVE IO_THREAD; include/wait_for_slave_io_to_start.inc connection con1; SET GLOBAL debug=''; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead connection slave; START SLAVE IO_THREAD; include/wait_for_slave_io_to_start.inc diff --git a/mysql-test/suite/sys_vars/r/debug_basic.result b/mysql-test/suite/sys_vars/r/debug_basic.result index a97ad65b40f96..5a77446d0369f 100644 --- a/mysql-test/suite/sys_vars/r/debug_basic.result +++ b/mysql-test/suite/sys_vars/r/debug_basic.result @@ -1,6 +1,4 @@ set session debug="L"; -Warnings: -Warning 1287 '@@debug' is deprecated and will be removed in a future release. Please use '@@debug_dbug' instead select @@global.debug="1"; @@global.debug="1" 0 diff --git a/mysql-test/suite/sys_vars/r/sysvars_debug.result b/mysql-test/suite/sys_vars/r/sysvars_debug.result index 0d77b0211a1ea..c7357790e6b30 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_debug.result +++ b/mysql-test/suite/sys_vars/r/sysvars_debug.result @@ -38,7 +38,7 @@ GLOBAL_VALUE_ORIGIN COMPILE-TIME DEFAULT_VALUE VARIABLE_SCOPE SESSION VARIABLE_TYPE VARCHAR -VARIABLE_COMMENT Built-in DBUG debugger +VARIABLE_COMMENT Built-in DBUG debugger. Alias for --debug NUMERIC_MIN_VALUE NULL NUMERIC_MAX_VALUE NULL NUMERIC_BLOCK_SIZE NULL diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 081c489de5397..cd5e87bc54ce8 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -1002,11 +1002,10 @@ static Sys_var_charptr_fscs Sys_datadir( static Sys_var_dbug Sys_dbug( "debug", "Built-in DBUG debugger", sys_var::SESSION, CMD_LINE(OPT_ARG, '#'), DEFAULT(""), NO_MUTEX_GUARD, NOT_IN_BINLOG, - ON_CHECK(check_has_super), ON_UPDATE(0), - DEPRECATED("'@@debug_dbug'")); // since 5.5.37 + ON_CHECK(check_has_super)); static Sys_var_dbug Sys_debug_dbug( - "debug_dbug", "Built-in DBUG debugger", sys_var::SESSION, + "debug_dbug", "Built-in DBUG debugger. Alias for --debug", sys_var::SESSION, CMD_LINE(OPT_ARG, '#'), DEFAULT(""), NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(check_has_super)); #endif From ba6bf7ad9e52c1dc31a22a619c17e1bb55b46d5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 29 Nov 2023 10:48:10 +0200 Subject: [PATCH 06/11] MDEV-32899 instrumentation In debug builds, let us declare dict_sys.latch as index_lock instead of srw_lock, so that we will benefit from the full tracking of lock ownership. lock_table_for_trx(): Assert that the current thread is not holding dict_sys.latch. If the dict_sys.unfreeze() call were moved to the end of lock_table_children(), this assertion would fail in the test innodb.innodb and many other tests that use FOREIGN KEY. --- .../perfschema/r/sxlock_func,debug.rdiff | 22 ++++++++ .../suite/perfschema/t/sxlock_func.test | 1 + storage/innobase/dict/dict0dict.cc | 37 +++++++----- storage/innobase/handler/ha_innodb.cc | 8 ++- storage/innobase/include/dict0dict.h | 56 +++++++++---------- storage/innobase/lock/lock0lock.cc | 2 + 6 files changed, 84 insertions(+), 42 deletions(-) create mode 100644 mysql-test/suite/perfschema/r/sxlock_func,debug.rdiff diff --git a/mysql-test/suite/perfschema/r/sxlock_func,debug.rdiff b/mysql-test/suite/perfschema/r/sxlock_func,debug.rdiff new file mode 100644 index 0000000000000..0596810e55320 --- /dev/null +++ b/mysql-test/suite/perfschema/r/sxlock_func,debug.rdiff @@ -0,0 +1,22 @@ +@@ -7,7 +7,6 @@ + WHERE name LIKE 'wait/synch/rwlock/innodb/%' + AND name!='wait/synch/rwlock/innodb/btr_search_latch' ORDER BY name; + name +-wait/synch/rwlock/innodb/dict_operation_lock + wait/synch/rwlock/innodb/fil_space_latch + wait/synch/rwlock/innodb/lock_latch + wait/synch/rwlock/innodb/trx_i_s_cache_lock +@@ -19,11 +18,13 @@ + select name from performance_schema.setup_instruments + where name like "wait/synch/sxlock/%" order by name; + name ++wait/synch/sxlock/innodb/dict_operation_lock + wait/synch/sxlock/innodb/index_tree_rw_lock + SELECT DISTINCT name FROM performance_schema.rwlock_instances + WHERE name LIKE 'wait/synch/sxlock/innodb/%' + ORDER BY name; + name ++wait/synch/sxlock/innodb/dict_operation_lock + wait/synch/sxlock/innodb/index_tree_rw_lock + create table t1(a int) engine=innodb; + begin; diff --git a/mysql-test/suite/perfschema/t/sxlock_func.test b/mysql-test/suite/perfschema/t/sxlock_func.test index c43adc849d2a1..24d0e07ca418a 100644 --- a/mysql-test/suite/perfschema/t/sxlock_func.test +++ b/mysql-test/suite/perfschema/t/sxlock_func.test @@ -5,6 +5,7 @@ --source include/not_embedded.inc --source include/have_perfschema.inc --source include/have_innodb.inc +--source include/maybe_debug.inc UPDATE performance_schema.setup_instruments SET enabled = 'NO', timed = 'YES'; diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 0680e60d81c6a..25f72a974c31a 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -958,11 +958,12 @@ void dict_sys_t::lock_wait(SRW_LOCK_ARGS(const char *file, unsigned line)) if (latch_ex_wait_start.compare_exchange_strong (old, now, std::memory_order_relaxed, std::memory_order_relaxed)) { +#ifdef UNIV_DEBUG + latch.x_lock(SRW_LOCK_ARGS(file, line)); +#else latch.wr_lock(SRW_LOCK_ARGS(file, line)); +#endif latch_ex_wait_start.store(0, std::memory_order_relaxed); - ut_ad(!latch_readers); - ut_ad(!latch_ex); - ut_d(latch_ex= pthread_self()); return; } @@ -977,33 +978,39 @@ void dict_sys_t::lock_wait(SRW_LOCK_ARGS(const char *file, unsigned line)) if (waited > threshold / 4) ib::warn() << "A long wait (" << waited << " seconds) was observed for dict_sys.latch"; +#ifdef UNIV_DEBUG + latch.x_lock(SRW_LOCK_ARGS(file, line)); +#else latch.wr_lock(SRW_LOCK_ARGS(file, line)); - ut_ad(!latch_readers); - ut_ad(!latch_ex); - ut_d(latch_ex= pthread_self()); +#endif } #ifdef UNIV_PFS_RWLOCK ATTRIBUTE_NOINLINE void dict_sys_t::unlock() { - ut_ad(latch_ex == pthread_self()); - ut_ad(!latch_readers); - ut_d(latch_ex= 0); +# ifdef UNIV_DEBUG + latch.x_unlock(); +# else latch.wr_unlock(); +# endif } ATTRIBUTE_NOINLINE void dict_sys_t::freeze(const char *file, unsigned line) { +# ifdef UNIV_DEBUG + latch.s_lock(file, line); +# else latch.rd_lock(file, line); - ut_ad(!latch_ex); - ut_d(latch_readers++); +# endif } ATTRIBUTE_NOINLINE void dict_sys_t::unfreeze() { - ut_ad(!latch_ex); - ut_ad(latch_readers--); +# ifdef UNIV_DEBUG + latch.s_unlock(); +# else latch.rd_unlock(); +# endif } #endif /* UNIV_PFS_RWLOCK */ @@ -4533,7 +4540,11 @@ void dict_sys_t::close() temp_id_hash.free(); unlock(); +#ifdef UNIV_DEBUG + latch.free(); +#else latch.destroy(); +#endif mysql_mutex_destroy(&dict_foreign_err_mutex); diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index ea4f85229b4cd..b38b1218fb6e4 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -598,7 +598,13 @@ static PSI_rwlock_info all_innodb_rwlocks[] = # ifdef BTR_CUR_HASH_ADAPT { &btr_search_latch_key, "btr_search_latch", 0 }, # endif - { &dict_operation_lock_key, "dict_operation_lock", 0 }, + { &dict_operation_lock_key, "dict_operation_lock", +# ifdef UNIV_DEBUG + PSI_RWLOCK_FLAG_SX +# else + 0 +# endif + }, { &fil_space_latch_key, "fil_space_latch", 0 }, { &trx_i_s_cache_lock_key, "trx_i_s_cache_lock", 0 }, { &trx_purge_latch_key, "trx_purge_latch", 0 }, diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h index 895743be84bba..a0ba7d6ff5c5a 100644 --- a/storage/innobase/include/dict0dict.h +++ b/storage/innobase/include/dict0dict.h @@ -1316,14 +1316,14 @@ class dict_sys_t /** The my_hrtime_coarse().val of the oldest lock_wait() start, or 0 */ std::atomic latch_ex_wait_start; - /** the rw-latch protecting the data dictionary cache */ - alignas(CPU_LEVEL1_DCACHE_LINESIZE) srw_lock latch; #ifdef UNIV_DEBUG - /** whether latch is being held in exclusive mode (by any thread) */ - Atomic_relaxed latch_ex; - /** number of S-latch holders */ - Atomic_counter latch_readers; + typedef index_lock dict_lock; +#else + typedef srw_lock dict_lock; #endif + + /** the rw-latch protecting the data dictionary cache */ + alignas(CPU_LEVEL1_DCACHE_LINESIZE) dict_lock latch; public: /** Indexes of SYS_TABLE[] */ enum @@ -1491,15 +1491,12 @@ class dict_sys_t } #ifdef UNIV_DEBUG - /** @return whether any thread (not necessarily the current thread) - is holding the latch; that is, this check may return false - positives */ - bool frozen() const { return latch_readers || latch_ex; } - /** @return whether any thread (not necessarily the current thread) - is holding a shared latch */ - bool frozen_not_locked() const { return latch_readers; } + /** @return whether the current thread is holding the latch */ + bool frozen() const { return latch.have_any(); } + /** @return whether the current thread is holding a shared latch */ + bool frozen_not_locked() const { return latch.have_s(); } /** @return whether the current thread holds the exclusive latch */ - bool locked() const { return latch_ex == pthread_self(); } + bool locked() const { return latch.have_x(); } #endif private: /** Acquire the exclusive latch */ @@ -1514,13 +1511,11 @@ class dict_sys_t /** Exclusively lock the dictionary cache. */ void lock(SRW_LOCK_ARGS(const char *file, unsigned line)) { - if (latch.wr_lock_try()) - { - ut_ad(!latch_readers); - ut_ad(!latch_ex); - ut_d(latch_ex= pthread_self()); - } - else +#ifdef UNIV_DEBUG + if (!latch.x_lock_try()) +#else + if (!latch.wr_lock_try()) +#endif lock_wait(SRW_LOCK_ARGS(file, line)); } @@ -1535,24 +1530,29 @@ class dict_sys_t /** Unlock the data dictionary cache. */ void unlock() { - ut_ad(latch_ex == pthread_self()); - ut_ad(!latch_readers); - ut_d(latch_ex= 0); +# ifdef UNIV_DEBUG + latch.x_unlock(); +# else latch.wr_unlock(); +# endif } /** Acquire a shared lock on the dictionary cache. */ void freeze() { +# ifdef UNIV_DEBUG + latch.s_lock(); +# else latch.rd_lock(); - ut_ad(!latch_ex); - ut_d(latch_readers++); +# endif } /** Release a shared lock on the dictionary cache. */ void unfreeze() { - ut_ad(!latch_ex); - ut_ad(latch_readers--); +# ifdef UNIV_DEBUG + latch.s_unlock(); +# else latch.rd_unlock(); +# endif } #endif diff --git a/storage/innobase/lock/lock0lock.cc b/storage/innobase/lock/lock0lock.cc index 150984157829b..c9072998e662e 100644 --- a/storage/innobase/lock/lock0lock.cc +++ b/storage/innobase/lock/lock0lock.cc @@ -3940,6 +3940,8 @@ static void lock_table_dequeue(lock_t *in_lock, bool owns_wait_mutex) dberr_t lock_table_for_trx(dict_table_t *table, trx_t *trx, lock_mode mode, bool no_wait) { + ut_ad(!dict_sys.frozen()); + mem_heap_t *heap= mem_heap_create(512); sel_node_t *node= sel_node_create(heap); que_thr_t *thr= pars_complete_graph_for_exec(node, trx, heap, nullptr); From 968061fd9c7a1c501badb5bc82a9b7c3c44b6f6f Mon Sep 17 00:00:00 2001 From: Vlad Lesin Date: Mon, 27 Nov 2023 21:03:21 +0300 Subject: [PATCH 07/11] MDEV-28682 gcol.gcol_purge contaminates further execution of innodb.gap_locks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ha_innobase::extra() invokes check_trx_exists() unconditionally even for not supported operations. check_trx_exists() creates and registers trx_t object if THD does not contain pointer to it. If ha_innobase::extra() does not support some operation, it just invokes check_trx_exists() and quites. If check_trx_exists() creates and registers new trx_t object for such operation, it will never be freed and deregistered. For example, if ha_innobase::extra() is invoked from purge thread with operation = HA_EXTRA_IS_ATTACHED_CHILDREN, like it goes in gcol.gcol_purge test, trx_t object will be registered, but not deregisreted, and this causes innodb.gap_lock failure, as "SHOW ENGINE INNODB STATUS" shows information about unexpected transaction at the end of trx_sys.trx_list. The fix is not to invoke check_trx_exists() for unsupported operations in ha_innobase::extra(). Reviewed by: Marko Mäkelä --- storage/innobase/handler/ha_innodb.cc | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 58626c4667c55..d955b63c817ff 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -15412,29 +15412,33 @@ ha_innobase::extra( enum ha_extra_function operation) /*!< in: HA_EXTRA_FLUSH or some other flag */ { - check_trx_exists(ha_thd()); - /* Warning: since it is not sure that MySQL calls external_lock before calling this function, the trx field in m_prebuilt can be obsolete! */ + trx_t *trx; switch (operation) { case HA_EXTRA_FLUSH: + (void)check_trx_exists(ha_thd()); if (m_prebuilt->blob_heap) { row_mysql_prebuilt_free_blob_heap(m_prebuilt); } break; case HA_EXTRA_RESET_STATE: + trx = check_trx_exists(ha_thd()); reset_template(); - thd_to_trx(ha_thd())->duplicates = 0; + trx->duplicates = 0; break; case HA_EXTRA_NO_KEYREAD: + (void)check_trx_exists(ha_thd()); m_prebuilt->read_just_key = 0; break; case HA_EXTRA_KEYREAD: + (void)check_trx_exists(ha_thd()); m_prebuilt->read_just_key = 1; break; case HA_EXTRA_KEYREAD_PRESERVE_FIELDS: + (void)check_trx_exists(ha_thd()); m_prebuilt->keep_other_fields_on_keyread = 1; break; @@ -15445,18 +15449,23 @@ ha_innobase::extra( either, because the calling threads may change. CAREFUL HERE, OR MEMORY CORRUPTION MAY OCCUR! */ case HA_EXTRA_INSERT_WITH_UPDATE: - thd_to_trx(ha_thd())->duplicates |= TRX_DUP_IGNORE; + trx = check_trx_exists(ha_thd()); + trx->duplicates |= TRX_DUP_IGNORE; break; case HA_EXTRA_NO_IGNORE_DUP_KEY: - thd_to_trx(ha_thd())->duplicates &= ~TRX_DUP_IGNORE; + trx = check_trx_exists(ha_thd()); + trx->duplicates &= ~TRX_DUP_IGNORE; break; case HA_EXTRA_WRITE_CAN_REPLACE: - thd_to_trx(ha_thd())->duplicates |= TRX_DUP_REPLACE; + trx = check_trx_exists(ha_thd()); + trx->duplicates |= TRX_DUP_REPLACE; break; case HA_EXTRA_WRITE_CANNOT_REPLACE: - thd_to_trx(ha_thd())->duplicates &= ~TRX_DUP_REPLACE; + trx = check_trx_exists(ha_thd()); + trx->duplicates &= ~TRX_DUP_REPLACE; break; case HA_EXTRA_BEGIN_ALTER_COPY: + (void)check_trx_exists(ha_thd()); m_prebuilt->table->skip_alter_undo = 1; if (m_prebuilt->table->is_temporary() || !m_prebuilt->table->versioned_by_id()) { @@ -15470,6 +15479,7 @@ ha_innobase::extra( .first->second.set_versioned(0); break; case HA_EXTRA_END_ALTER_COPY: + (void)check_trx_exists(ha_thd()); m_prebuilt->table->skip_alter_undo = 0; break; default:/* Do nothing */ From 89a5a8d234832ef9ed5ee814e4db42c636fcde1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 30 Nov 2023 09:43:36 +0200 Subject: [PATCH 08/11] =?UTF-8?q?MDEV-32269=20InnoDB=20after=20ALTER=20TAB?= =?UTF-8?q?LE=E2=80=A6IMPORT=20TABLESPACE=20may=20not=20be=20crash=20safe?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mtr_t::commit(): If IMPORT TABLESPACE is first-time-dirtying blocks, acquire both log_sys.mutex and log_sys.flush_order_mutex to assign a valid m_commit_lsn so that the block will be inserted into the correct position of buf_pool.flush_list. This fixes occasional debug assertion failures when running the regression test suite. Reviewed by: Vladislav Lesin --- storage/innobase/mtr/mtr0mtr.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/storage/innobase/mtr/mtr0mtr.cc b/storage/innobase/mtr/mtr0mtr.cc index b138dd1307395..dd9036f9e085e 100644 --- a/storage/innobase/mtr/mtr0mtr.cc +++ b/storage/innobase/mtr/mtr0mtr.cc @@ -453,10 +453,16 @@ void mtr_t::commit() { ut_ad(m_log_mode == MTR_LOG_NO_REDO); ut_ad(m_log.size() == 0); - m_commit_lsn= log_sys.get_lsn(); - lsns= { m_commit_lsn, PAGE_FLUSH_NO }; if (UNIV_UNLIKELY(m_made_dirty)) /* This should be IMPORT TABLESPACE */ + { + mysql_mutex_lock(&log_sys.mutex); + m_commit_lsn= log_sys.get_lsn(); mysql_mutex_lock(&log_sys.flush_order_mutex); + mysql_mutex_unlock(&log_sys.mutex); + } + else + m_commit_lsn= log_sys.get_lsn(); + lsns= { m_commit_lsn, PAGE_FLUSH_NO }; } if (m_freed_pages) From 0ee9b119bf336a89b5667d43ce4edc8997394870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 30 Nov 2023 09:46:25 +0200 Subject: [PATCH 09/11] MDEV-31817 SIGSEGV after btr_page_get_father_block() returns nullptr on corrupted data btr_attach_half_pages(), btr_lift_page_up(), btr_compress(): Return DB_CORRUPTION if btr_page_get_father_block() returns nullptr Reviewed by: Thirunarayanan Balathandayuthapani --- storage/innobase/btr/btr0btr.cc | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc index ee2f8d00857c5..1e4e15a54339f 100644 --- a/storage/innobase/btr/btr0btr.cc +++ b/storage/innobase/btr/btr0btr.cc @@ -873,7 +873,8 @@ static rec_offs *btr_page_get_parent(rec_offs *offsets, mem_heap_t *heap, /************************************************************//** Returns the upper level node pointer to a page. It is assumed that mtr holds an x-latch on the tree. -@return rec_get_offsets() of the node pointer record */ +@return rec_get_offsets() of the node pointer record +@retval nullptr on corruption */ static rec_offs* btr_page_get_father_block( @@ -2541,6 +2542,11 @@ btr_attach_half_pages( offsets = btr_page_get_father_block(nullptr, heap, mtr, &cursor); + if (UNIV_UNLIKELY(!offsets)) { + mem_heap_free(heap); + return DB_CORRUPTION; + } + /* Replace the address of the old child node (= page) with the address of the new lower half */ @@ -3476,6 +3482,14 @@ btr_lift_page_up( offsets = btr_page_get_father_block(offsets, heap, mtr, &cursor); } + + if (UNIV_UNLIKELY(!offsets)) { +parent_corrupted: + mem_heap_free(heap); + *err = DB_CORRUPTION; + return nullptr; + } + father_block = btr_cur_get_block(&cursor); father_page_zip = buf_block_get_page_zip(father_block); @@ -3500,6 +3514,10 @@ btr_lift_page_up( &cursor); } + if (UNIV_UNLIKELY(!offsets)) { + goto parent_corrupted; + } + blocks[n_blocks++] = b = btr_cur_get_block(&cursor); } @@ -3715,6 +3733,11 @@ btr_compress( NULL, heap, mtr, &father_cursor); } + if (UNIV_UNLIKELY(!offsets)) { + err = DB_CORRUPTION; + goto func_exit; + } + if (adjust) { nth_rec = page_rec_get_n_recs_before(btr_cur_get_rec(cursor)); if (UNIV_UNLIKELY(!nth_rec || nth_rec == ULINT_UNDEFINED)) { From bb511def1d316ffbdd815f2fc99d0a5813671814 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 30 Nov 2023 10:35:53 +0200 Subject: [PATCH 10/11] MDEV-32371 Deadlock between buf_page_get_zip() and buf_pool_t::corrupted_evict() buf_page_get_zip(): Do not wait for the page latch while holding hash_lock. If the latch is not available, ensure that any concurrent buf_pool_t::corrupted_evict() will be able to acquire the hash_lock, and then retry the lookup. If the page was corrupted, we will finally "goto must_read_page", retry the read once more, and then report an error. Reviewed by: Thirunarayanan Balathandayuthapani --- storage/innobase/buf/buf0buf.cc | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 12a27f92d07a7..77b27d9b1ba1c 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -2235,14 +2235,21 @@ buf_page_t* buf_page_get_zip(const page_id_t page_id, ulint zip_size) if (discard_attempted || !bpage->frame) { - /* Even when we are holding a hash_lock, it should be - acceptable to wait for a page S-latch here, because - buf_page_t::read_complete() will not wait for buf_pool.mutex, - and because S-latch would not conflict with a U-latch - that would be protecting buf_page_t::write_complete(). */ - bpage->lock.s_lock(); + const bool got_s_latch= bpage->lock.s_lock_try(); hash_lock.unlock_shared(); - break; + if (UNIV_LIKELY(got_s_latch)) + break; + /* We may fail to acquire bpage->lock because + buf_page_t::read_complete() may be invoking + buf_pool_t::corrupted_evict() on this block, which it would + hold an exclusive latch on. + + Let us aqcuire and release buf_pool.mutex to ensure that any + buf_pool_t::corrupted_evict() will proceed before we reacquire + the hash_lock that it could be waiting for. */ + mysql_mutex_lock(&buf_pool.mutex); + mysql_mutex_unlock(&buf_pool.mutex); + goto lookup; } hash_lock.unlock_shared(); From 9d07b0520c65d8088a522e3d8b1292ad723dba15 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Fri, 6 Oct 2023 22:36:32 +0200 Subject: [PATCH 11/11] MDEV-31608 - Connector/NET fails to connect since 10.10 Connector/NET does not expect collation IDs returned by "show collations" to be NULL, runs into an exception. The fix is to determine connector/net using its connection attributes, then make sure "show collations" does not output NULL IDs. The patch introduces new old_mode NO_NULL_COLLATION_IDs, that is automatically set, once MySQL Connector/NET connection is determined. A test was added, that uses MySql.Data from powershell - only works if MySql.Data is installed into GAC (i.e with C/NET MSI package) --- mysql-test/main/mysql_connector_net.ps1 | 58 +++++++++++++++++++ mysql-test/main/mysql_connector_net.result | 2 + mysql-test/main/mysql_connector_net.test | 11 ++++ mysql-test/main/mysqld--help,win.rdiff | 20 ++++--- mysql-test/main/mysqld--help.result | 3 +- mysql-test/main/old-mode.result | 10 ++++ mysql-test/main/old-mode.test | 8 +++ .../suite/sys_vars/r/old_mode_basic.result | 4 +- .../sys_vars/r/sysvars_server_embedded.result | 2 +- .../r/sysvars_server_notembedded.result | 2 +- .../suite/sys_vars/t/old_mode_basic.test | 2 +- sql/sql_acl.cc | 41 +++++++++++-- sql/sql_class.h | 1 + sql/sql_show.cc | 3 +- sql/sys_vars.cc | 1 + 15 files changed, 148 insertions(+), 20 deletions(-) create mode 100644 mysql-test/main/mysql_connector_net.ps1 create mode 100644 mysql-test/main/mysql_connector_net.result create mode 100644 mysql-test/main/mysql_connector_net.test diff --git a/mysql-test/main/mysql_connector_net.ps1 b/mysql-test/main/mysql_connector_net.ps1 new file mode 100644 index 0000000000000..159acf9361c86 --- /dev/null +++ b/mysql-test/main/mysql_connector_net.ps1 @@ -0,0 +1,58 @@ +$assembly = [system.reflection.Assembly]::LoadWithPartialName("MySql.Data") +if ($assembly -eq $null) +{ + "Can't load assembly MySql.Data" + exit 100 +} + +try +{ + $connectionString =[string]::Format("server=127.0.0.1;uid=root;port={0};Connection Reset=true;",$Env:MASTER_MYPORT) + $connection = [MySql.Data.MySqlClient.MySqlConnection]@{ConnectionString=$connectionString} + $connection.Open() + + # Test ExecuteReader() + $command = New-Object MySql.Data.MySqlClient.MySqlCommand + $command.Connection = $connection + $command.CommandText = "SELECT @@old_mode" + $reader = $command.ExecuteReader() + $reader.GetName(0) + while ($reader.Read()) + { + $reader.GetValue(0) + } + + # Test connection reset + $connection.Close() + $connection.Open() + # Test ExecuteNonQuery() + $command.CommandText="do 1"; + $affected_rows = $command.ExecuteNonQuery() + if ($affected_rows -ne 0) + { + "Expected affected rows 0, actual $affected_rows" + exit 1 + } + # Test Prepared Statement + $command.CommandText = "SELECT @var"; + [void]$command.Parameters.AddWithValue("@var", 1); + $command.Prepare(); + $out = $command.ExecuteScalar(); + if ($out -ne 1) + { + "Expected output 1, actual $out" + exit 1 + } + $connection.Close() +} +catch +{ + # Dump exception + $_ + $inner = $PSItem.Exception.InnerException + if ($inner -ne $null) + { + $PSItem.Exception.InnerException.Message + $PSItem.Exception.InnerException.StackTrace + } +} diff --git a/mysql-test/main/mysql_connector_net.result b/mysql-test/main/mysql_connector_net.result new file mode 100644 index 0000000000000..f2fa39df3e70f --- /dev/null +++ b/mysql-test/main/mysql_connector_net.result @@ -0,0 +1,2 @@ +@@old_mode +UTF8_IS_UTF8MB3,NO_NULL_COLLATION_IDS diff --git a/mysql-test/main/mysql_connector_net.test b/mysql-test/main/mysql_connector_net.test new file mode 100644 index 0000000000000..c1dce65adc807 --- /dev/null +++ b/mysql-test/main/mysql_connector_net.test @@ -0,0 +1,11 @@ +--source include/windows.inc +let $sys_errno=0; + +# Error 100 is returned by the powershell script +# if MySql.Data is not installed +--error 0,100 +--exec powershell -NoLogo -NoProfile -File main\mysql_connector_net.ps1 +if ($sys_errno != 0) +{ + --skip Connector/NET is not installed +} diff --git a/mysql-test/main/mysqld--help,win.rdiff b/mysql-test/main/mysqld--help,win.rdiff index 3e9541d7d2fac..a42c0c6f81f05 100644 --- a/mysql-test/main/mysqld--help,win.rdiff +++ b/mysql-test/main/mysqld--help,win.rdiff @@ -1,4 +1,6 @@ -@@ -180,6 +180,7 @@ +--- main/mysqld--help.result 2023-11-30 02:21:51.951132200 +0100 ++++ main/mysqld--help,win.reject 2023-11-30 02:35:44.404612300 +0100 +@@ -191,6 +191,7 @@ --console Write error output on screen; don't remove the console window on windows. --core-file Write core on crashes @@ -6,7 +8,7 @@ -h, --datadir=name Path to the database root directory --date-format=name The DATE format (ignored) --datetime-format=name -@@ -650,6 +651,7 @@ +@@ -696,6 +697,7 @@ Use MySQL-5.6 (instead of MariaDB-5.3) format for TIME, DATETIME, TIMESTAMP columns. (Defaults to on; use --skip-mysql56-temporal-format to disable.) @@ -14,7 +16,7 @@ --net-buffer-length=# Buffer length for TCP/IP and socket communication --net-read-timeout=# -@@ -1327,6 +1328,10 @@ +@@ -1351,6 +1353,10 @@ Alias for log_slow_query_file. Log slow queries to given log file. Defaults logging to 'hostname'-slow.log. Must be enabled to activate other slow log options @@ -25,7 +27,7 @@ --socket=name Socket file to use for connection --sort-buffer-size=# Each thread that needs to do a sort allocates a buffer of -@@ -1351,6 +1356,7 @@ +@@ -1376,6 +1382,7 @@ deleting or updating every row in a table. --stack-trace Print a symbolic stack trace on failure (Defaults to on; use --skip-stack-trace to disable.) @@ -33,7 +35,7 @@ --standard-compliant-cte Allow only CTEs compliant to SQL standard (Defaults to on; use --skip-standard-compliant-cte to disable.) -@@ -1426,6 +1432,11 @@ +@@ -1454,6 +1461,11 @@ --thread-pool-max-threads=# Maximum allowed number of worker threads in the thread pool @@ -45,7 +47,7 @@ --thread-pool-oversubscribe=# How many additional active worker threads in a group are allowed. -@@ -1464,8 +1475,8 @@ +@@ -1493,8 +1505,8 @@ automatically convert it to an on-disk MyISAM or Aria table. -t, --tmpdir=name Path for temporary files. Several paths may be specified, @@ -56,7 +58,7 @@ --transaction-alloc-block-size=# Allocation block size for transactions to be stored in binary log -@@ -1685,6 +1696,7 @@ +@@ -1716,6 +1728,7 @@ myisam-stats-method NULLS_UNEQUAL myisam-use-mmap FALSE mysql56-temporal-format TRUE @@ -64,7 +66,7 @@ net-buffer-length 16384 net-read-timeout 30 net-retry-count 10 -@@ -1841,6 +1853,7 @@ +@@ -1874,6 +1887,7 @@ slave-type-conversions slow-launch-time 2 slow-query-log FALSE @@ -72,7 +74,7 @@ sort-buffer-size 2097152 sql-mode STRICT_TRANS_TABLES,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION sql-safe-updates FALSE -@@ -1867,6 +1880,8 @@ +@@ -1901,6 +1915,8 @@ thread-pool-exact-stats FALSE thread-pool-idle-timeout 60 thread-pool-max-threads 65536 diff --git a/mysql-test/main/mysqld--help.result b/mysql-test/main/mysqld--help.result index d271f1a308d00..867d5cd3c5783 100644 --- a/mysql-test/main/mysqld--help.result +++ b/mysql-test/main/mysqld--help.result @@ -721,7 +721,8 @@ The following specify which files/extra groups are read (specified before remain MySQL versions. Any combination of: NO_DUP_KEY_WARNINGS_WITH_IGNORE, NO_PROGRESS_INFO, ZERO_DATE_TIME_CAST, UTF8_IS_UTF8MB3, - IGNORE_INDEX_ONLY_FOR_JOIN, COMPAT_5_1_CHECKSUM + IGNORE_INDEX_ONLY_FOR_JOIN, COMPAT_5_1_CHECKSUM, + NO_NULL_COLLATION_IDS Use 'ALL' to set all combinations. --old-passwords Use old password encryption method (needed for 4.0 and older clients) diff --git a/mysql-test/main/old-mode.result b/mysql-test/main/old-mode.result index daa2a4dc915ab..cb87c45ada8e6 100644 --- a/mysql-test/main/old-mode.result +++ b/mysql-test/main/old-mode.result @@ -257,3 +257,13 @@ Warning 1264 Out of range value for column 'a' at row 2 DROP TABLE t1; SET @@time_zone=DEFAULT; SET TIMESTAMP=DEFAULT; +# +# MDEV-31608 - Connector/NET fails to connect since 10.10 +# +select count(*) > 0 from information_schema.collations where id IS NULL; +count(*) > 0 +1 +SET old_mode=no_null_collation_ids; +select count(*) > 0 from information_schema.collations where id IS NULL; +count(*) > 0 +0 diff --git a/mysql-test/main/old-mode.test b/mysql-test/main/old-mode.test index e4928329b47e9..177e00edce8bd 100644 --- a/mysql-test/main/old-mode.test +++ b/mysql-test/main/old-mode.test @@ -169,3 +169,11 @@ DROP TABLE t1; SET @@time_zone=DEFAULT; SET TIMESTAMP=DEFAULT; + +--echo # +--echo # MDEV-31608 - Connector/NET fails to connect since 10.10 +--echo # +select count(*) > 0 from information_schema.collations where id IS NULL; +SET old_mode=no_null_collation_ids; +select count(*) > 0 from information_schema.collations where id IS NULL; + diff --git a/mysql-test/suite/sys_vars/r/old_mode_basic.result b/mysql-test/suite/sys_vars/r/old_mode_basic.result index 252316dc1cb9b..776d45a1fe3c6 100644 --- a/mysql-test/suite/sys_vars/r/old_mode_basic.result +++ b/mysql-test/suite/sys_vars/r/old_mode_basic.result @@ -114,8 +114,8 @@ SET @@global.old_mode = 4; SELECT @@global.old_mode; @@global.old_mode ZERO_DATE_TIME_CAST -SET @@global.old_mode = 64; -ERROR 42000: Variable 'old_mode' can't be set to the value of '64' +SET @@global.old_mode = 128; +ERROR 42000: Variable 'old_mode' can't be set to the value of '128' SELECT @@global.old_mode; @@global.old_mode ZERO_DATE_TIME_CAST diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result index 48eb32e969871..ac8752ac82e3f 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_embedded.result @@ -2299,7 +2299,7 @@ VARIABLE_COMMENT Used to emulate old behavior from earlier MariaDB or MySQL vers NUMERIC_MIN_VALUE NULL NUMERIC_MAX_VALUE NULL NUMERIC_BLOCK_SIZE NULL -ENUM_VALUE_LIST NO_DUP_KEY_WARNINGS_WITH_IGNORE,NO_PROGRESS_INFO,ZERO_DATE_TIME_CAST,UTF8_IS_UTF8MB3,IGNORE_INDEX_ONLY_FOR_JOIN,COMPAT_5_1_CHECKSUM +ENUM_VALUE_LIST NO_DUP_KEY_WARNINGS_WITH_IGNORE,NO_PROGRESS_INFO,ZERO_DATE_TIME_CAST,UTF8_IS_UTF8MB3,IGNORE_INDEX_ONLY_FOR_JOIN,COMPAT_5_1_CHECKSUM,NO_NULL_COLLATION_IDS READ_ONLY NO COMMAND_LINE_ARGUMENT REQUIRED VARIABLE_NAME OLD_PASSWORDS diff --git a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result index 6cb556308a87b..1f600a8a7180a 100644 --- a/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result +++ b/mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result @@ -2469,7 +2469,7 @@ VARIABLE_COMMENT Used to emulate old behavior from earlier MariaDB or MySQL vers NUMERIC_MIN_VALUE NULL NUMERIC_MAX_VALUE NULL NUMERIC_BLOCK_SIZE NULL -ENUM_VALUE_LIST NO_DUP_KEY_WARNINGS_WITH_IGNORE,NO_PROGRESS_INFO,ZERO_DATE_TIME_CAST,UTF8_IS_UTF8MB3,IGNORE_INDEX_ONLY_FOR_JOIN,COMPAT_5_1_CHECKSUM +ENUM_VALUE_LIST NO_DUP_KEY_WARNINGS_WITH_IGNORE,NO_PROGRESS_INFO,ZERO_DATE_TIME_CAST,UTF8_IS_UTF8MB3,IGNORE_INDEX_ONLY_FOR_JOIN,COMPAT_5_1_CHECKSUM,NO_NULL_COLLATION_IDS READ_ONLY NO COMMAND_LINE_ARGUMENT REQUIRED VARIABLE_NAME OLD_PASSWORDS diff --git a/mysql-test/suite/sys_vars/t/old_mode_basic.test b/mysql-test/suite/sys_vars/t/old_mode_basic.test index 631d638767f8a..cb18796729e5f 100644 --- a/mysql-test/suite/sys_vars/t/old_mode_basic.test +++ b/mysql-test/suite/sys_vars/t/old_mode_basic.test @@ -172,7 +172,7 @@ SET @@global.old_mode = 4; SELECT @@global.old_mode; --Error ER_WRONG_VALUE_FOR_VAR -SET @@global.old_mode = 64; +SET @@global.old_mode = 128; SELECT @@global.old_mode; # use of decimal values diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 6b6e1c30da148..ebd2949a64ec1 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -13576,8 +13576,37 @@ static bool find_mpvio_user(MPVIO_EXT *mpvio) DBUG_RETURN(0); } + +/** + Determine if the client is MySQL Connector/NET. + + Checks whether the given connection attributes blob corresponds to + MySQL Connector/NET by examining the "_client_name" attribute, which is + expected to be the first attribute in the blob. + + @param connection_attrs - The connection attributes blob. + @param length - The length of the blob. + + @return true if the client is MySQL Connector/NET, false otherwise. +*/ +static inline bool is_connector_net_client(const char *connection_attrs, + size_t length) +{ + constexpr LEX_CSTRING prefix= + {STRING_WITH_LEN("\x0c_client_name\x13mysql-connector-net")}; + + if (length < prefix.length) + return false; + + /* Optimization to avoid following memcmp in common cases.*/ + if (connection_attrs[prefix.length - 1] != prefix.str[prefix.length - 1]) + return false; + + return !memcmp(connection_attrs, prefix.str, prefix.length); +} + static bool -read_client_connect_attrs(char **ptr, char *end, CHARSET_INFO *from_cs) +read_client_connect_attrs(char **ptr, char *end, THD* thd) { ulonglong length; char *ptr_save= *ptr; @@ -13600,10 +13629,14 @@ read_client_connect_attrs(char **ptr, char *end, CHARSET_INFO *from_cs) if (length > 65535) return true; - if (PSI_CALL_set_thread_connect_attrs(*ptr, (uint)length, from_cs) && + if (PSI_CALL_set_thread_connect_attrs(*ptr, (uint)length, thd->charset()) && current_thd->variables.log_warnings) sql_print_warning("Connection attributes of length %llu were truncated", length); + + /* Connector/Net crashes, when "show collations" returns NULL IDs*/ + if (is_connector_net_client(*ptr, length)) + thd->variables.old_behavior |= OLD_MODE_NO_NULL_COLLATION_IDS; return false; } @@ -13737,7 +13770,7 @@ static bool parse_com_change_user_packet(MPVIO_EXT *mpvio, uint packet_length) } if ((thd->client_capabilities & CLIENT_CONNECT_ATTRS) && - read_client_connect_attrs(&next_field, end, thd->charset())) + read_client_connect_attrs(&next_field, end, thd)) { my_message(ER_UNKNOWN_COM_ERROR, ER_THD(thd, ER_UNKNOWN_COM_ERROR), MYF(0)); @@ -13987,7 +14020,7 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio, if ((thd->client_capabilities & CLIENT_CONNECT_ATTRS) && read_client_connect_attrs(&next_field, ((char *)net->read_pos) + pkt_len, - mpvio->auth_info.thd->charset())) + mpvio->auth_info.thd)) return packet_error; /* diff --git a/sql/sql_class.h b/sql/sql_class.h index beb33d8394e2e..b53de8d55c02a 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -201,6 +201,7 @@ enum enum_binlog_row_image { #define OLD_MODE_UTF8_IS_UTF8MB3 (1 << 3) #define OLD_MODE_IGNORE_INDEX_ONLY_FOR_JOIN (1 << 4) #define OLD_MODE_COMPAT_5_1_CHECKSUM (1 << 5) +#define OLD_MODE_NO_NULL_COLLATION_IDS (1 << 6) extern char internal_table_name[2]; extern char empty_c_string[1]; diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 32b29468c32a9..c21043aaba227 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -6398,7 +6398,8 @@ int fill_schema_collation(THD *thd, TABLE_LIST *tables, COND *cond) tmp_cl->get_collation_name(MY_COLLATION_NAME_MODE_CONTEXT); LEX_CSTRING full_collation_name= tmp_cl->get_collation_name(MY_COLLATION_NAME_MODE_FULL); - bool is_context= cmp(context_collation_name, full_collation_name); + bool is_context= cmp(context_collation_name, full_collation_name) && + !(thd->variables.old_behavior & OLD_MODE_NO_NULL_COLLATION_IDS); /* Some collations are applicable to multiple character sets. Display them only once, with the short name (without the diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc index 0319298a9d7f9..23f1ca3a2591c 100644 --- a/sql/sys_vars.cc +++ b/sql/sys_vars.cc @@ -3886,6 +3886,7 @@ static const char *old_mode_names[]= "UTF8_IS_UTF8MB3", "IGNORE_INDEX_ONLY_FOR_JOIN", "COMPAT_5_1_CHECKSUM", + "NO_NULL_COLLATION_IDS", 0 };