- developers - lists.mariadb.org

Re: [Maria-developers] fcf2c5fff14: MDEV-26299: Some views force server (and mysqldump) to generate invalid SQL for their definitions
by Sergei Golubchik 16 Oct '21

16 Oct '21

Hi, Oleksandr! On Oct 16, Oleksandr Byelkin wrote: > revision-id: fcf2c5fff14 (mariadb-10.2.40-78-gfcf2c5fff14) > parent(s): d5a15f04f4a > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2021-10-01 14:46:22 +0200 > message: > > MDEV-26299: Some views force server (and mysqldump) to generate invalid SQL for their definitions > > Do not print illegal table field names for non-top-level SELECT list, > they will not be refered in any case but create problem for parsing > of printed result. > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index ff584e936b7..ebee9c85791 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -25797,6 +25797,11 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) > > //Item List > bool first= 1; > + /* > + outer_select() can not be used here because it is for name resolution > + and will return NULL at any end of name resolution chain (view/derived) > + */ > + bool top_level= (get_master()->get_master() == 0); > List_iterator_fast<Item> it(item_list); > Item *item; > while ((item= it++)) > @@ -25806,7 +25811,8 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) > else > str->append(','); > > - if (is_subquery_function() && item->is_autogenerated_name) > + if ((is_subquery_function() && item->is_autogenerated_name) || > + !item->name) Where can item->name be NULL? I thought the either specified by the user or autogenerated, but never NULL. > { > /* > Do not print auto-generated aliases in subqueries. It has no purpose > @@ -25815,7 +25821,26 @@ void st_select_lex::print(THD *thd, String *str, enum_query_type query_type) > item->print(str, query_type); > } > else > - item->print_item_w_name(str, query_type); > + { > + /* > + Despite presence of item->name_length it is incorrect in many cases > + till we start using LEX_STRING for the name. > + */ > + size_t len= strlen(item->name); > + > + /* > + Do not print illegal names (if it is not top level SELECT). > + Top level view checked (and correct name are assigned), > + other cases of top level SELECT are not important, because > + it is not "table field". > + */ > + if (top_level || > + !item->is_autogenerated_name || > + !check_table_name(item->name, len, TRUE)) why check_table_name() if it isn't a table? it's an incorrect check to use for columns. > + item->print_item_w_name(str, query_type); > + else > + item->print(str, query_type); > + } > } > > /* > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] baaaa971662: MDEV-22883: Assertion `!parse_error || lex.sphead == 0' failed in Table_triggers_list::check_n_load | SIGSEGV in strxmov
by Sergei Golubchik 15 Oct '21

15 Oct '21

