- developers - lists.mariadb.org

Re: [Maria-developers] Review for MDEV-19751.
by Aleksey Midenkov 14 May '20

14 May '20

Hi, Alexey! Actually the bug is applicable only to primary key and only when default field list is set (i.e. empty KEY() clause). So updated the patch accordingly. Please peek again. On Wed, May 13, 2020 at 2:40 PM Alexey Botchkov <holyfoot(a)mariadb.com> wrote: > > Hello, Aleksey! > > > commit 5ef168e00f7b8c821463e46535a4beccb47ec8ea > + if (key_info->flags & HA_INVISIBLE_KEY) > + continue; > > I'd add || !(key_info->flags & HA_NOSAME). > Can the non-unique index participate in the KEY partitioning? > > + for (uint kp= 0; kp < key_info->user_defined_key_parts; ++kp) > + { > + const KEY_PART_INFO &key_part= key_info->key_part[kp]; > + for (Field **part_field= tab_part_info->part_field_array; > + *part_field; ++part_field) > + { > + if (*part_field == key_part.field) > + { > + *partition_changed= TRUE; > + goto search_finished; > + } > + } // for (part_field) > + } // for (key_part) > > So you check if the key has any of the partitioning fields in it. > In this case we are going to disallow INPLACE with no reason if the > unrelated key is removed just containing the primary key column. > I'd say we should check for the exact match, when the key has > all the part_field_array in that exact order. Disagree? > > And for the test - please add the case for a table with no PRIMARY KEY, bug with the UNIQUE instead. > > Best regards! > HF > -- All the best, Aleksey Midenkov @midenok

1 0

[Maria-developers] Progress Report - Week 1
by Mohammed Hammaad Mateen 12 May '20

12 May '20

Greetings, Hope you are safe and doing great, This post describes the things I've done in my first week [ 4-10 May ] of Community Bonding Period under the mentor-ship of Sergei Golubchik and Oleksandr Byelkin for GSoC-20 and I must acknowledge that it was really an interactive experience and at the same time informative as well. Things I've done include: 1: *Raising my first ever pull request for MariaDB under MDEV-14558 *: Learnt about *strnxmov*, a function of C++ which I had never used earlier. I also came across the procedure of testing my implemented code. Wrote some test cases for the same and generated their corresponding results. 2:* Understand the working of the functions under multi_delete*: Ran it in the debugger and tried to familiarize myself with the internals of how Multiple Table Delete actually works. Things to-do: 1: Though I've raised the pull request, MDEV-14558 is not completed tested as it does not include tests for *mysql \s*, moreover, test cases have to be refactored. 2: Learn more about the working of *multi_delete* so as to develop a parser for Multiple Table Delete Returning. 3: Write some test cases for Multiple Table Delete Returning before hand. Regards, Mohammed Hammaad Mateen

2 1

[Maria-developers] Review for: MDEV-17399 Add support for JSON_TABLE, part #3
by Sergey Petrunia 10 May '20

10 May '20