Hi, Oleksandr! On Oct 15, Oleksandr Byelkin wrote: > revision-id: baaaa971662 (mariadb-10.2.40-61-gbaaaa971662) > parent(s): dce490e9d4d > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2021-09-19 19:23:49 +0200 > message: > > MDEV-22883: Assertion `!parse_error || lex.sphead == 0' failed in Table_triggers_list::check_n_load | SIGSEGV in strxmov > > Abort execution of SHOW in case of fatal error. Does this patch fixes all test cases from the bug report? There are quite a few. > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 721bb053343..105b2719882 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -4849,7 +4849,8 @@ static int fill_schema_table_from_frm(THD *thd, TABLE *table, > */ > DBUG_ASSERT(thd->open_tables == NULL); > thd->mdl_context.rollback_to_savepoint(open_tables_state_backup->mdl_system_tables_svp); > - thd->clear_error(); > + if (!thd->is_fatal_error) > + thd->clear_error(); Hmm. THD::clear_error() starts with a comment: /** Clear the current error, if any. We do not clear is_fatal_error or is_fatal_sub_stmt_error since we assume this is never called if the fatal error is set. Unfortunately, it doesn't have an assert to go with this, otherwise all these bugs would've probably failed identically and early. Please add DBUG_ASSERT(!is_fatal_error); DBUG_ASSERT(!is_fatal_sub_stmt_error); or something like that to THD::clear_error() > return res; > } > > @@ -5064,7 +5065,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > continue; > } > > - if (thd->killed == ABORT_QUERY) > + if (thd->killed == ABORT_QUERY || thd->is_fatal_error) > { > error= 0; > goto err; that's kind of weird. Why does it continue executing if the statement is killed? Perhaps it'd better be if (thd->killed || thd->is_fatal_error) { if (thd->killed == ABORT_QUERY) error= 0; goto err; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 2f812e800c0: MDEV-22447: SIGSEGV in Item::val_uint on EXPLAIN
by Sergei Golubchik 15 Oct '21

15 Oct '21

Hi, Oleksandr! On Oct 15, Oleksandr Byelkin wrote: > revision-id: 2f812e800c0 (mariadb-10.2.40-57-g2f812e800c0) > parent(s): 696de6d06c0 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2021-09-16 14:12:48 +0200 > message: > > MDEV-22447: SIGSEGV in Item::val_uint on EXPLAIN > > take into account possible NULL > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index ff584e936b7..d1b738e94a0 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -389,7 +389,7 @@ bool handle_select(THD *thd, LEX *lex, select_result *result, > ER_QUERY_EXCEEDED_ROWS_EXAMINED_LIMIT, > ER_THD(thd, ER_QUERY_EXCEEDED_ROWS_EXAMINED_LIMIT), > thd->accessed_rows_and_keys, > - thd->lex->limit_rows_examined->val_uint()); > + thd->lex->get_limit_rows_examined()); > thd->abort_on_warning= saved_abort_on_warning; > thd->reset_killed(); > } How comes, it gets into ER_QUERY_EXCEEDED_ROWS_EXAMINED_LIMIT if limit_rows_examined is NULL? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] cd4cac2b95a: MDEV-17964: Assertion `status == 0' failed in add_role_user_mapping_action
by Sergei Golubchik 15 Oct '21

15 Oct '21

Hi, Vicențiu! The patch is ok, please push. But given the time it took us to understand the issue, I'm afraid that simply stating that "HASH implementation shuffles elements around" is not enough. Better to explain how elements are shuffled around. For example: in this case the hash has 131 element, DROP ROLE removes the element [119]. Its [119]->next was element [129], so [129] is moved to [119]. Now we need to compact the hash, removing the last element [130]. It gets one bit off its hash value and becomes element [2]. The existing element [2] is moved to [129], and old [130] is moved to [2]. We cannot simply move [130] to [129] and make [2]->next=130, it won't work if [2] is itself in the collision list and doesn't belong in [2]. On Oct 15, Vicențiu Ciorbaru wrote: > revision-id: cd4cac2b95a (mariadb-10.2.40-91-gcd4cac2b95a) > parent(s): 2bb8d7c2f36 > author: Vicențiu Ciorbaru > committer: Vicențiu Ciorbaru > timestamp: 2021-10-13 13:28:50 +0300 > message: > > MDEV-17964: Assertion `status == 0' failed in add_role_user_mapping_action > > This happens upon CREATE USER and DROP ROLE. > > The underlying problem is that our HASH implementation shuffles > elements around when performing an update or delete. This means that > when doing a scan through the HASH table by index, in search of > elements to delete or update one must restart the scan to make sure > nothing is missed if at least one delete / update happened. > > The handle_grant_struct code assumed that it is safe to continue by > only reexamining the currently modified / deleted element index, but > that is not true. > > Missing to delete an element in the hash triggered the assertion in > the test case. DROP ROLE would not clear all necessary role->role or > role->user mappings. > > To fix the problem we ensure that the scan is restarted, only if an > element was deleted / updated, similar to how bubble-sort keeps > sorting until it finds no more elements to swap. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 97458a7767b: MDEV-26832: ROW_NUMBER in SIGNAL/RESIGNAL causes a syntax error
by Sergei Golubchik 15 Oct '21

15 Oct '21

Hi, Rucha! Looks quite good. A couple of small comments, see below. After you fix them, no need to ask for a new review, just push into bb-10.7-row_number On Oct 15, Rucha Deodhar wrote: > revision-id: 97458a7767b (mariadb-10.6.1-131-g97458a7767b) > parent(s): a6cf8b34a83 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-10-15 01:54:51 +0530 > message: > > MDEV-26832: ROW_NUMBER in SIGNAL/RESIGNAL causes a syntax error > > Analysis: Parser was missing ROW_NUMBER as syntax for SIGNAL and RESIGNAL. > Fix: Fix parser and fix how m_row_number is copied like other attributes > to avoid ROW_NUMBER from assuming default value. > > diff --git a/mysql-test/main/get_diagnostics.test b/mysql-test/main/get_diagnostics.test > index 83ea1dee343..186d1bfc438 100644 > --- a/mysql-test/main/get_diagnostics.test > +++ b/mysql-test/main/get_diagnostics.test > @@ -1552,3 +1552,53 @@ GET DIAGNOSTICS CONDITION 2 @rnum = ROW_NUMBER, @msg = MESSAGE_TEXT, @err = MYSQ ... > + > +CREATE PROCEDURE resignal_syntax() > +BEGIN > + DECLARE CONTINUE HANDLER > + FOR 1146 > + BEGIN > + RESIGNAL SET > + MESSAGE_TEXT = '`temptab` does not exist', ROW_NUMBER= 105; > + END; > + SELECT `c` FROM `temptab`; > +END| > + > +DELIMITER ;| > + > +--error ER_NO_SUCH_TABLE > +CALL resignal_syntax(); You test that the above isn't a syntax error. But you also need to verify that row_number was, indeed, set. Like, do GET DIAGNOSTICS and print the ROW_NUMBER. > + > +DROP PROCEDURE resignal_syntax; > diff --git a/sql/sql_error.cc b/sql/sql_error.cc > index a2178831697..84065ddce15 100644 > --- a/sql/sql_error.cc > +++ b/sql/sql_error.cc > @@ -190,6 +190,11 @@ static void copy_string(MEM_ROOT *mem_root, String* dst, const String* src) > dst->length(0); > } > > +void copy_value(ulong *dst, const ulong *src) > +{ > + memcpy(dst, src, sizeof(*src)); > +} > + > void > Sql_condition::copy_opt_attributes(const Sql_condition *cond) > { > @@ -204,6 +209,7 @@ Sql_condition::copy_opt_attributes(const Sql_condition *cond) > copy_string(m_mem_root, & m_table_name, & cond->m_table_name); > copy_string(m_mem_root, & m_column_name, & cond->m_column_name); > copy_string(m_mem_root, & m_cursor_name, & cond->m_cursor_name); > + copy_value(& m_row_number, & cond->m_row_number); That makes no sense. Other values were strings and had to be strdup-ed, but that's a number, you can simply do m_row_number= cond->m_row_number; > } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 6ec64caec5d: MDEV-26650: Failed ALTER USER/GRANT statement removes the password from the cache
by Sergei Golubchik 15 Oct '21

15 Oct '21