Hi Alexey, Please find the next batch of input below. (I haven't finished the review but I'm sending it now so you can start addressing it) > commit 68ca18518337443090bdeddf6d612129b6843c21 > Author: Alexey Botchkov <holyfoot(a)askmonty.org> > Date: Fri Apr 17 14:42:40 2020 +0400 > > MDEV-17399 Add support for JSON_TABLE. > > Syntax for JSON_TABLE added. > The ha_json_table handler added. Executing the JSON_TABLE we > create the temporary table of the ha_json_table, add dependencies > of other tables and sometimes conditions to WHERE. > ... > +public: > + ha_json_table(TABLE_SHARE *share_arg, Table_function_json_table *jt): > + handler(&table_function_hton.m_hton, share_arg), m_jt(jt) > + { > + mark_trx_read_write_done= 1; This definitely needs a comment. > + ~ha_json_table() {} > + handler *clone(const char *name, MEM_ROOT *mem_root) { return NULL; } > + const char *index_type(uint inx) { return "HASH"; } Why is ha_json_table still trying to pretend it supports indexes? Is this a left-over from the old way of interfacing with the optimizer that can now be removed? > +int Table_function_json_table::setup(THD *thd, TABLE_LIST *sql_table, > + COND **cond) > +{ > + if (m_json->fix_fields(thd, &m_json)) > + return TRUE; > + > + m_dep_tables= m_json->used_tables(); Why does this function have a 'cond' argument? Please remove it if it's not necessary. == Duplicate column names are allowed == select * from JSON_TABLE('{"color": "black", "price": 1000}', "$" COLUMNS (rowSeq FOR ORDINALITY, color VARCHAR(100) PATH '$.color', color VARCHAR(100) PATH '$.price') ) as T; +--------+-------+-------+ | rowSeq | color | color | +--------+-------+-------+ | 0 | black | 1000 | +--------+-------+-------+ Is it really okay that duplicate column names are allowed? == RIGHT JOIN is allowed == create table t1 (item_name varchar(32), item_props varchar(1024)); insert into t1 values ('Laptop', '{"color": "black", "price": 1000}'); insert into t1 values ('Jeans', '{"color": "blue", "price": 50}'); This query should not be allowed, because JSON_TABLE has a dependency on an inner side of an outer join: select * from t1 right join json_table(t1.item_props, '$' columns(color varchar(100) path '$.color', f2 for ordinality) ) as T on 1; === NATURAL JOIN crashes == create table t10 (color varchar(100), f2 int); select * from t10 natural join json_table(t1.item_props, '$' columns(color varchar(100) path '$.color', f2 for ordinality) ) as JT ; Thread 31 "mysqld" received signal SIGSEGV, Segmentation fault. 0x0000555555d8ac12 in Select_limit_counters::get_select_limit (this=0xa5a5a5a5a5a5ad2d) at /home/psergey/dev-git/10.5-json/sql/sql_limit.h:67 (gdb) wher #0 0x0000555555d8ac12 in Select_limit_counters::get_select_limit (this=0xa5a5a5a5a5a5ad2d) at /home/psergey/dev-git/10.5-json/sql/sql_limit.h:67 #1 0x0000555555e2a4a0 in JOIN::optimize_inner (this=0x7fffac018520) at /home/psergey/dev-git/10.5-json/sql/sql_select.cc:1805 #2 0x0000555555e29b12 in JOIN::optimize (this=0x7fffac018520) at /home/psergey/dev-git/10.5-json/sql/sql_select.cc:1612 BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 1

[Maria-developers] OSX fix need on MariaDB-10.5
by Daniel Black 09 May '20

09 May '20

Mac lovers (who can use C/C++ and a debugger), As your MariaDB developers are hard at work getting 10.5 ready for you, unfortunately a bug has slipped into the 10.5 branch making it unusable on OSX (at least that's what it looks like on the CI infrastructure). The bug report is here: https://jira.mariadb.org/browse/MDEV-22173 As the time of developers is a little short as is the availability of OSX hardware, I'm hoping some of you are able to spend a few hours tackling this task to understand it better, and if possible, create patch. Steps: 1. build environment https://mariadb.com/kb/en/Build_Environment_Setup_for_Mac/ 2. clone the source git clone --branch 10.5 https://github.com/MariaDB/server.git mariadb-server cd mariadb-server git submodule update --recursive --init 3. build instructions https://mariadb.com/kb/en/generic-build-instructions/ (you don't need to install) 4. test (from build directory) mysql-test/mysql-test-run main.select might be useful here: --debugger=NAME or --gdb client-debugger=NAME or client-gdb To collaborative share your findings and to ask for further help MariaDB's community chat stream has been created for you. https://mariadb.zulipchat.com/#narrow/stream/118759-general/topic/Get.20OSX… To paraphrase J.F.Kennedy ask not what your database server community can do for you — ask what you can do for your database server community. Keep the community spirit going, Daniel Black -- Sent as a community member who noticed OSX failures and that everyone is busy

2 3

Re: [Maria-developers] 4312f382b6f: MDEV-22312: Bad error message for SET DEFAULT ROLE when user account is not granted the role
by Sergei Golubchik 08 May '20

08 May '20

Hi, Anel! On May 08, Anel Husakovic wrote: > revision-id: 4312f382b6f (mariadb-10.1.43-138-g4312f382b6f) > parent(s): 8c4b5261210 > author: Anel Husakovic <anel(a)mariadb.org> > committer: Anel Husakovic <anel(a)mariadb.org> > timestamp: 2020-05-07 16:49:33 +0200 > message: > > MDEV-22312: Bad error message for SET DEFAULT ROLE when user account is not granted the role > > diff --git a/mysql-test/suite/roles/set_role-recursive.test b/mysql-test/suite/roles/set_role-recursive.test > index 23d623e1966..dcf3dd5b6fc 100644 > --- a/mysql-test/suite/roles/set_role-recursive.test > +++ b/mysql-test/suite/roles/set_role-recursive.test > @@ -44,7 +44,7 @@ select * from mysql.roles_mapping; > > --sorted_result > show grants; > ---error ER_INVALID_ROLE > +--error ER_SET_DEFAULT_ROLE_FOR_USER > set role test_role2; > select current_user(), current_role(); > --sorted_result > diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt > index c5e83062f0f..83ecd4735e8 100644 > --- a/sql/share/errmsg-utf8.txt > +++ b/sql/share/errmsg-utf8.txt > @@ -7144,3 +7144,5 @@ ER_WARN_AGGFUNC_DEPENDENCE > ukr "Агрегатна функція '%-.192s)' з SELECTу #%d належить до SELECTу #%d" > WARN_INNODB_PARTITION_OPTION_IGNORED > eng "<%-.64s> option ignored for InnoDB partition" > +ER_SET_DEFAULT_ROLE_FOR_USER > + eng "User '%s' has not been granted a role '%s'." Sorry, this is an absolute no-no. We cannot add new error messages to the old GA versions. Only to the very last RC/GA or to any alpha/beta. When you add an error message, all error messages added after it in newer versions will shift, change their numbers. And error numbers in RC/GA versions cannot change. Instead of a new error message and my_error(ER_SET_DEFAULT_ROLE_FOR_USER, MYF(0), c_usr.ptr(), rolename); you have to write my_printf_error(ER_INVALID_ROLE, "User %`s has not been granted a role %`s", MYF(0), c_usr.c_ptr(), rolename); note other changes: * %`s instead of '%s' for correct identifier quoting * no dot at the end * c_ptr() because printf expects a zero-terminated C string, and ptr() doesn't guarantee that. > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index b6a6f806e50..37141dd2390 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2041,6 +2041,9 @@ static int check_user_can_set_role(const char *user, const char *host, > ACL_USER_BASE *acl_user_base; > ACL_USER *UNINIT_VAR(acl_user); > bool is_granted= FALSE; > + bool user_can_see_role= FALSE; > + char buf[1024]; > + String c_usr(buf, sizeof(buf), &my_charset_bin); use StringBuffer<1024> c_str; > int result= 0; > > /* clear role privileges */ > @@ -2083,14 +2086,39 @@ static int check_user_can_set_role(const char *user, const char *host, > is_granted= TRUE; > break; > } > + > + for (uint i=0; i < acl_user->role_grants.elements; i++) > + { > + ACL_ROLE *r= *(dynamic_element(&acl_user->role_grants, i, ACL_ROLE**)); > + if (!strcmp(safe_str(r->user.str), safe_str(rolename))) > + { > + size_t len= acl_user->user.length + acl_user->hostname_length + 2; > + c_usr.alloc(len); > + const char c= '@'; > + strmov(strmov(strmov(const_cast<char*>(c_usr.ptr()), > + safe_str(user)), &c), safe_str(host)); > + user_can_see_role= TRUE; > + } > + } No, you repeat this for() block for evert role grant of every role's grantee. And you only use it when there was an error, an uncommon case. Do not do the work that you aren't going to use. And the check doesn't seem correct. You want to use the same rule as for applicable_roles, you need to traverse the graph. Perhaps you can simply use check_role_is_granted() though. And also you need to check whether the current user has read access on mysql.user table - in that case it can see all roles, granted or not. > } > > /* According to SQL standard, the same error message must be presented */ this comment now belongs inside the if(), above ER_INVALID_ROLE > if (!is_granted) > { > - my_error(ER_INVALID_ROLE, MYF(0), rolename); > - result= 1; > - goto end; > + /* Handling of the bad error message - MDEV-22312 */ don't put MDEV numbers into the code, please. > + /* If the current user can see the role generate different type of error. */ > + if(user_can_see_role) > + { > + my_error(ER_SET_DEFAULT_ROLE_FOR_USER, MYF(0), c_usr.ptr(), rolename); > + result= 1; > + goto end; > + } > + else > + { > + my_error(ER_INVALID_ROLE, MYF(0), rolename); > + result= 1; > + goto end; > + } > } > > if (access) Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Welcome to Google Summer of Code 2020 with MariaDB
by Vicențiu Ciorbaru 04 May '20

04 May '20

Hi! GSoC 2020 accepted proposals have just been announced and you are part of the successful applicants. This year we had a lot of very good proposals and so we have a one of the highest number of students (7) to mentor for GSoC. Congratulations on the effort to learn about the program, get involved and submit successful proposals! The fun part begins now! If you have not done so already, please reach out to your mentor and seek further advice about your project. Each one of your projects can be implemented in multiple ways, some more complex than others. I strongly advise you to create a concrete plan based off of your proposal with your mentor and aim to stick to it. The MariaDB Server codebase is a complex one with lots of different parts, however your mentor can guide you to ensure you have a successful GSoC. Remember that you are here to learn about Open Source, what it means to be part of an Open Source community, to interact with the community, all while becoming better programmers. Please take advantage of this opportunity and keep your technical discussions public, on Zulip https://mariadb.zulipchat.com, and on our developer mailing list https://launchpad.net/~maria-developers. By keeping your discussions public, you get the benefit of not only having your mentor see your questions but all of the MariaDB community. You never know when someone might lend a hand in case your mentor is not yet online. :) The mantra of our community should be that we are always welcoming to new people. Do not be afraid to ask questions. There are no bad or stupid or silly questions. The only key requirement is that you first make an effort to find the answer yourself and only afterwards seek outside help. If you can show a bit of involvement, the community will show you the same. I recommend that you agree with your mentor on some form of weekly progress reporting, via either a blog-post, an email, or something similar that can be publicly shared. I can not stress this enough: we are working on an open source project, our work must be done in the open and it is important to *show the world the good we are doing*. Hope all of you will have a great GSoC! Vicențiu MariaDB Foundation GSoC Admin

1 0

Re: [Maria-developers] d08860b28f3: MDEV-22374: VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE
by Sergei Golubchik 04 May '20

04 May '20

Hi, Oleksandr! On May 04, Oleksandr Byelkin wrote: > revision-id: d08860b28f3 (mariadb-5.5.67-15-gd08860b28f3) > parent(s): ac2604f923f > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-04-28 09:16:33 +0200 > message: > > MDEV-22374: VIEW with security definer require FILE privilege from definer not invoker in case of INTO OUTFILE > > Check INTO OUTFILE clause always from invoker. > > --- > mysql-test/r/view_grant.result | 21 +++++++++++++++++++++ > mysql-test/t/view_grant.test | 33 +++++++++++++++++++++++++++++++++ > sql/sql_parse.cc | 18 +++++++++--------- > 3 files changed, 63 insertions(+), 9 deletions(-) > > diff --git a/mysql-test/r/view_grant.result b/mysql-test/r/view_grant.result > index b2d3a0b8ca4..37dc6adc978 100644 > --- a/mysql-test/r/view_grant.result > +++ b/mysql-test/r/view_grant.result > @@ -1587,3 +1587,24 @@ USE test; > DROP DATABASE mysqltest1; > DROP USER 'mysqluser1'@'%'; > DROP USER 'mysqluser2'@'%'; > +# > +# MDEV-22374: VIEW with security definer require FILE privilege from definer > +# not invoker in case of INTO OUTFILE > +# > +create user test@localhost; > +grant select on test.* to test@localhost; > +create table t1 (a int); > +create definer=test@localhost sql security definer view v1 as select * from t1; > +# check that ot works without view typo "it" > +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1; > +# check that ot works without file same why "//" ? > +select * from v1; > +a > +# rights for file should be taken from current user not view > +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1; > +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1; > +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1; same > +drop view v1; please add reverse tests too: +create sql security definer view v1 as select * from t1; +connect(con1, localhost, test); +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from t1; +select * from v1; +error 1045; +select * INTO OUTFILE '../../tmp/test_out_txt' from (select count(*) from v1) as dv1; +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from (select * from v1) as dv1; +error 1045; +select * INTO OUTFILE '../..//tmp/test_out_txt' from v1; +connection default; > +drop table t1; > +drop user test@localhost; > +# End of 5.5 tests > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index ae5a6b4cd35..515990c879f 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -2206,15 +2206,15 @@ mysql_execute_command(THD *thd) > lex->exchange != NULL implies SELECT .. INTO OUTFILE and this > requires FILE_ACL access. > */ > - ulong privileges_requested= lex->exchange ? SELECT_ACL | FILE_ACL : > - SELECT_ACL; > - > - if (all_tables) > - res= check_table_access(thd, > - privileges_requested, > - all_tables, FALSE, UINT_MAX, FALSE); > - else > - res= check_access(thd, privileges_requested, any_db, NULL, NULL, 0, 0); > + if (lex->exchange) > + res= check_access(thd, FILE_ACL, any_db, NULL, NULL, 0, 0); > + if (!res) > + { > + if (all_tables) > + res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, UINT_MAX, FALSE); > + else > + res= check_access(thd, SELECT_ACL, any_db, NULL, NULL, 0, 0); > + } No, I think this special check for a special SELECT ... INTO OUTFILE case is wrong. Why does the server even checks global privileges via views? It should not do it, I think, not for FILE not for any other global or db level privilege. I'd try something like diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -4699,7 +4699,7 @@ bool check_grant(THD *thd, ulong want_access, TABLE_LIST > Save a copy of the privileges without the SHOW_VIEW_ACL attribute. It will be checked during making view. */ - tl->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + tl->grant.orig_want_privilege= want_access & ~SHOW_VIEW_ACL & TABLE_ACLS; } mysql_rwlock_rdlock(&LOCK_grant); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5204,7 +5204,8 @@ check_table_access(THD *thd, ulong requirements,TABLE_LIS> Register access for view underlying table. Remove SHOW_VIEW_ACL, because it will be checked during making view */ - table_ref->grant.orig_want_privilege= (want_access & ~SHOW_VIEW_ACL); + table_ref->grant.orig_want_privilege= + want_access & ~SHOW_VIEW_ACL & TABLE_ACLS; if (table_ref->schema_table_reformed) { Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 3ead2cea95c: MDEV-13266: Race condition in ANALYZE TABLE / statistics collection
by Sergey Petrunia 04 May '20

04 May '20

Hi Varun, On Sun, Apr 12, 2020 at 09:08:37PM +0530, Varun wrote: > revision-id: 3ead2cea95c32b7ceaf6e6ec81f7afbd9137cfe9 (mariadb-10.2.31-103-g3ead2cea95c) > parent(s): d565895bbd8e2a163be48b9bac51fccbf3949c80 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2020-04-12 21:05:36 +0530 > message: > > MDEV-13266: Race condition in ANALYZE TABLE / statistics collection > > Fixing a race condition while collecting the engine independent statistics. > The issue here was when the statistics was collected on specific indexes > then because of some race condition the statistics for indexes was not > collected. The TABLE::keys_in_use_for_query was set to 0 in such cases. > > This happens when the table is opened from TABLE_SHARE instead of the > table share stored in table_cache. It would be nice to write down the race condition that we've caught (in case we need this info later) Is my understanding correct: the error scenario is as follows: Thread1> start running "ANALYZE TABLE t PERISTENT FOR COLUMNS (..) INDEXES ($list)" Thread1> Walk through $list and save it in TABLE::keys_in_use_for_query Thread1> Close/re-open tables Thread2> Make some use of table t. This involves taking table t from Thread2> the table cache, and putting it back (with TABLE::keys_in_use_for_query reset to 0) Thread1> continue collecting EITS stats. Since TABLE::keys_in_use_for_query we will not collect statistics for indexes in $list. Please confirm (and if not, describe the race condition). The patch also introduces this behaviour: analyze table ... persistent for ... indexes(no_such_index); will now cause engine statistics to be still collected. Before the patch it exited with an error. But then, this is consistent with what happens for analyze table ... persistent for columns(no_such_column) ... So I guess this is ok. > --- > sql/sql_admin.cc | 46 +++++++++++++++++++++------------------------- > 1 file changed, 21 insertions(+), 25 deletions(-) > > diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc > index ab95fdc340c..0613495f202 100644 > --- a/sql/sql_admin.cc > +++ b/sql/sql_admin.cc > @@ -769,31 +769,6 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, > (table->table->s->table_category == TABLE_CATEGORY_USER && > (get_use_stat_tables_mode(thd) > NEVER || > lex->with_persistent_for_clause)); > - > - > - if (!lex->index_list) > - { > - tab->keys_in_use_for_query.init(tab->s->keys); > - } > - else > - { > - int pos; > - LEX_STRING *index_name; > - List_iterator_fast<LEX_STRING> it(*lex->index_list); > - > - tab->keys_in_use_for_query.clear_all(); > - while ((index_name= it++)) > - { > - if (tab->s->keynames.type_names == 0 || > - (pos= find_type(&tab->s->keynames, index_name->str, > - index_name->length, 1)) <= 0) > - { > - compl_result_code= result_code= HA_ADMIN_INVALID; > - break; > - } > - tab->keys_in_use_for_query.set_bit(--pos); > - } > - } > } > > if (result_code == HA_ADMIN_OK) > @@ -878,6 +853,27 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, > } > tab->file->column_bitmaps_signal(); > } > + if (!lex->index_list) > + tab->keys_in_use_for_query.init(tab->s->keys); > + else > + { > + int pos; > + LEX_STRING *index_name; > + List_iterator_fast<LEX_STRING> it(*lex->index_list); > + > + tab->keys_in_use_for_query.clear_all(); > + while ((index_name= it++)) > + { > + if (tab->s->keynames.type_names == 0 || > + (pos= find_type(&tab->s->keynames, index_name->str, > + index_name->length, 1)) <= 0) > + { > + compl_result_code= result_code= HA_ADMIN_INVALID; > + break; > + } > + tab->keys_in_use_for_query.set_bit(--pos); > + } > + } > if (!(compl_result_code= > alloc_statistics_for_table(thd, table->table)) && > !(compl_result_code= > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 2

[Maria-developers] Igor please review: MDEV-22377: Subquery in an [UPDATE] query uses full scan instead of range
by Sergey Petrunia 30 Apr '20

30 Apr '20

Hello Igor, Could you please review the below: commit 793b100cc01228f01ead841d6c45f9e2b07786f9 Author: Sergei Petrunia <psergey(a)askmonty.org> Date: Thu Apr 30 01:25:11 2020 +0300 MDEV-22377: Subquery in an [UPDATE] query uses full scan instead of range When doing IN->EXISTS rewrite, Item_in_subselect::inject_in_to_exists_cond injects equalities into subquery's WHERE condition. The problem is that build_equal_items() has already been called for the subquery's WHERE, and tampering with the WHERE condition can prevent equality propagation from working. If the subquery's WHERE is an Item_cond_and with multiple equalities: - Item_equal objects form a suffix sub-list of the list in WHERE's Item_cond_and::list. The suffix is stored in JOIN::cond_equal->current_level. - Item_cond_and::m_cond_equal must also be preserved. This patch makes inject_in_to_exists_cond() not to break these properties diff --git a/mysql-test/main/subselect2.result b/mysql-test/main/subselect2.result index a3d7fda7abc..c54d635230f 100644 --- a/mysql-test/main/subselect2.result +++ b/mysql-test/main/subselect2.result @@ -262,7 +262,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DEPENDENT SUBQUERY t2c ALL NULL NULL NULL NULL 2 100.00 Using where; Using join buffer (incremental, BNL join) Warnings: Note 1276 Field or reference 'test.t1.a' of SELECT #2 was resolved in SELECT #1 -Note 1003 /* select#1 */ select `test`.`t1`.`a` AS `a`,`test`.`t1`.`c1` AS `c1` from `test`.`t1` where !<expr_cache><`test`.`t1`.`c1`,`test`.`t1`.`a`>(<in_optimizer>(`test`.`t1`.`c1`,<exists>(/* select#2 */ select `test`.`t2a`.`c2` from `test`.`t2` `t2a` join `test`.`t2` `t2b` join `test`.`t2` `t2c` where (`test`.`t2b`.`m` <> `test`.`t1`.`a` or `test`.`t2b`.`m` = `test`.`t2a`.`m`) and trigcond(<cache>(`test`.`t1`.`c1`) = `test`.`t2a`.`c2` or `test`.`t2a`.`c2` is null) and `test`.`t2c`.`c2` = `test`.`t2b`.`c2` and `test`.`t2b`.`n` = `test`.`t2a`.`m` having trigcond(`test`.`t2a`.`c2` is null)))) +Note 1003 /* select#1 */ select `test`.`t1`.`a` AS `a`,`test`.`t1`.`c1` AS `c1` from `test`.`t1` where !<expr_cache><`test`.`t1`.`c1`,`test`.`t1`.`a`>(<in_optimizer>(`test`.`t1`.`c1`,<exists>(/* select#2 */ select `test`.`t2a`.`c2` from `test`.`t2` `t2a` join `test`.`t2` `t2b` join `test`.`t2` `t2c` where `test`.`t2c`.`c2` = `test`.`t2b`.`c2` and `test`.`t2b`.`n` = `test`.`t2a`.`m` and (`test`.`t2b`.`m` <> `test`.`t1`.`a` or `test`.`t2b`.`m` = `test`.`t2a`.`m`) and trigcond(<cache>(`test`.`t1`.`c1`) = `test`.`t2a`.`c2` or `test`.`t2a`.`c2` is null) having trigcond(`test`.`t2a`.`c2` is null)))) DROP TABLE t1,t2; # # MDEV-614, also MDEV-536, also LP:1050806: diff --git a/mysql-test/main/subselect3.result b/mysql-test/main/subselect3.result index 5c4544a1b05..0034f61ac23 100644 --- a/mysql-test/main/subselect3.result +++ b/mysql-test/main/subselect3.result @@ -169,7 +169,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DEPENDENT SUBQUERY t2 ref a a 5 test.t1.b 1 100.00 Using where Warnings: Note 1276 Field or reference 'test.t3.oref' of SELECT #2 was resolved in SELECT #1 -Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a` or `test`.`t1`.`a` is null) and `test`.`t2`.`a` = `test`.`t1`.`b` having trigcond(`test`.`t1`.`a` is null)))) AS `Z` from `test`.`t3` +Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`a` = `test`.`t1`.`b` and `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a` or `test`.`t1`.`a` is null) having trigcond(`test`.`t1`.`a` is null)))) AS `Z` from `test`.`t3` drop table t1, t2, t3; create table t1 (a int NOT NULL, b int NOT NULL, key(a)); insert into t1 values @@ -197,7 +197,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DEPENDENT SUBQUERY t2 ref a a 4 test.t1.b 1 100.00 Using where Warnings: Note 1276 Field or reference 'test.t3.oref' of SELECT #2 was resolved in SELECT #1 -Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a`) and `test`.`t2`.`a` = `test`.`t1`.`b`))) AS `Z` from `test`.`t3` +Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`a` = `test`.`t1`.`b` and `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a`)))) AS `Z` from `test`.`t3` drop table t1,t2,t3; create table t1 (oref int, grp int); insert into t1 (oref, grp) values diff --git a/mysql-test/main/subselect3_jcl6.result b/mysql-test/main/subselect3_jcl6.result index 4260676cc37..b7b18bf80e0 100644 --- a/mysql-test/main/subselect3_jcl6.result +++ b/mysql-test/main/subselect3_jcl6.result @@ -172,7 +172,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DEPENDENT SUBQUERY t2 ref a a 5 test.t1.b 1 100.00 Using where; Using join buffer (flat, BKA join); Key-ordered Rowid-ordered scan Warnings: Note 1276 Field or reference 'test.t3.oref' of SELECT #2 was resolved in SELECT #1 -Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a` or `test`.`t1`.`a` is null) and `test`.`t2`.`a` = `test`.`t1`.`b` having trigcond(`test`.`t1`.`a` is null)))) AS `Z` from `test`.`t3` +Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`a` = `test`.`t1`.`b` and `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a` or `test`.`t1`.`a` is null) having trigcond(`test`.`t1`.`a` is null)))) AS `Z` from `test`.`t3` drop table t1, t2, t3; create table t1 (a int NOT NULL, b int NOT NULL, key(a)); insert into t1 values @@ -200,7 +200,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DEPENDENT SUBQUERY t2 ref a a 4 test.t1.b 1 100.00 Using where; Using join buffer (flat, BKA join); Key-ordered Rowid-ordered scan Warnings: Note 1276 Field or reference 'test.t3.oref' of SELECT #2 was resolved in SELECT #1 -Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a`) and `test`.`t2`.`a` = `test`.`t1`.`b`))) AS `Z` from `test`.`t3` +Note 1003 /* select#1 */ select `test`.`t3`.`a` AS `a`,`test`.`t3`.`oref` AS `oref`,<expr_cache><`test`.`t3`.`a`,`test`.`t3`.`oref`>(<in_optimizer>(`test`.`t3`.`a`,<exists>(/* select#2 */ select `test`.`t1`.`a` from `test`.`t1` join `test`.`t2` where `test`.`t2`.`a` = `test`.`t1`.`b` and `test`.`t2`.`b` = `test`.`t3`.`oref` and trigcond(<cache>(`test`.`t3`.`a`) = `test`.`t1`.`a`)))) AS `Z` from `test`.`t3` drop table t1,t2,t3; create table t1 (oref int, grp int); insert into t1 (oref, grp) values diff --git a/mysql-test/main/subselect4.result b/mysql-test/main/subselect4.result index 3dc019cc6c1..e355cfc60f0 100644 --- a/mysql-test/main/subselect4.result +++ b/mysql-test/main/subselect4.result @@ -2349,7 +2349,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 1 PRIMARY t2 system NULL NULL NULL NULL 1 100.00 2 DEPENDENT SUBQUERY t1 ALL NULL NULL NULL NULL 2 100.00 Using where Warnings: -Note 1003 /* select#1 */ select 3 AS `f` from dual where !<expr_cache><3>(<in_optimizer>(3,<exists>(/* select#2 */ select `test`.`t1`.`b` from `test`.`t1` where (`test`.`t1`.`c` = 'USA' or `test`.`t1`.`c` <> 'USA') and trigcond(<cache>(3) = `test`.`t1`.`b` or `test`.`t1`.`b` is null) and `test`.`t1`.`b` = `test`.`t1`.`a` having trigcond(`test`.`t1`.`b` is null)))) +Note 1003 /* select#1 */ select 3 AS `f` from dual where !<expr_cache><3>(<in_optimizer>(3,<exists>(/* select#2 */ select `test`.`t1`.`b` from `test`.`t1` where `test`.`t1`.`b` = `test`.`t1`.`a` and (`test`.`t1`.`c` = 'USA' or `test`.`t1`.`c` <> 'USA') and trigcond(<cache>(3) = `test`.`t1`.`b` or `test`.`t1`.`b` is null) having trigcond(`test`.`t1`.`b` is null)))) SELECT * FROM t2 WHERE f NOT IN (SELECT b FROM t1 WHERE 0 OR (c IN ('USA') OR c NOT IN ('USA')) AND a = b); @@ -2582,3 +2582,29 @@ id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY <subquery2> eq_ref distinct_key distinct_key 4 func 1 2 MATERIALIZED t2 ALL NULL NULL NULL NULL 100 drop table t0, t1, t2; +# +# MDEV-22377: Subquery in an UPDATE query uses full scan instead of range +# +create table t0(a int); +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t1(nkey1 int, col1 int); +insert into t1 select A.a+10*B.a+100*C.a, A.a+10*B.a+100*C.a from t0 A, t0 B, t0 C; +create table t2(key1 int, col1 int, key(key1)); +insert into t2 select A.a+10*B.a, A.a+10*B.a from t0 A, t0 B; +alter table t0 add b int; +# This must use the join order of t0,t2,t1 +# And table t2 must use range access, not full scan: +explain +select * +from t0 +where +a not in (SELECT t1.col1 +FROM t1 JOIN t2 ON t2.key1 = t1.nkey1 +WHERE +t1.nkey1 IN (1,2,3,4,5) and t2.col1>t0.b +); +id select_type table type possible_keys key key_len ref rows Extra +1 PRIMARY t0 ALL NULL NULL NULL NULL 10 Using where +2 DEPENDENT SUBQUERY t2 range key1 key1 5 NULL 5 Using index condition; Using where +2 DEPENDENT SUBQUERY t1 ALL NULL NULL NULL NULL 1000 Using where; Using join buffer (flat, BNL join) +drop table t0, t1, t2; diff --git a/mysql-test/main/subselect4.test b/mysql-test/main/subselect4.test index bc8db41c505..e3d8b5be832 100644 --- a/mysql-test/main/subselect4.test +++ b/mysql-test/main/subselect4.test @@ -2115,6 +2115,32 @@ where exists (select * from t2 where t2.a=t1.a order by t2.b limit 2,3); explain select * from t1 where t1.a in (select t2.a from t2 order by t2.b); +drop table t0, t1, t2; + +--echo # +--echo # MDEV-22377: Subquery in an UPDATE query uses full scan instead of range +--echo # +create table t0(a int); +insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); +create table t1(nkey1 int, col1 int); +insert into t1 select A.a+10*B.a+100*C.a, A.a+10*B.a+100*C.a from t0 A, t0 B, t0 C; + +create table t2(key1 int, col1 int, key(key1)); +insert into t2 select A.a+10*B.a, A.a+10*B.a from t0 A, t0 B; + +alter table t0 add b int; +--echo # This must use the join order of t0,t2,t1 +--echo # And table t2 must use range access, not full scan: +explain +select * +from t0 +where + a not in (SELECT t1.col1 + FROM t1 JOIN t2 ON t2.key1 = t1.nkey1 + WHERE + t1.nkey1 IN (1,2,3,4,5) and t2.col1>t0.b + ); drop table t0, t1, t2; + diff --git a/mysql-test/main/subselect_mat_cost_bugs.result b/mysql-test/main/subselect_mat_cost_bugs.result index a18c5e608f1..755f1e2ad13 100644 --- a/mysql-test/main/subselect_mat_cost_bugs.result +++ b/mysql-test/main/subselect_mat_cost_bugs.result @@ -100,7 +100,7 @@ id select_type table type possible_keys key key_len ref rows filtered Extra 2 DEPENDENT SUBQUERY t2 index c3 c3 9 NULL 2 100.00 Using where; Using index; Using join buffer (flat, BNL join) Warnings: Note 1276 Field or reference 'test.t1.pk' of SELECT #2 was resolved in SELECT #1 -Note 1003 /* select#1 */ select `test`.`t1`.`pk` AS `pk` from `test`.`t1` where <expr_cache><`test`.`t1`.`c1`,`test`.`t1`.`pk`>(<in_optimizer>(`test`.`t1`.`c1`,<exists>(/* select#2 */ select `test`.`t1a`.`c1` from `test`.`t1b` join `test`.`t2` left join `test`.`t1a` on(`test`.`t1a`.`c2` = `test`.`t1b`.`pk` and 2) where `test`.`t1`.`pk` <> 0 and <cache>(`test`.`t1`.`c1`) = `test`.`t1a`.`c1` and `test`.`t2`.`c3` = `test`.`t1b`.`c4`))) +Note 1003 /* select#1 */ select `test`.`t1`.`pk` AS `pk` from `test`.`t1` where <expr_cache><`test`.`t1`.`c1`,`test`.`t1`.`pk`>(<in_optimizer>(`test`.`t1`.`c1`,<exists>(/* select#2 */ select `test`.`t1a`.`c1` from `test`.`t1b` join `test`.`t2` left join `test`.`t1a` on(`test`.`t1a`.`c2` = `test`.`t1b`.`pk` and 2) where `test`.`t2`.`c3` = `test`.`t1b`.`c4` and `test`.`t1`.`pk` <> 0 and <cache>(`test`.`t1`.`c1`) = `test`.`t1a`.`c1`))) SELECT pk FROM t1 WHERE c1 IN @@ -364,7 +364,7 @@ AND a = SOME (SELECT b FROM t5)); id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY t3 ALL NULL NULL NULL NULL 2 Using where 2 DEPENDENT SUBQUERY t5 index c c 10 NULL 2 Using where; Using index; Start temporary -2 DEPENDENT SUBQUERY t4 eq_ref PRIMARY PRIMARY 4 test.t5.b 1 Using index condition; Using where; End temporary +2 DEPENDENT SUBQUERY t4 eq_ref PRIMARY PRIMARY 4 test.t5.b 1 Using where; End temporary SELECT * FROM t3 WHERE t3.b > ALL ( diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 736dfbd33b4..726c58e49a2 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -2704,9 +2704,28 @@ bool Item_in_subselect::inject_in_to_exists_cond(JOIN *join_arg) and_args= ((Item_cond*) join_arg->conds)->argument_list(); if (join_arg->cond_equal) and_args->disjoin((List<Item> *) &join_arg->cond_equal->current_level); + + /* + If subquery's WHERE is an Item_cond_and, add the injected conditions + into it manually. If we just called and_items(...) that could + potentially replace the top-level Item_cond_and and current top-level + Item_cond_and::m_cond_equal will be lost. + */ + if (where_item->type() == Item::COND_ITEM && + ((Item_cond*)where_item)->functype() == Item_func::COND_AND_FUNC) + { + List_iterator<Item> it(*((Item_cond*)where_item)->argument_list()); + while (Item *item=it++) + and_args->push_back(item); + } + else + and_args->push_back(where_item); + + where_item= join_arg->conds; } + else + where_item= and_items(thd, join_arg->conds, where_item); - where_item= and_items(thd, join_arg->conds, where_item); if (where_item->fix_fields_if_needed(thd, 0)) DBUG_RETURN(true); // TIMOUR TODO: call optimize_cond() for the new where clause @@ -2719,12 +2738,15 @@ bool Item_in_subselect::inject_in_to_exists_cond(JOIN *join_arg) { /* The argument list of the top-level AND may change after fix fields. */ and_args= ((Item_cond*) join_arg->conds)->argument_list(); - List_iterator<Item_equal> li(join_arg->cond_equal->current_level); - Item_equal *elem; - while ((elem= li++)) - { - and_args->push_back(elem, thd->mem_root); - } + + /* + Note that Item_equal objects must be present in this form: + Item_cond::conds list must have the end of the list to be the + same linked list as join_arg->cond_equal->current_level. + The code in substitute_for_best_equal_field depends on it (see the + disjoin() call). + */ + and_args->append((List<Item> *)&join_arg->cond_equal->current_level); } } BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] 96aee3229a2: MENT-703: Disable rpl.rpl_skip_replication, rpl.rpl_set_statement_default_master, rpl.rpl_extra_col_master_myisam, rpl.rpl_slave_load_tmpdir_not_exist tests in ES 10.2 and above
by sujatha 29 Apr '20