Hi, Oleksandr! All good, ok to push. Just add a comment right after memcpy, that /* if acl_user->nauth was >= nauth, then we've just updated the actual acl_user, not the copy. We rely on the fact that acl_update_user() cannot fail anywhere below. */ On Oct 15, Oleksandr Byelkin wrote: > revision-id: 6ec64caec5d (mariadb-10.4.21-64-g6ec64caec5d) > parent(s): a736a3174a4 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2021-10-14 16:19:09 +0200 > message: > > MDEV-26650: Failed ALTER USER/GRANT statement removes the password from the cache > > Starting from 10.4 AUTH is not part of ACL_USER so changes have to be done > over a copy, and bring in the cache only in case of success. > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-26742 Assertion `field->type_handler() == this' failed...
by Sergei Golubchik 12 Oct '21

12 Oct '21

Hi, Alexander! See comments below > diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test b/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test > index dd6049abbf3..3f3a6fdac2c 100644 > --- a/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test > +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6_innodb.test > @@ -12,6 +12,30 @@ > SET default_storage_engine=InnoDB; > --source type_inet6_engines.inc > > +--echo # > +--echo # MDEV-26742 Assertion `field->type_handler() == this' failed in FixedBinTypeBundle<NATIVE_LEN, MAX_CHAR_LEN>::Type_handler_fbt::stored_field_cmp_to_item > +--echo # > + > +CREATE TABLE t1 (pk inet6, c text) engine=myisam; > +INSERT INTO t1 VALUES ('::',1); > +CREATE TABLE t2 (d text, KEY (d)) engine=innodb ; > +INSERT INTO t2 VALUES (2); > +--error ER_TRUNCATED_WRONG_VALUE > +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; > +SET sql_mode=''; > +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; > +SET sql_mode=DEFAULT; > +SELECT * FROM t1; > +SELECT * FROM t2; > +DROP TABLE t1, t2; > + > +CREATE TABLE t1 (pk inet6, c text) engine=myisam; > +INSERT INTO t1 VALUES ('::',1); > +CREATE TABLE t2 ( d text, KEY (d)) engine=innodb ; I don't get it, why do you recreate tables with exactly the same definition and exactly the same content? > +INSERT INTO t2 VALUES (2); > +SELECT * FROM t2 JOIN t1 ON ( t1.pk > t2.d); > +DROP TABLE t1, t2; > + > > --echo # > --echo # End of 10.5 tests > diff --git a/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test b/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test > index c5183f01cf0..1ccd654181c 100644 > --- a/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test > +++ b/plugin/type_inet/mysql-test/type_inet/type_inet6_myisam.test > @@ -10,6 +10,21 @@ > SET default_storage_engine=MyISAM; > --source type_inet6_engines.inc > > +--echo # > +--echo # MDEV-26742 Assertion `field->type_handler() == this' failed in FixedBinTypeBundle<NATIVE_LEN, MAX_CHAR_LEN>::Type_handler_fbt::stored_field_cmp_to_item > +--echo # > + > +CREATE TABLE t1 (c varchar(64), key(c)) engine=myisam; > +INSERT INTO t1 VALUES ('::1'),('::2'); > +SELECT * FROM t1 WHERE c>CAST('::1' AS INET6); > +DROP TABLE t1; > + > +CREATE TABLE t1 (c varchar(64), key(c)) engine=myisam; > +INSERT INTO t1 VALUES ('::1'),('0::1'); also don't see much point in recreating > +SELECT * FROM t1 WHERE c=CAST('::1' AS INET6); > +EXPLAIN SELECT * FROM t1 WHERE c=CAST('::1' AS INET6); > +DROP TABLE t1; > + > > --echo # > --echo # End of 10.5 tests > diff --git a/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test b/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test > index 062e25d366b..27e3214af15 100644 > --- a/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test > +++ b/plugin/type_uuid/mysql-test/type_uuid/type_uuid_innodb.test > @@ -12,5 +12,27 @@ SET default_storage_engine=InnoDB; > --source type_uuid_engines.inc > > --echo # > ---echo # End of 10.5 tests > +--echo # MDEV-26742 Assertion `field->type_handler() == this' failed in FixedBinTypeBundle<NATIVE_LEN, MAX_CHAR_LEN>::Type_handler_fbt::stored_field_cmp_to_item > +--echo # > + > +CREATE TABLE t1 ( pk uuid, c text) engine=myisam; > +INSERT INTO t1 VALUES ('00000000-0000-0000-0000-000000000000',1); > +CREATE TABLE t2 ( d text, KEY (d)) engine=innodb ; > +INSERT INTO t2 VALUES (2); > +--error ER_TRUNCATED_WRONG_VALUE > +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; > +SET sql_mode=''; > +UPDATE t2 JOIN t1 ON ( t1.pk > t2.d) SET t1.c = 1; > +SET sql_mode=DEFAULT; > +DROP TABLE t1, t2; > + > +CREATE TABLE t1 (pk uuid, c text) engine=myisam; > +INSERT INTO t1 VALUES ('00000000-0000-0000-0000-000000000000',1); > +CREATE TABLE t2 ( d text, KEY (d)) engine=innodb ; > +INSERT INTO t2 VALUES (2); same here > +SELECT * FROM t2 JOIN t1 ON ( t1.pk > t2.d); > +DROP TABLE t1, t2; > + > +--echo # > +--echo # End of 10.7 tests > --echo # > diff --git a/sql/field.cc b/sql/field.cc > index 46a3a1deea3..fc3456e2ccb 100644 > --- a/sql/field.cc > +++ b/sql/field.cc > @@ -7368,7 +7368,9 @@ bool > Field_longstr::cmp_to_string_with_same_collation(const Item_bool_func *cond, > const Item *item) const > { > + Type_handler_hybrid_field_type cmp(type_handler_for_comparison()); Can this be anything else but type_handler_long_blob ? if not, then I'd suggest Type_handler_hybrid_field_type cmp(type_handler_long_blob); DBUG_ASSERT(type_handler_for_comparison() == &type_handler_long_blob); > + return !cmp.aggregate_for_comparison(item->type_handler_for_comparison()) && > + cmp.type_handler() == type_handler_for_comparison() && > - return item->cmp_type() == STRING_RESULT && > charset() == cond->compare_collation(); This, basically, means, that cmp_type() is now meaningless, one cannot use it anymore? There's no INET6_RESULT and UUID_RESULT for comparison, so what's the point of cmp_type() now? One more question. What type do we have where you need to aggregate here and simple item->type_handler_for_comparison() != &type_handler_long_blob won't do? > } > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 4