29 Apr '20

revision-id: 96aee3229a288dae52c457ca808966092da6d821 (mariadb-10.4.5-669-g96aee3229a2) parent(s): a284ea1909d66e4284a964a4ca7d45b75c450ed7 author: Sujatha committer: Sujatha timestamp: 2020-04-29 13:28:23 +0530 message: MENT-703: Disable rpl.rpl_skip_replication, rpl.rpl_set_statement_default_master, rpl.rpl_extra_col_master_myisam, rpl.rpl_slave_load_tmpdir_not_exist tests in ES 10.2 and above Problem: ======= Disable following tests in ES 10.2 and above, till MDEV-13258 and MDEV-14203 are fixed. rpl.rpl_skip_replication, rpl.rpl_set_statement_default_master, rpl.rpl_extra_col_master_myisam, rpl.rpl_slave_load_tmpdir_not_exist --- mysql-test/suite/rpl/disabled.def | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mysql-test/suite/rpl/disabled.def b/mysql-test/suite/rpl/disabled.def index 77b01bd8d1d..5a04f4ccd15 100644 --- a/mysql-test/suite/rpl/disabled.def +++ b/mysql-test/suite/rpl/disabled.def @@ -19,3 +19,7 @@ rpl_parallel2 : fails after MDEV-16172 rpl_semi_sync_after_sync : fails after MDEV-16172 rpl_slave_grp_exec: MDEV-10514 rpl_auto_increment_update_failure : disabled for now +rpl_skip_replication : MDEV-13258 +rpl_set_statement_default_master : MDEV-13258 +rpl_extra_col_master_myisam : MDEV-14203 +rpl_slave_load_tmpdir_not_exist : MDEV-14203

1 0