[Maria-developers] How can I separate networking test from mysql_test
by Lukas Javorsky 11 Oct '21

11 Oct '21

Hello there, I've open this topic on the maria-discuss mailing list, but, I didn't get any response. Maybe this mailing list could be a better fit for this request. > Hi guys, > I've found few particular tests from the test suite that is failing due to the OpenSSL-3 version and I want to separate them and give them to the OpenSSL team in Red Hat. > > The idea behind this is to give them a set of commands/config options (reproducer) for each test and they will try to find out what's the exact problem there and help us develop the patch. > > The tests that I'm interested in are (there are more of them, I just need to see how to reproduce few of them and I'll do the same for the other ones as well): > encryption.corrupted_during_recovery > encryption.innodb-checksum-algorithm > encryption.innodb-discard-import-change > encryption.innodb_page_encryption_key_change > binlog_encryption.rpl_loaddata_local > binlog_encryption.rpl_parallel_free_deferred_event > > IMPORTANT: These tests are failing only with the 'cbc' cipher in use, the 'ctr' is passing. > > Could you please help me with how to make a reproducer for each test, so I can create them and give them to the OpenSSL team? > > Thank you for any help you can provide > Lukas -- S pozdravom/ Best regards Lukáš Javorský Associate Software Engineer, Core service - Databases Red Hat <https://www.redhat.com> Purkyňova 115 (TPB-C) 612 00 Brno - Královo Pole ljavorsk(a)redhat.com <https://www.redhat.com>

1 0

Re: [Maria-developers] dcbdec60893: MDEV-26650: Failed ALTER USER/GRANT statement removes the password from the cache
by Sergei Golubchik 07 Oct '21

07 Oct '21

Hi, Oleksandr! Ok to push! A couple of typos below On Oct 07, Oleksandr Byelkin wrote: > revision-id: dcbdec60893 (mariadb-10.4.21-56-gdcbdec60893) > parent(s): a75813d4673 > author: Oleksandr Byelkin > committer: Oleksandr Byelkin > timestamp: 2021-10-07 11:39:02 +0200 > message: > > MDEV-26650: Failed ALTER USER/GRANT statement removes the password from the cache > > Starting from 10.4 AUTH is not part of ACL_USER so have to be safely copied. > Patch made in line with preriouse code ideas. "previous" > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 0e2c044e3a1..cef51038540 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -188,6 +188,9 @@ class ACL_USER_PARAM > class ACL_USER :public ACL_USER_BASE, > public ACL_USER_PARAM > { > +protected: > + // default shellow copy constructor (protect from non-authorized usage) "shallow" > + ACL_USER(const ACL_USER &)= default; > public: Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] INSTEAD OF triggers
by Alexandre hadjinlian guerra 05 Oct '21

05 Oct '21

Hello Im looking forward to implement this feature in MariaDB, and will look after trigger definition code/modules in MariaDB, but any wisdom that could be shared to speed up my efforts would be greatly appreciated. The idea is to provide the same functionality as describe here https://aws.amazon.com/pt/blogs/database/implementing-table-partitioning-in… Thanks

3 2