- developers - lists.mariadb.org

[Maria-developers] Lag Free ALTER Table for Slave
by Sachin Setiya 27 Feb '20

27 Feb '20

Hi Everyone! This mail will describe usage and implementation of Lag Free Alter. And this will also answer the question raised by Kristian and Simon J Mudd(in 2016) Desc:- This will split Alter into 2 different commits. START ALTER and COMMIT /ROLLBACK ALTER , Start Alter will be written in binlog as soon as we get the locks for the table, alter will proceeds as usual and at the time of writing binlog if alter is successful we will write COMMIT Alter other wise ROLLBACK Alter. Usage:- Using this feature is quite simple. 1. On master you have to turn on `BINLOG_SPLIT_ALTER` dynamic variable. 2. Slave must be using parallel replication. Advance Usage:- So alter is divided like this. 1. START identifier Actual_alter_stmt 2. COMMIT/ROLLBACK identifier Actual_alter_stmt. OR 2. COMMIT/ROLLBACK identifier ALTER identifier is thread_id. Questions by Simon Mudd. >>* this behaviour should be configurable? Yes. >> - new global variable on the master to allow injection of this changed event stream? Right , `BINLOG_SPLIT_ALTER` >> - a new session variable set in the session where the command is triggered ? Right , `BINLOG_SPLIT_ALTER` >> - on slave a setting to indicate how many INPLACE ALTER TABLEs can run at once? No setting so far , but I am thinking of maximum no of CONCURRENT ALTER = slave_parallel_threads >>* how does a DBA monitor what’s going on? >> - the progress and number of active DDL statements So as we there is 2 part, So progress will go like this 1. Executing of start alter (this will take most of time) 2. Waiting for commit/rollback Signal 3. Commit/ Rollback Alter. Number of active ALTER , these will create new threads so DBA can know using this or I am thinking of adding variable in SHOW SLAVE INFO. which will show active DDL. >> - please consider adding counters/metrics for: >> * number of “asynchronous DDLs” in progress / completed successfully / failed or rolled back Okay, We can have counter for these metrics. If i get time to implement this. >> * sizes of the DDL changes made so far Not sure if we need this. >> * number of threads busy in this state (maybe can be implied from SHOW PROCESSLIST or equivalent but nicer to have an easy to query metric) Show processlist will show the busy threads. Sample Output when concurrent alter is running on slave. slave_parallel_threads= 10 Every 1.0s: mysql -uroot -S /home/sachin/alter/build/mysql-test/var/tmp/mysqld.2.sock -e show processlist sachin-sp52: Wed Jan 22 16:56:00 2020 Id User Host db Command Time State Info Progress 1 system user NULL Daemon NULL InnoDB purge coordinator NULL 0.000 3 system user NULL Daemon NULL InnoDB purge worker NULL 0.000 2 system user NULL Daemon NULL InnoDB purge worker NULL 0.000 4 system user NULL Daemon NULL InnoDB purge worker NULL 0.000 5 system user NULL Daemon NULL InnoDB shutdown handler NULL 0.000 10 root localhost:44478 test Sleep 6 NULL 0.000 11 root localhost:44480 test Sleep 7 NULL 0.000 15 root localhost:44492 test Sleep 6 NULL 0.000 16 root localhost:44494 test Sleep 6 NULL 0.000 17 system user NULL Slave_IO 6 Waiting for master to send event NULL 0.000 19 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 22 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 20 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 23 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 24 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 25 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 26 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 21 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 27 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 28 system user NULL Slave_worker 6 Waiting for work from SQL thread NULL 0.000 18 system user NULL Slave_SQL 6 Slave has read all relay log; waiting for the slave I/O thread to update it NULL 0.000 29 system user test Slave_worker 6 NULL /*!100001 START 19 alter table t3 add column c int, force, algorithm=inplace */ 0.000 30 system user test Slave_worker 6 NULL /*!100001 START 17 alter table t1 add column c int, force, algorithm=inplace */ 0.000 31 system user test Slave_worker 6 NULL /*!100001 START 18 alter table t2 add column c int, force, algorithm=inplace */ 0.000 32 system user test Slave_worker 6 NULL /*!100001 START 20 alter table t4 add column c int, force, algorithm=inplace */ 0.000 33 system user test Slave_worker 6 NULL /*!100001 START 21 alter table t5 add column c int, force, algorithm=inplace */ 0.000 34 root localhost NULL Query 0 Init show processlist 0.000 >>* what happens if the commit/rollback ddl event never arrives? (the statement on the master could be skipped for one of several reasons) So here are 2 options If slave is still running DBA can manually run COMMIT/ROLLBACK identifier alter_stmt/ALTER , where identifier is same as START ALTER identifier. so this will either commit or rollback alter. If slave is not running. Since slave is not running all context is lost , so this will take as much as time a normal alter. >> - I assume that users on the slave see the old structure all the time until it completes Right, we work on copy of alter for myisam and for other case innodb will take care of it. >> - would this have a storage or performance penalty on the slave if the commit/rollback DDL event never arrives? Not performance but yes storage and ram penalty. >> - can a “commit” / “rollback” be manually triggered by the DBA in such circumstances? yes. >>* what happens if the server crashes while this process (or these processes) are ongoing when the server starts up again? So if we crash in START ALTER no issue , we can run it again , since it is transactional(we are working on copy so ..) If we crash after start alter and we receive COMMIT alter then it will be treated as normal alter. ROLLBACK ALTER will be instant. If we crash in COMMIT ALTER this will be same as crash in normal ALTER table , so DBA has to deal with it. Architecture:- Lets look at sample alter perf data for innodb and myisam Myisam + 96.72% 0.00% mysqld mysqld [.] mysql_parse + 96.72% 0.00% mysqld mysqld [.] mysql_execute_command + 96.72% 0.00% mysqld mysqld [.] Sql_cmd_alter_table::execute - 96.72% 0.00% mysqld mysqld [.] mysql_alter_table - mysql_alter_table - 96.63% copy_data_between_tables + 79.64% handler::ha_write_row + 6.24% TABLE::update_default_fields + 5.71% READ_RECORD::read_record + 1.87% do_copy_null 1.05% Field::do_field_int + 0.95% _mcount InnoDB + 41.27% 0.00% mysqld mysqld [.] mysql_parse + 41.27% 0.00% mysqld mysqld [.] mysql_execute_command + 41.27% 0.00% mysqld mysqld [.] Sql_cmd_alter_table::execute - 41.27% 0.00% mysqld mysqld [.] mysql_alter_table - mysql_alter_table - 41.27% mysql_inplace_alter_table - 41.15% handler::ha_inplace_alter_table ha_innobase::inplace_alter_table - row_merge_build_indexes - 41.00% row_merge_read_clustered_index + 22.88% row_merge_insert_index_tuples + 5.89% row_build_w_add_vcol + 5.20% BtrBulk::finish + 3.20% row_merge_buf_add + 0.87% page_cur_move_to_next 0.62% rec_get_offsets_func So as we can see most of the work is done by copy_data_between_tables/ mysql_inplace_alter_table. Master Side:- There is not much change into master side. So after locking table and before executing these functions we write START _id_ alter_stmt into binlog. And at the time of write_bin_log we write COMMIT _id_ alter_stmt into binlog. So alter statement will be divided into 2 , hence 2 gtid. START_ALTER will have special flag FL_START_ALTER_E1(4), this will be used on slave side to create new worker thread for start alter processing. Slave Side:- This require parallel replication on slave side. So in do_event when we get a gtid_event with FL_START_ALTER_E1, ::choose_thread will call rpl_parallel_add_extra_worker, which will create a new worker thread and this will do start alter processing. So the gtid_log_event and next Query_log_event will be scheduled to this new thread. And this thread will be exited as soon as alter is finished. On slave side START ALTER will binlog after successfully getting MDL lock and thd lock. Untill this point we are executed as DDL(so new GCO), but after getting locks we will call finish_event_group. So that new events can proceed in parallel. Then it will continue to execute code in mysql_alter_table until we reach a non transactional part(like renaming table/dropping table in myisam) So before executing NON-Transactional part it will wait signal from other worker thread to either abort or proceed forward. We will add a entry into mi-> start_alter_list, with thread_id(of master) as a key. COMMIT/ROLLBACK Alter is treated as a normal query_log_event so it will be assigned normal worker. This command will take SQLCOM_COMMIT_PREVIOUS path in mysql_execute_command. We will simple search for thread_id into start_alter_list and change status from ::WAITING to ::COMMIT_ALTER and signal the wait condition if we dont find the thread_id we will wait on wait condition, So simple it is just consumer producer between start alter and commit/rollback. Questions by Kristian:- >Can you explain what you mean by "true LOCK=NONE"? It is not clear from your >description. I think you mean something that will allow to run an ALTER >TABLE on the slave in parallel with the same statement on the master? Yes >There will be a number of things to consider with this. Previously, all >transactions in the binlog have been atomic; this introduces related events >that can be arbitrarily separated in the binlog. >For example, suppose the slave stops in the middle between BEGIN_DDL_EVENT >and COMMIT_DDL_EVENT/ROLLBACK_DDL_EVENT. What is then the slave's binlog >position / GTID position? Now we are using 2 gtid for ALTER , So it wont be issue. >Hopefully, the exiting pool of slave worker threads (currently used for >parallel replication) can be used for this as well? Doesn't seem right to >introduce a new kind of thread pool... We are using same(global) thread pool but I am creating new threads for START ALTER. Otherwise we will have a deadlock , Suppose if we have 5 concurrent START ALTER and just 5 slave-worker-threads , all worker will be waiting for COMMIT/ROLLBACK, but we cant execute commit rollback because we dont have any free worker. There can be more case when we have concurrent DML too , so creating a new thread was the safest option. >Won't you need some mechanism to prevent following events to access the >table before ALTER TABLE in a worker thread can acquire the metadata lock? Right, So Untill Locking START ALTER is executed as DDL so no following event will be executed before locking. And one more thing regarding locks , So there will we only one thread will be doing whole work of alter, (we can just assume that there is some sleep on slave) so execution on master and slave will be equivalent. >There are some nasty issues to be aware of with potential deadlocks related >to FLUSH TABLES WITH GLOBAL READ LOCK and such, when multiple threads are >trying to coordinate waiting and locking; there were already some quite hard >issues with this for parallel replication. I have to test for it. Code branch bb-10.5-olter Jira (Mdev-11675(https://jira.mariadb.org/browse/MDEV-11675)) Regards Sachin -- Regards Sachin Setiya Software Engineer at MariaDB

3 4

Re: [Maria-developers] ec1a860bd09: MDEV-21743 Split up SUPER privilege to smaller privileges
by Sergei Golubchik 27 Feb '20

27 Feb '20

Hi, Alexander! Thanks! Looks very straightforward. See few comments below. The main one - I am not sure I like the idea of creating numerous aliases for privileges. This approach looks rather confusing to me. On Feb 26, Alexander Barkov wrote: > revision-id: ec1a860bd09 (mariadb-10.5.0-231-gec1a860bd09) > parent(s): b8b5a6a2f9d > author: Alexander Barkov <bar(a)mariadb.com> > committer: Alexander Barkov <bar(a)mariadb.com> > timestamp: 2020-02-23 22:09:55 +0400 > message: > > MDEV-21743 Split up SUPER privilege to smaller privileges > diff --git a/mysql-test/main/alter_user.test b/mysql-test/main/alter_user.test > index 9ea98615272..60a36499a55 100644 > --- a/mysql-test/main/alter_user.test > +++ b/mysql-test/main/alter_user.test > @@ -30,7 +30,7 @@ alter user foo; > > --echo # Grant super privilege to the user. > connection default; > -grant super on *.* to foo; > +grant READ_ONLY ADMIN on *.* to foo; --echo comments are now wrong > --echo # We now have super privilege. We should be able to run alter user. > connect (b, localhost, foo); > diff --git a/mysql-test/main/grant.result b/mysql-test/main/grant.result > index e83083be4ed..1452ada11f5 100644 > --- a/mysql-test/main/grant.result > +++ b/mysql-test/main/grant.result > @@ -631,6 +634,10 @@ Super Server Admin To use KILL thread, SET GLOBAL, CHANGE MASTER, etc. > Trigger Tables To use triggers > Create tablespace Server Admin To create/alter/drop tablespaces > Update Tables To update existing rows > +Set user Server To create views and stored routines with a different definer > +Federated admin Server To execute the CREATE SERVER, ALTER SERVER, DROP SERVER statements > +Connection admin Server To skip connection related limits tests ^^^ allows KILL too > +Read_only admin Server To perform write operations even if @@read_only=ON > Usage Server Admin No privileges - allow connect only > connect root,localhost,root,,test,$MASTER_MYPORT,$MASTER_MYSOCK; > connection root; > diff --git a/sql/lock.cc b/sql/lock.cc > index 6f86c0a38f6..9a4024606f8 100644 > --- a/sql/lock.cc > +++ b/sql/lock.cc > @@ -114,7 +113,7 @@ lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags) > DBUG_ENTER("lock_tables_check"); > > system_count= 0; > - is_superuser= (thd->security_ctx->master_access & SUPER_ACL) != NO_ACL; > + is_superuser= (thd->security_ctx->master_access & IGNORE_READ_ONLY_ACL) != NO_ACL; may be then s/is_superuser/ignores_read_only/ ? > log_table_write_query= (is_log_table_write_query(thd->lex->sql_command) > || ((flags & MYSQL_LOCK_LOG_TABLE) != 0)); > > diff --git a/sql/privilege.h b/sql/privilege.h > index 5dbc0b6dbdf..f80e726d8d0 100644 > --- a/sql/privilege.h > +++ b/sql/privilege.h > @@ -59,24 +59,60 @@ enum privilege_t: unsigned long long > TRIGGER_ACL = (1UL << 27), > CREATE_TABLESPACE_ACL = (1UL << 28), > DELETE_HISTORY_ACL = (1UL << 29), // Added in 10.3.4 > + SET_USER_ACL = (1UL << 30), // Added in 10.5.2 > + FEDERATED_ADMIN_ACL = (1UL << 31), // Added in 10.5.2 > + CONNECTION_ADMIN_ACL = (1ULL << 32), // Added in 10.5.2 > + READ_ONLY_ADMIN_ACL = (1ULL << 33), // Added in 10.5.2 > + REPL_SLAVE_ADMIN_ACL = (1ULL << 34), // Added in 10.5.2 > + REPL_MASTER_ADMIN_ACL = (1ULL << 35), // Added in 10.5.2 > + BINLOG_ADMIN_ACL = (1ULL << 36) // Added in 10.5.2 > /* > - don't forget to update > - 1. static struct show_privileges_st sys_privileges[] > - 2. static const char *command_array[] and static uint command_lengths[] > - 3. mysql_system_tables.sql and mysql_system_tables_fix.sql > - 4. acl_init() or whatever - to define behaviour for old privilege tables > - 5. sql_yacc.yy - for GRANT/REVOKE to work > - 6. Add a new ALL_KNOWN_ACL_VERSION > - 7. Change ALL_KNOWN_ACL to ALL_KNOWN_ACL_VERSION > - 8. Update User_table_json::get_access() > + don't forget to update: > + In this file: > + - Add a new LAST_version_ACL > + - Fix PRIVILEGE_T_MAX_BIT > + - Add a new ALL_KNOWN_ACL_version > + - Change ALL_KNOWN_ACL to ALL_KNOWN_ACL_version > + - Change GLOBAL_ACLS if needed > + - Change SUPER_ADDED_SINCE_USER_TABLE_ACL if needed > + > + In other files: > + - static struct show_privileges_st sys_privileges[] > + - static const char *command_array[] and static uint command_lengths[] > + - mysql_system_tables.sql and mysql_system_tables_fix.sql > + - acl_init() or whatever - to define behaviour for old privilege tables > + - Update User_table_json::get_access() > + - sql_yacc.yy - for GRANT/REVOKE to work > + > + Important: the enum should contain only single-bit values. > + In this case, debuggers print bit combinations in the readable form: > + (gdb) p (privilege_t) (15) > + $8 = (SELECT_ACL | INSERT_ACL | UPDATE_ACL | DELETE_ACL) > + > + Bit-OR combinations of the above values should be declared outside! > */ > - > - // A combination of all bits defined in 10.3.4 (and earlier) > - ALL_KNOWN_ACL_100304 = (1UL << 30) - 1 > }; > > > -constexpr privilege_t ALL_KNOWN_ACL= ALL_KNOWN_ACL_100304; > +// Version markers > +constexpr privilege_t LAST_100304_ACL= DELETE_HISTORY_ACL; > +constexpr privilege_t LAST_100502_ACL= BINLOG_ADMIN_ACL; > +constexpr privilege_t LAST_CURRENT_ACL= LAST_100502_ACL; > +constexpr uint PRIVILEGE_T_MAX_BIT= 36; > + > +static_assert((privilege_t)(1ULL << PRIVILEGE_T_MAX_BIT) == LAST_CURRENT_ACL, > + "LAST_CURRENT_ACL and PRIVILEGE_T_MAX_BIT do not match"); why wouldn't you define PRIVILEGE_T_MAX_BIT via LAST_CURRENT_ACL instead? > + > +// A combination of all bits defined in 10.3.4 (and earlier) > +constexpr privilege_t ALL_KNOWN_ACL_100304 = > + (privilege_t) ((LAST_100304_ACL << 1) - 1); > + > +// A combination of all bits defined in 10.5.2 > +constexpr privilege_t ALL_KNOWN_ACL_100502= > + (privilege_t) ((LAST_100502_ACL << 1) - 1); > + > +// A combination of all bits defined as of the current version > +constexpr privilege_t ALL_KNOWN_ACL= ALL_KNOWN_ACL_100502; > > > // Unary operators > @@ -229,6 +280,104 @@ constexpr privilege_t SHOW_CREATE_TABLE_ACLS= > constexpr privilege_t TMP_TABLE_ACLS= > COL_DML_ACLS | ALL_TABLE_DDL_ACLS; > > + > + > +/* > + If a VIEW has a `definer=invoker@host` clause and > + the specified definer does not exists, then > + - The invoker with REVEAL_MISSING_DEFINER_ACL gets: > + ERROR: The user specified as a definer ('definer1'@'localhost') doesn't exist > + - The invoker without MISSING_DEFINER_ACL gets a generic access error, > + without revealing details that the definer does not exists. > + > + TODO: we should eventually test the same privilege when processing > + other objects that have the DEFINER clause (e.g. routines, triggers). > + Currently the missing definer is revealed for non-privileged invokers > + in case of routines, triggers, etc. > +*/ > +constexpr privilege_t REVEAL_MISSING_DEFINER_ACL= SUPER_ACL; 1. why did you create these aliases? I don't think they make the code simpler, on the contrary, now one can never know whether say, IGNORE_READ_ONLY_ACL is a real privilege like in GRANT IGNORE READ_ONLY ON *.* TO user@host or it's just an alias. 2. REVEAL_MISSING_DEFINER_ACL should be SET_USER_ACL, I think > +constexpr privilege_t DES_DECRYPT_ONE_ARG_ACL= SUPER_ACL; > +constexpr privilege_t LOG_BIN_TRUSTED_SP_CREATOR_ACL= SUPER_ACL; this could be SET_USER_ACL too > +constexpr privilege_t DEBUG_ACL= SUPER_ACL; > +constexpr privilege_t SET_GLOBAL_SYSTEM_VARIABLE_ACL= SUPER_ACL; > +constexpr privilege_t SET_RESTRICTED_SESSION_SYSTEM_VARIABLE_ACL= SUPER_ACL; > + > +/* Privileges related to --read-only */ > +constexpr privilege_t IGNORE_READ_ONLY_ACL= READ_ONLY_ADMIN_ACL; > + > +/* Privileges related to connection handling */ > +constexpr privilege_t IGNORE_INIT_CONNECT_ACL= CONNECTION_ADMIN_ACL; > +constexpr privilege_t IGNORE_MAX_USER_CONNECTIONS_ACL= CONNECTION_ADMIN_ACL; > +constexpr privilege_t IGNORE_MAX_CONNECTIONS_ACL= CONNECTION_ADMIN_ACL; > +constexpr privilege_t IGNORE_MAX_PASSWORD_ERRORS_ACL= CONNECTION_ADMIN_ACL; > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t KILL_OTHER_USER_PROCESS_ACL= CONNECTION_ADMIN_ACL; > + > + > +/* > + Binary log related privileges that are checked regardless > + of active replication running. > +*/ > + > +/* > + This command was renamed from "SHOW MASTER STATUS" > + to "SHOW BINLOG STATUS" in 10.5.2. > + Was SUPER_ACL | REPL_CLIENT_ACL prior to 10.5.2 > +*/ > +constexpr privilege_t STMT_SHOW_BINLOG_STATUS_ACL= REPL_CLIENT_ACL; > + > +// Was SUPER_ACL | REPL_CLIENT_ACL prior to 10.5.2 > +constexpr privilege_t STMT_SHOW_BINARY_LOGS_ACL= REPL_CLIENT_ACL; > + > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_PURGE_BINLOG_ACL= BINLOG_ADMIN_ACL; > + > +// Was REPL_SLAVE_ACL prior to 10.5.2 > +constexpr privilege_t STMT_SHOW_BINLOG_EVENTS_ACL= PROCESS_ACL; > + > + > +/* > + Privileges for replication related statements > + that are executed on the master. > +*/ > +constexpr privilege_t COM_REGISTER_SLAVE_ACL= REPL_SLAVE_ACL; > +constexpr privilege_t COM_BINLOG_DUMP_ACL= REPL_SLAVE_ACL; > +// Was REPL_SLAVE_ACL prior to 10.5.2 > +constexpr privilege_t STMT_SHOW_SLAVE_HOSTS_ACL= REPL_MASTER_ADMIN_ACL; > + > + > +/* Privileges for statements that are executed on the slave */ > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_START_SLAVE_ACL= REPL_SLAVE_ADMIN_ACL; > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_STOP_SLAVE_ACL= REPL_SLAVE_ADMIN_ACL; > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_CHANGE_MASTER_ACL= REPL_SLAVE_ADMIN_ACL; > +// Was (SUPER_ACL | REPL_CLIENT_ACL) prior to 10.5.2 > +constexpr privilege_t STMT_SHOW_SLAVE_STATUS_ACL= REPL_SLAVE_ADMIN_ACL; > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_BINLOG_ACL= REPL_SLAVE_ADMIN_ACL; > +// Was REPL_SLAVE_ACL prior to 10.5.2 > +constexpr privilege_t STMT_SHOW_RELAYLOG_EVENTS_ACL= REPL_SLAVE_ADMIN_ACL; > + > + > +/* Privileges for federated database related statements */ > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_CREATE_SERVER_ACL= FEDERATED_ADMIN_ACL; > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_ALTER_SERVER_ACL= FEDERATED_ADMIN_ACL; > +// Was SUPER_ACL prior to 10.5.2 > +constexpr privilege_t STMT_DROP_SERVER_ACL= FEDERATED_ADMIN_ACL; > + > + > +/* Privileges related to processes */ > +constexpr privilege_t COM_PROCESS_INFO_ACL= PROCESS_ACL; > +constexpr privilege_t STMT_SHOW_EXPLAIN_ACL= PROCESS_ACL; > +constexpr privilege_t STMT_SHOW_ENGINE_STATUS_ACL= PROCESS_ACL; > +constexpr privilege_t STMT_SHOW_ENGINE_MUTEX_ACL= PROCESS_ACL; > +constexpr privilege_t STMT_SHOW_PROCESSLIST_ACL= PROCESS_ACL; > + > + > /* > Defines to change the above bits to how things are stored in tables > This is needed as the 'host' and 'db' table is missing a few privileges > diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc > index e2a3c482ae4..b096bfa7a95 100644 > --- a/sql/sql_connect.cc > +++ b/sql/sql_connect.cc > @@ -1246,7 +1245,8 @@ void prepare_new_connection_state(THD* thd) > thd->set_command(COM_SLEEP); > thd->init_for_queries(); > > - if (opt_init_connect.length && !(sctx->master_access & SUPER_ACL)) > + if (opt_init_connect.length && > + (sctx->master_access & IGNORE_INIT_CONNECT_ACL) == NO_ACL) dunno, I kind of like !(access & SOME_ACL) why not to keep privilege_t -> bool? > { > execute_init_command(thd, &opt_init_connect, &LOCK_sys_init_connect); > if (unlikely(thd->is_error())) > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index dac5b025821..7f3a436a4c2 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -7138,8 +7138,7 @@ bool check_some_access(THD *thd, privilege_t want_access, TABLE_LIST *table) > @warning > One gets access right if one has ANY of the rights in want_access. > This is useful as one in most cases only need one global right, > - but in some case we want to check if the user has SUPER or > - REPL_CLIENT_ACL rights. > + but in some case we want to check multiple rights. In what cases? Are there any left? > > @retval > 0 ok Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 4c263b6d30b: MDEV-20632: Recursive CTE cycle detection using CYCLE clause
by Sergei Golubchik 26 Feb '20

26 Feb '20

Hi, Sanja! _some_ comments are below. The main thing, I still don't understand your changes in sql_select.cc. Why did you create separate counters and code paths with if() for CYCLE? I'd think it could be just a generalization of DISTINCT On Feb 20, Oleksandr Byelkin wrote: > revision-id: 4c263b6d30b (mariadb-10.5.0-92-g4c263b6d30b) > parent(s): 6f2e2285291 > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2020-01-27 21:56:02 +0100 > message: > > MDEV-20632: Recursive CTE cycle detection using CYCLE clause > > Added CYCLE clause to recursive CTE. > > diff --git a/sql/item.h b/sql/item.h > index c1f68a4f942..2cbaf880e00 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -622,6 +622,13 @@ class st_select_lex_unit; > class Item_func_not; > class Item_splocal; > > +/* Item::common_flags */ > +/* Indicates that name of this Item autogenerated or set by user */ > +#define IS_AUTO_GENETATED_NAME 1 typo ^^^ > +/* Inticates that this item is in CYCLE clause of WITH */ typo ^^^ > +#define IS_IN_WITH_CYCLE 2 > + > + > /** > String_copier that sends Item specific warnings. > */ > diff --git a/sql/sql_cte.cc b/sql/sql_cte.cc > index 5b3d3108da5..971844120bc 100644 > --- a/sql/sql_cte.cc > +++ b/sql/sql_cte.cc > @@ -982,6 +982,38 @@ With_element::rename_columns_of_derived_unit(THD *thd, are you sure what you're doing below can still be called "rename_columns_of_derived_unit" ? > else > make_valid_column_names(thd, select->item_list); > > + if (cycle_list) > + { > + List_iterator_fast<Item> it(select->item_list); > + List_iterator_fast<LEX_CSTRING> nm(*cycle_list); > + List_iterator_fast<LEX_CSTRING> nm_check(*cycle_list); > + DBUG_ASSERT(cycle_list->elements != 0); > + while (LEX_CSTRING *name= nm++) > + { > + Item *item; > + // unique check > + LEX_CSTRING *check; > + nm_check.rewind(); > + while ((check= nm_check++) && check != name) > + { > + if (check->length == name->length && > + strncmp(check->str, name->str, name->length) == 0) > + { > + my_error(ER_DUP_FIELDNAME, MYF(0), check->str); > + return true; > + } > + } > + while ((item= it++) && > + (item->name.length != name->length || > + strncmp(item->name.str, name->str, name->length) != 0)); > + if (item == NULL) > + { > + my_error(ER_BAD_FIELD_ERROR, MYF(0), name->str, "CYCLE clause"); > + return true; > + } this pattern is used so often that we might want to have a helper, like, List_iterator_fast<>::find() not in this commit, though. > + item->common_flags|= IS_IN_WITH_CYCLE; > + } > + } > unit->columns_are_renamed= true; > > return false; > @@ -1425,6 +1457,21 @@ void With_clause::print(String *str, enum_query_type query_type) > } > > > +static void list_strlex_print(String *str, List<LEX_CSTRING> *list) > +{ > + List_iterator_fast<LEX_CSTRING> li(*list); > + bool first= TRUE; > + while(LEX_CSTRING *col_name= li++) > + { > + if (first) > + first= FALSE; > + else > + str->append(','); > + str->append(col_name); should be "append_identifier", shouldn't it? add a test where a column name is a reserved word. > + } > +} > + > + > /** > @brief > Print this with element > @@ -1444,22 +1491,20 @@ void With_element::print(String *str, enum_query_type query_type) > { > List_iterator_fast<LEX_CSTRING> li(column_list); > str->append('('); > - for (LEX_CSTRING *col_name= li++; ; ) > - { > - str->append(col_name); > - col_name= li++; > - if (!col_name) > - { > - str->append(')'); > - break; > - } > - str->append(','); > - } > + list_strlex_print(str, &column_list); any other code that could make use of your list_strlex_print ? > + str->append(')'); > } > - str->append(STRING_WITH_LEN(" as ")); > - str->append('('); > + str->append(STRING_WITH_LEN(" as (")); > spec->print(str, query_type); > str->append(')'); > + > + if (cycle_list) > + { > + DBUG_ASSERT(cycle_list->elements != 0); > + str->append(STRING_WITH_LEN(" CYCLE (")); > + list_strlex_print(str, cycle_list); > + str->append(") "); > + } > } > > > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index c115d9352aa..0d60ab8579f 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -14704,9 +14704,30 @@ with_list_element: > if (elem->set_unparsed_spec(thd, spec_start, $6.pos(), > spec_start - query_start)) > MYSQL_YYABORT; > + if ($7) > + { > + elem->set_cycle_list($7); > + } > } > ; > > +opt_cycle: > + /* empty */ > + { $$= NULL; } > + | > + CYCLE_SYM > + { > + if (!Lex->curr_with_clause->with_recursive) > + { > + thd->parse_error(ER_SYNTAX_ERROR, $1.pos()); > + } > + } > + '(' with_column_list ')' Where did you see that the standard requires parentheses here? > + { > + $$= $4; > + } > + ; > + > > opt_with_column_list: > /* empty */ Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] Re MDEV-21610, cleanup patch
by Sergey Petrunia 25 Feb '20

25 Feb '20

Hello Igor, > @spetrunia: your "cleanup" of commit e637355156cb28388a291b0e3a5e9ee863b2854d > actually changes the architecture: you forced the pushed filter to be the > same as the filter of SQL layer. I'm not sure I understand that. I assume the layers are: 1. SQL layer 2. handler level (generic, with ha_xxx methods) 3. DS-MRR layer 4. The storage engine internals handler::pushed_rowid_filter is at level 2. Its value matches the rowid filter that has been pushed to level 4. What the code does after my fix: // Take the filter from level 2, put it to level 3 (DS-MRR) rowid_filter= h_arg->pushed_rowid_filter; // Inform level 4 that it doesn't need to do filtering // (This also affects level 2 as handler::pushed_rowid_filter will be // cleared) h_arg->cancel_pushed_rowid_filter(); I see a slight problem here that handler->pushed_rowid_filter will be NULL while actually storage engine + MRR (levels 2,3,4 together) will still be checking the rowid filter. But this seems to be unavoidable in current design (and it was also the case before my cleanup patch). > Although they are the same now for each engine where filters are allowed > this might be not the case in future development. I'm not sure what scenario you have in mind, could you please elaborate? > I agree that accessing filter from the handler layer is not a clean solution, > but in this case the owner of SQL layer filter must be the TABLE structure. > Summing up: if we decide not to use a pushed filter in the engine we have to > take the filter from SQL layer. So, you're suggesting that taking the filter from handler::pushed_rowid_filter is worse than taking it from table->reginfo.join_tab ? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] MDEV-21580: Allow packed sort keys in sort buffer
by Sergey Petrunia 20 Feb '20

20 Feb '20

Hi Varun, Please find some input below. > commit f75829eebe96db55508cbc03c967e1c340da0cfc > Author: Varun Gupta <varun.gupta(a)mariadb.com> > Date: Fri Feb 7 02:30:06 2020 +0530 > > MDEV-21580: Allow packed sort keys in sort buffer > > This task deals with packing the sort key inside the sort buffer, which would > lead to efficient usage of the memory allocated for the sort buffer. > > The changes brought by this feature are > 1) Sort buffers would have sort keys of variable length > 2) The format for sort keys inside the sort buffer would look like > |<sort_length><null_byte><key_part1><null_byte><key_part2>.......| > sort_length is the extra bytes that are required to store the variable > length of a sort key. > 3) When packing of sort key is done we store the ORIGINAL VALUES inside > the sort buffer and not the STRXFRM form (mem-comparable sort keys). > 4) Special comparison function packed_keys_comparison() is introduced > to compare 2 sort keys. > > diff --git a/mysql-test/main/mdev21580.result b/mysql-test/main/mdev21580.result > new file mode 100644 > index 00000000000..c504b79d52f > --- /dev/null > +++ b/mysql-test/main/mdev21580.result > @@ -0,0 +1,6427 @@ > +create table t1(a int); > diff --git a/mysql-test/main/mdev21580.test b/mysql-test/main/mdev21580.test This test is too long, I assume it will be gone after the "diff solution" is implemented. > diff --git a/sql/filesort.cc b/sql/filesort.cc > index 763f9f59246..1cdc8e7af00 100644 > --- a/sql/filesort.cc > +++ b/sql/filesort.cc > @@ -215,16 +219,14 @@ SORT_INFO *filesort(THD *thd, TABLE *table, Filesort *filesort, > error= 1; > sort->found_rows= HA_POS_ERROR; > > - param.init_for_filesort(sortlength(thd, filesort->sortorder, s_length, > - &multi_byte_charset), > - table, max_rows, filesort->sort_positions); > + param.sort_keys= sort_keys; > + uint sort_len= sortlength(thd, sort_keys, &multi_byte_charset, > + &allow_packing_for_sortkeys); psergey: I think this doesn't work as intended. Consider the two tests: create table t2 (a text collate utf8_unicode_ci, b varchar(32)); insert into t2 select a,a from ten; Q1: select * from t2 order by a; Q2: select * from t2 order by a, b; When debugging Q1, I can see allow_packing_for_sortkeys=false after the sortlength() call. This is ok I think. When debugging Q2, can see allow_packing_for_sortkeys= true after the sortlength() call. This is wrong. > @@ -491,12 +503,20 @@ uint Filesort::make_sortorder(THD *thd, JOIN *join, table_map first_table_bit) > for (ord = order; ord; ord= ord->next) > count++; > if (!sortorder) > - sortorder= (SORT_FIELD*) thd->alloc(sizeof(SORT_FIELD) * (count + 1)); > + sortorder= (SORT_FIELD*) thd->alloc(sizeof(SORT_FIELD) * count); > + void *rawmem= thd->alloc(sizeof(Sort_keys)); > pos= sort= sortorder; > > if (!pos) > DBUG_RETURN(0); > > + Sort_keys_array sort_keys_array(sortorder, count); > + Sort_keys* sort_keys= new (rawmem) Sort_keys(sort_keys_array); > + psergey: Why not inherit the class from Sql_alloc and use its operator new? The above is probably correct but it makes me wonder what is it doing every time I encounter this piece of code. > + if (!sort_keys) > + DBUG_RETURN(0); psergey: You're checking this (which cant fail) but not checking the result of thd->alloc() call above (which can fail)? > + > + pos= sort_keys->begin(); > for (ord= order; ord; ord= ord->next, pos++) > { > Item *first= ord->item[0]; ... > +/* > + @brief > + Comparison function to compare two packed sort keys > + > + @param sort_param cmp argument > + @param a_ptr packed sort key > + @param b_ptr packed sort key > + > + @retval > + >0 key a_ptr greater than b_ptr > + =0 key a_ptr equal to b_ptr > + <0 key a_ptr less than b_ptr > + > +*/ > + psergey: function names typically start with a verb. Can this one follow the convention? > +int packed_keys_comparison(void *sort_param, > + unsigned char **a_ptr, unsigned char **b_ptr) > +{ > + int retval= 0; > + size_t a_len, b_len; > + Sort_param *param= (Sort_param*)sort_param; > + Sort_keys *sort_keys= param->sort_keys; > + uchar *a= *a_ptr; > + uchar *b= *b_ptr; > + ... > @@ -772,6 +777,21 @@ static int rr_cmp(uchar *a,uchar *b) > #endif > } > > + > +/** > + Copy (unpack) values appended to sorted fields from a buffer back to > + their regular positions specified by the Field::ptr pointers. > + > + @param addon_field Array of descriptors for appended fields > + @param buff Buffer which to unpack the value from > + > + @note > + The function is supposed to be used only as a callback function > + when getting field values for the sorted result set. > + > + @return > + void. psergey: the above two lines do not have any value, please remove :) > +*/ > template<bool Packed_addon_fields> > inline void SORT_INFO::unpack_addon_fields(uchar *buff) > { BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 1

Re: [Maria-developers] 8790c095ab0: MDEV-21704 Add a new JSON field "version_id" into mysql.global_priv.priv
by Sergei Golubchik 20 Feb '20

20 Feb '20

Hi, Alexander! Looks ok, just one test related comment. On Feb 20, Alexander Barkov wrote: > revision-id: 8790c095ab0 (mariadb-10.5.0-225-g8790c095ab0) > parent(s): 2c34315df6e > author: Alexander Barkov <bar(a)mariadb.com> > committer: Alexander Barkov <bar(a)mariadb.com> > timestamp: 2020-02-14 21:56:12 +0400 > message: > > MDEV-21704 Add a new JSON field "version_id" into mysql.global_priv.priv > > +--log-error=$MYSQLTEST_VARDIR/tmp/system_mysql_db_error_log.err ... > +CREATE TABLE error_log (msg TEXT); > +--disable_query_log > +--eval LOAD DATA INFILE "$MYSQLTEST_VARDIR/tmp/system_mysql_db_error_log.err" INTO TABLE error_log; > +--enable_query_log > + > +# On some platforms hours less than 10 can be logged without the leading zero: > +# '2020-02-13 8:16:56' rather than > +# '2020-02-13 08:16:56' > +# Hence the space character in the hour part of the regular expression. > +# Also, replace '\r' to '', so the output is equal on Linux and Windows. > + > +SELECT > + REPLACE( > + REGEXP_REPLACE(msg, > + '^[0-9]{4}-[0-9]{2}-[0-9]{2} [ 0-9]{2}:[0-9]{2}:[0-9]{2}', > + 'YYYY-MM-DD hh:mm:ss'), > + '\r','' > + ) AS msg > +FROM error_log WHERE msg LIKE '%user%entry%'; instead of --log-error and LOAD DATA with REGEXP_REPLACE, please use something like --let SEARCH_FILE=$MYSQLTEST_VARDIR/log/mysqld.1.err --let SEARCH_PATTERN="'user' entry 'bad_.*has a wrong" --source include/search_pattern_in_file.inc this will print that there were 4 matches. Alternatively you can use four very specific SEARCH_PATTERN, one for each expected error. It's easier than what you've used, keeps the error log in one file, and doesn't run unnecessary sql on the server (otherwise debugging quickly becomes very annoying, grr, replication tests). Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] bd98ef106c8: MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
by Sergei Golubchik 20 Feb '20

20 Feb '20

Hi, Aleksey! 1. It should be possible to enable/disable auto-creation. For example, CREATE TABLE ... PARTITION BY SYSTEM_TIME ... PARTITIONS AUTO; this solves few problems at once: * a user explicitly tells when auto-creation should work * you don't need to worry "if this name is already occupied" * you can define that these partitions can never overflow (for INTERVAL) * if you know that AUTO partitions never overflow, you can keep the old behavior for ALTER TABLE ADD PARTITION. 2. Separate thread is an interesting solution. Nicely avoids lots of problems. Still: * it's asynchronous, all tests have to take it into account * it's racy. "low probability" or not, with our number of users they'll hit it and rightfully will report it as a bug * if LOCK TABLE prevents it, partitions can overflow But I think these problems are easier to solve than those we'll face if auto-creation will happen in the main connection thread. So, let's keep your approach with a thread. But instead of going through the parser and mysql_execute_command, create a function that takes a TABLE or a TABLE_SHARE. And adds a partition there. It'll fix the "racy" part. This function can be called from a new thread. As for the LOCK TABLES - if you're under LOCK TABLES, you can simply call that function (to add a partition) directly at the end of the main INSERT/UPDATE/DELETE statement. It'll solve the last problem, LOCK TABLES won't prevent auto-creation. A couple of style comments: 1. create a one-liner make_partition_name (or something) with: sprintf(move_ptr, "p%u", i) and use it both in create_default_partition_names() and in your new code. Just to make sure partition names are always generated uniformly by the same function and if we'll want to change it to "pp%u" it should be done only in one place. 2. don't overload operators, use, say, bool greater_than(size_t seconds) But overall it's pretty good. A nice idea with a separate thread. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org On Nov 07, Aleksey Midenkov wrote: > revision-id: bd98ef106c8 (mariadb-10.4.7-55-gbd98ef106c8) > parent(s): 6d1fa01e8f1 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2019-08-19 12:01:14 +0300 > message: > > MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT > > When there are E empty partitions left, auto-create N new empty > partitions. > > This scheme must not allow partition overflow. I.e. E-fill time must > not exceed N-creation time. This means that low values for INTERVAL > and LIMIT must not be allowed for auto-creation. In case when overflow > is detected there is no need to do anything special: a warning will be > issued and the user will run manual rebuild to redistribute records > correctly. This is important because automatic ADD must be done fast, > without forced rebuild, by the obvious reason. > > Initial version implements hard-coded values of 1 for E and N. As well > as auto-creation threshold MinInterval = 1 hour, MinLimit = 1000. > > The name for newly added partition will be first chosen as "pX", where > X is partition number and "p" is hard-coded name prefix. If this name > is already occupied, the X will be incremented until the resulting > name will be free to use. > > Auto-creation mechanism is applied to every table having LIMIT or > INTERVAL clause. Note that there is no much sense in specifying > explicit partition list in this case and this is covered by > MDEV-19903. The syntax to explicitly turn it on/off as well as > user-defined values for E, N and name prefix is subject for further > discussion and feature requests. > > ALTER TABLE ADD PARTITION is now always fast. If there some history > partition overflow occurs manual ALTER TABLE REBUILD PARTITION is > needed.

2 5

[Maria-developers] Fixing 10.4+ build on OpenBSD
by Brad Smith 16 Feb '20

16 Feb '20

Hi, Looking for some assistance in getting a patch commited to resolve MDEV-19603. Since Galera was upgraded from 3 to 4 the CMake bits include a hardcoded libdl which broke the build of 10.4+ on OpenBSD.

1 0

Re: [Maria-developers] [Commits] a906aae: MDEV-21610 Different query results from 10.4.11 to 10.4.12
by Sergey Petrunia 15 Feb '20

15 Feb '20

Hello Igor, I've found a deficiency in the scheme used by the patch: it doesn't work if the join buffer is re-filled multiple times. On the first execution, everything works as intended. dsmrr_init() executes these lines: rowid_filter= h_arg->pushed_rowid_filter; h_arg->cancel_pushed_rowid_filter(); then I can see that Mrr_ordered_rndpos_reader::refill_from_index_reader uses the filter. The MRR scan continues until it finishes. Then, SQL layer fills the join buffer again, and calls multi_range_read_init() again, which calls dsmrr_init(). And here, h_arg->pushed_rowid_filter==NULL (as we've cleared it previously), and the second MRR scan is not used anymore. Example that I used for debugging (maybe it's larger than necessary): create table t10 ( pk int primary key, a int, b int, filler char(100), key(a), key(b) ); insert into t10 select A.a + 1000 *B.a, A.a + 1000 *B.a, A.a + 1000 *B.a, 'filler-data=FILLER=DATA' from one_k A, one_k B; create table t11 (a int); insert into t11 select a from one_k where a < 200; set optimizer_switch='mrr=on'; set join_cache_level=6; set join_buffer_size=128; MariaDB [test]> explain select * from t11, t10 where t10.a=t11.a and t10.b < 300; +------+-------------+-------+------------+---------------+------+---------+------------+--------+-----------------------------------------------------------------------------------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+-------+------------+---------------+------+---------+------------+--------+-----------------------------------------------------------------------------------------+ | 1 | SIMPLE | t11 | ALL | NULL | NULL | NULL | NULL | 200 | Using where | | 1 | SIMPLE | t10 | ref|filter | a,b | a|b | 5|5 | test.t11.a | 1 (0%) | Using where; Using join buffer (flat, BKA join); Rowid-ordered scan; Using rowid filter | +------+-------------+-------+------------+---------------+------+---------+------------+--------+-----------------------------------------------------------------------------------------+ select * from t11, t10 where t10.a=t11.a and t10.b < 300; Ideas about the solution: 1. DS-MRR code should put the rowid filter back into the 'h_arg' handler when the scan finishes (i.e. returns HA_ERR_END_OF_FILE) 2. DS-MRR code should store the rowid filter internally (and clear it up when dsmrr_close() is called. I haven't investigated which is better. On Thu, Feb 13, 2020 at 10:55:56PM -0800, IgorBabaev wrote: > revision-id: a906aaee26a7be57fe2db62214179476ec124486 (mariadb-10.4.11-38-ga906aae) > parent(s): 7ea413ac2d80c7f03d1dbad90ac30ecddd8b2835 > author: Igor Babaev > committer: Igor Babaev > timestamp: 2020-02-13 22:55:56 -0800 > message: > > MDEV-21610 Different query results from 10.4.11 to 10.4.12 > > This patch fixes the following defects/bugs. > 1. If BKA[H] algorithm was used to join a table for which the optimizer > had decided to employ a rowid filter the filter actually was not built. > 2. The patch for the bug MDEV-21356 that added the code canceling pushing > rowid filter into an engine for the table joined with employment of > BKA[H] and MRR was not quite correct for Innodb engine because this > cancellation was done after InnoDB code had already bound the the pushed > filter to internal InnoDB structures. > > --- > mysql-test/main/rowid_filter_innodb.result | 333 +++++++++++++++++++++++++++++ > mysql-test/main/rowid_filter_innodb.test | 153 +++++++++++++ > sql/multi_range_read.cc | 39 ++-- > sql/multi_range_read.h | 5 +- > sql/opt_range.cc | 3 +- > sql/sql_join_cache.cc | 2 + > 6 files changed, 515 insertions(+), 20 deletions(-) > > diff --git a/mysql-test/main/rowid_filter_innodb.result b/mysql-test/main/rowid_filter_innodb.result > index c59b95b..9423fb1 100644 > --- a/mysql-test/main/rowid_filter_innodb.result > +++ b/mysql-test/main/rowid_filter_innodb.result > @@ -2522,3 +2522,336 @@ id select_type table type possible_keys key key_len ref rows r_rows filtered r_f > 1 SIMPLE t1 index a,b PRIMARY 4 NULL 3008 3008.00 1.36 0.00 Using where > DROP TABLE t1; > SET global innodb_stats_persistent= @stats.save; > +# > +# MDEV-21610: Using rowid filter with BKA+MRR > +# > +set @stats.save= @@innodb_stats_persistent; > +set global innodb_stats_persistent=on; > +CREATE TABLE acli ( > +id bigint(20) NOT NULL, > +rid varchar(255) NOT NULL, > +tp smallint(6) NOT NULL DEFAULT 0, > +PRIMARY KEY (id), > +KEY acli_rid (rid), > +KEY acli_tp (tp) > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > +insert into acli(id,rid,tp) values > +(184929059698905997,'ABABABABABABABABAB',103), > +(184929059698905998,'ABABABABABABABABAB',121), > +(283586039035985921,'00000000000000000000000000000000',103), > +(2216474704108064678,'020BED6D07B741CE9B10AB2200FEF1DF',103), > +(2216474704108064679,'020BED6D07B741CE9B10AB2200FEF1DF',121), > +(3080602882609775593,'B5FCC8C7111E4E3CBC21AAF5012F59C2',103), > +(3080602882609775594,'B5FCC8C7111E4E3CBC21AAF5012F59C2',121), > +(3080602882609776594,'B5FCC8C7111E4E3CBC21AAF5012F59C2',121), > +(3080602882609777595,'B5FCC8C7111E4E3CBC21AAF5012F59C2',121), > +(4269412446747236214,'SCSCSCSCSCSCSCSC',103), > +(4269412446747236215,'SCSCSCSCSCSCSCSC',121), > +(6341490487802728356,'6072D47E513F4A4794BBAB2200FDB67D',103), > +(6341490487802728357,'6072D47E513F4A4794BBAB2200FDB67D',121); > +CREATE TABLE acei ( > +id bigint(20) NOT NULL, > +aclid bigint(20) NOT NULL DEFAULT 0, > +atp smallint(6) NOT NULL DEFAULT 0, > +clus smallint(6) NOT NULL DEFAULT 0, > +PRIMARY KEY (id), > +KEY acei_aclid (aclid), > +KEY acei_clus (clus) > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > +insert into acei(id,aclid,atp,clus) values > +(184929059698905999,184929059698905997,0,1), > +(184929059698906000,184929059698905997,0,1), > +(184929059698906001,184929059698905997,1,1), > +(184929059698906002,184929059698905998,1,1), > +(283586039035985922,283586039035985921,1,1), > +(2216474704108064684,2216474704108064678,0,1), > +(2216474704108064685,2216474704108064678,0,1), > +(2216474704108064686,2216474704108064678,1,1), > +(2216474704108064687,2216474704108064679,1,1), > +(3080602882609775595,3080602882609775593,0,1), > +(3080602882609775596,3080602882609775593,0,1), > +(3080602882609775597,3080602882609775593,1,1), > +(3080602882609775598,3080602882609775594,1,1), > +(3080602882609776595,3080602882609776594,1,1), > +(3080602882609777596,3080602882609777595,1,1), > +(4269412446747236216,4269412446747236214,0,1), > +(4269412446747236217,4269412446747236214,0,1), > +(4269412446747236218,4269412446747236214,1,1), > +(4269412446747236219,4269412446747236215,1,1), > +(6341490487802728358,6341490487802728356,0,1), > +(6341490487802728359,6341490487802728356,0,1), > +(6341490487802728360,6341490487802728356,1,1), > +(6341490487802728361,6341490487802728357,1,1); > +CREATE TABLE filt ( > +id bigint(20) NOT NULL, > +aceid bigint(20) NOT NULL DEFAULT 0, > +clid smallint(6) NOT NULL DEFAULT 0, > +fh bigint(20) NOT NULL DEFAULT 0, > +PRIMARY KEY (id), > +KEY filt_aceid (aceid), > +KEY filt_clid (clid), > +KEY filt_fh (fh) > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > +insert into filt(id,aceid,clid,fh) values > +(184929059698905999,184929059698905999,1,8948400944397203540), > +(184929059698906000,184929059698906000,1,-3516039679025944536), > +(184929059698906001,184929059698906001,1,-3516039679025944536), > +(184929059698906002,184929059698906001,1,2965370193075218252), > +(184929059698906003,184929059698906001,1,8948400944397203540), > +(184929059698906004,184929059698906002,1,2478709353550777738), > +(283586039035985922,283586039035985922,1,5902600816362013271), > +(2216474704108064686,2216474704108064684,1,8948400944397203540), > +(2216474704108064687,2216474704108064685,1,-7244708939311117030), > +(2216474704108064688,2216474704108064686,1,-7244708939311117030), > +(2216474704108064689,2216474704108064686,1,7489060986210282479), > +(2216474704108064690,2216474704108064686,1,8948400944397203540), > +(2216474704108064691,2216474704108064687,1,-3575268945274980038), > +(3080602882609775595,3080602882609775595,1,8948400944397203540), > +(3080602882609775596,3080602882609775596,1,-5420422472375069774), > +(3080602882609775597,3080602882609775597,1,-5420422472375069774), > +(3080602882609775598,3080602882609775597,1,8518228073041491534), > +(3080602882609775599,3080602882609775597,1,8948400944397203540), > +(3080602882609775600,3080602882609775598,1,6311439873746261694), > +(3080602882609775601,3080602882609775598,1,6311439873746261694), > +(3080602882609776595,3080602882609776595,1,-661101805245999843), > +(3080602882609777596,3080602882609777596,1,-661101805245999843), > +(3080602882609777597,3080602882609777596,1,2216865386202464067), > +(4269412446747236216,4269412446747236216,1,8948400944397203540), > +(4269412446747236217,4269412446747236217,1,-1143096194892676000), > +(4269412446747236218,4269412446747236218,1,-1143096194892676000), > +(4269412446747236219,4269412446747236218,1,5313391811364818290), > +(4269412446747236220,4269412446747236218,1,8948400944397203540), > +(4269412446747236221,4269412446747236219,1,7624499822621753835), > +(6341490487802728358,6341490487802728358,1,8948400944397203540), > +(6341490487802728359,6341490487802728359,1,8141092449587136068), > +(6341490487802728360,6341490487802728360,1,8141092449587136068), > +(6341490487802728361,6341490487802728360,1,1291319099896431785), > +(6341490487802728362,6341490487802728360,1,8948400944397203540), > +(6341490487802728363,6341490487802728361,1,6701841652906431497); > +analyze table filt, acei, acli; > +Table Op Msg_type Msg_text > +test.filt analyze status Engine-independent statistics collected > +test.filt analyze status OK > +test.acei analyze status Engine-independent statistics collected > +test.acei analyze status OK > +test.acli analyze status Engine-independent statistics collected > +test.acli analyze status OK > +set @save_optimizer_switch=@@optimizer_switch; > +set @save_join_cache_level=@@join_cache_level; > +set optimizer_switch='mrr=off'; > +set join_cache_level=2; > +set statement optimizer_switch='rowid_filter=off' for explain extended select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t index_merge PRIMARY,acli_rid,acli_tp acli_tp,acli_rid 2,767 NULL 2 100.00 Using intersect(acli_tp,acli_rid); Using where; Using index > +1 SIMPLE a ref PRIMARY,acei_aclid acei_aclid 8 test.t.id 1 100.00 Using where > +1 SIMPLE fi ref filt_aceid,filt_fh filt_aceid 8 test.a.id 1 17.14 Using where > +Warnings: > +Note 1003 select `test`.`t`.`id` AS `id`,`test`.`fi`.`id` AS `id`,`test`.`fi`.`aceid` AS `aceid`,`test`.`fi`.`clid` AS `clid`,`test`.`fi`.`fh` AS `fh` from `test`.`acli` `t` join `test`.`acei` `a` join `test`.`filt` `fi` where `test`.`t`.`tp` = 121 and `test`.`a`.`atp` = 1 and `test`.`fi`.`aceid` = `test`.`a`.`id` and `test`.`a`.`aclid` = `test`.`t`.`id` and `test`.`t`.`rid` = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and `test`.`fi`.`fh` in (6311439873746261694,-397087483897438286,8518228073041491534,-5420422472375069774) > +set statement optimizer_switch='rowid_filter=off' for select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id id aceid clid fh > +3080602882609775594 3080602882609775600 3080602882609775598 1 6311439873746261694 > +3080602882609775594 3080602882609775601 3080602882609775598 1 6311439873746261694 > +set statement optimizer_switch='rowid_filter=on' for explain extended select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t index_merge PRIMARY,acli_rid,acli_tp acli_tp,acli_rid 2,767 NULL 2 100.00 Using intersect(acli_tp,acli_rid); Using where; Using index > +1 SIMPLE a ref PRIMARY,acei_aclid acei_aclid 8 test.t.id 1 100.00 Using where > +1 SIMPLE fi ref|filter filt_aceid,filt_fh filt_aceid|filt_fh 8|8 test.a.id 1 (17%) 17.14 Using where; Using rowid filter > +Warnings: > +Note 1003 select `test`.`t`.`id` AS `id`,`test`.`fi`.`id` AS `id`,`test`.`fi`.`aceid` AS `aceid`,`test`.`fi`.`clid` AS `clid`,`test`.`fi`.`fh` AS `fh` from `test`.`acli` `t` join `test`.`acei` `a` join `test`.`filt` `fi` where `test`.`t`.`tp` = 121 and `test`.`a`.`atp` = 1 and `test`.`fi`.`aceid` = `test`.`a`.`id` and `test`.`a`.`aclid` = `test`.`t`.`id` and `test`.`t`.`rid` = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and `test`.`fi`.`fh` in (6311439873746261694,-397087483897438286,8518228073041491534,-5420422472375069774) > +set statement optimizer_switch='rowid_filter=on' for select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id id aceid clid fh > +3080602882609775594 3080602882609775600 3080602882609775598 1 6311439873746261694 > +3080602882609775594 3080602882609775601 3080602882609775598 1 6311439873746261694 > +set optimizer_switch='mrr=on'; > +set join_cache_level=6; > +set statement optimizer_switch='rowid_filter=off' for explain extended select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t index_merge PRIMARY,acli_rid,acli_tp acli_tp,acli_rid 2,767 NULL 2 100.00 Using intersect(acli_tp,acli_rid); Using where; Using index > +1 SIMPLE a ref PRIMARY,acei_aclid acei_aclid 8 test.t.id 1 100.00 Using where; Using join buffer (flat, BKA join); Rowid-ordered scan > +1 SIMPLE fi ref filt_aceid,filt_fh filt_aceid 8 test.a.id 1 17.14 Using where; Using join buffer (incremental, BKA join); Rowid-ordered scan > +Warnings: > +Note 1003 select `test`.`t`.`id` AS `id`,`test`.`fi`.`id` AS `id`,`test`.`fi`.`aceid` AS `aceid`,`test`.`fi`.`clid` AS `clid`,`test`.`fi`.`fh` AS `fh` from `test`.`acli` `t` join `test`.`acei` `a` join `test`.`filt` `fi` where `test`.`t`.`tp` = 121 and `test`.`a`.`atp` = 1 and `test`.`fi`.`aceid` = `test`.`a`.`id` and `test`.`a`.`aclid` = `test`.`t`.`id` and `test`.`t`.`rid` = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and `test`.`fi`.`fh` in (6311439873746261694,-397087483897438286,8518228073041491534,-5420422472375069774) > +set statement optimizer_switch='rowid_filter=off' for select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id id aceid clid fh > +3080602882609775594 3080602882609775600 3080602882609775598 1 6311439873746261694 > +3080602882609775594 3080602882609775601 3080602882609775598 1 6311439873746261694 > +set statement optimizer_switch='rowid_filter=on' for explain extended select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id select_type table type possible_keys key key_len ref rows filtered Extra > +1 SIMPLE t index_merge PRIMARY,acli_rid,acli_tp acli_tp,acli_rid 2,767 NULL 2 100.00 Using intersect(acli_tp,acli_rid); Using where; Using index > +1 SIMPLE a ref PRIMARY,acei_aclid acei_aclid 8 test.t.id 1 100.00 Using where; Using join buffer (flat, BKA join); Rowid-ordered scan > +1 SIMPLE fi ref|filter filt_aceid,filt_fh filt_aceid|filt_fh 8|8 test.a.id 1 (17%) 17.14 Using where; Using join buffer (incremental, BKA join); Rowid-ordered scan; Using rowid filter > +Warnings: > +Note 1003 select `test`.`t`.`id` AS `id`,`test`.`fi`.`id` AS `id`,`test`.`fi`.`aceid` AS `aceid`,`test`.`fi`.`clid` AS `clid`,`test`.`fi`.`fh` AS `fh` from `test`.`acli` `t` join `test`.`acei` `a` join `test`.`filt` `fi` where `test`.`t`.`tp` = 121 and `test`.`a`.`atp` = 1 and `test`.`fi`.`aceid` = `test`.`a`.`id` and `test`.`a`.`aclid` = `test`.`t`.`id` and `test`.`t`.`rid` = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and `test`.`fi`.`fh` in (6311439873746261694,-397087483897438286,8518228073041491534,-5420422472375069774) > +set statement optimizer_switch='rowid_filter=on' for select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +id id aceid clid fh > +3080602882609775594 3080602882609775600 3080602882609775598 1 6311439873746261694 > +3080602882609775594 3080602882609775601 3080602882609775598 1 6311439873746261694 > +set statement optimizer_switch='rowid_filter=on' for analyze format=json select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > +inner join filt fi on a.id = fi.aceid > +where > +t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > +t.tp = 121 and > +a.atp = 1 and > +fi.fh in (6311439873746261694,-397087483897438286, > +8518228073041491534,-5420422472375069774); > +ANALYZE > +{ > + "query_block": { > + "select_id": 1, > + "r_loops": 1, > + "r_total_time_ms": "REPLACED", > + "table": { > + "table_name": "t", > + "access_type": "index_merge", > + "possible_keys": ["PRIMARY", "acli_rid", "acli_tp"], > + "key_length": "2,767", > + "index_merge": { > + "intersect": { > + "range": { > + "key": "acli_tp", > + "used_key_parts": ["tp"] > + }, > + "range": { > + "key": "acli_rid", > + "used_key_parts": ["rid"] > + } > + } > + }, > + "r_loops": 1, > + "rows": 2, > + "r_rows": 3, > + "r_total_time_ms": "REPLACED", > + "filtered": 100, > + "r_filtered": 100, > + "attached_condition": "t.tp = 121 and t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2'", > + "using_index": true > + }, > + "block-nl-join": { > + "table": { > + "table_name": "a", > + "access_type": "ref", > + "possible_keys": ["PRIMARY", "acei_aclid"], > + "key": "acei_aclid", > + "key_length": "8", > + "used_key_parts": ["aclid"], > + "ref": ["test.t.id"], > + "r_loops": 1, > + "rows": 1, > + "r_rows": 3, > + "r_total_time_ms": "REPLACED", > + "filtered": 100, > + "r_filtered": 100 > + }, > + "buffer_type": "flat", > + "buffer_size": "8Kb", > + "join_type": "BKA", > + "mrr_type": "Rowid-ordered scan", > + "attached_condition": "a.atp = 1", > + "r_filtered": 100 > + }, > + "block-nl-join": { > + "table": { > + "table_name": "fi", > + "access_type": "ref", > + "possible_keys": ["filt_aceid", "filt_fh"], > + "key": "filt_aceid", > + "key_length": "8", > + "used_key_parts": ["aceid"], > + "ref": ["test.a.id"], > + "rowid_filter": { > + "range": { > + "key": "filt_fh", > + "used_key_parts": ["fh"] > + }, > + "rows": 6, > + "selectivity_pct": 17.143, > + "r_rows": 5, > + "r_selectivity_pct": 40, > + "r_buffer_size": "REPLACED", > + "r_filling_time_ms": "REPLACED" > + }, > + "r_loops": 1, > + "rows": 1, > + "r_rows": 2, > + "r_total_time_ms": "REPLACED", > + "filtered": 17.143, > + "r_filtered": 100 > + }, > + "buffer_type": "incremental", > + "buffer_size": "603", > + "join_type": "BKA", > + "mrr_type": "Rowid-ordered scan", > + "attached_condition": "fi.fh in (6311439873746261694,-397087483897438286,8518228073041491534,-5420422472375069774)", > + "r_filtered": 100 > + } > + } > +} > +set optimizer_switch=@save_optimizer_switch; > +set join_cache_level=@save_join_cache_level; > +drop table filt, acei, acli; > +set global innodb_stats_persistent= @stats.save; > diff --git a/mysql-test/main/rowid_filter_innodb.test b/mysql-test/main/rowid_filter_innodb.test > index 30e0ede..74349b8 100644 > --- a/mysql-test/main/rowid_filter_innodb.test > +++ b/mysql-test/main/rowid_filter_innodb.test > @@ -381,3 +381,156 @@ ORDER BY pk LIMIT 1; > > DROP TABLE t1; > SET global innodb_stats_persistent= @stats.save; > + > +--echo # > +--echo # MDEV-21610: Using rowid filter with BKA+MRR > +--echo # > + > +set @stats.save= @@innodb_stats_persistent; > +set global innodb_stats_persistent=on; > + > +CREATE TABLE acli ( > + id bigint(20) NOT NULL, > + rid varchar(255) NOT NULL, > + tp smallint(6) NOT NULL DEFAULT 0, > + PRIMARY KEY (id), > + KEY acli_rid (rid), > + KEY acli_tp (tp) > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > + > +insert into acli(id,rid,tp) values > +(184929059698905997,'ABABABABABABABABAB',103), > +(184929059698905998,'ABABABABABABABABAB',121), > +(283586039035985921,'00000000000000000000000000000000',103), > +(2216474704108064678,'020BED6D07B741CE9B10AB2200FEF1DF',103), > +(2216474704108064679,'020BED6D07B741CE9B10AB2200FEF1DF',121), > +(3080602882609775593,'B5FCC8C7111E4E3CBC21AAF5012F59C2',103), > +(3080602882609775594,'B5FCC8C7111E4E3CBC21AAF5012F59C2',121), > +(3080602882609776594,'B5FCC8C7111E4E3CBC21AAF5012F59C2',121), > +(3080602882609777595,'B5FCC8C7111E4E3CBC21AAF5012F59C2',121), > +(4269412446747236214,'SCSCSCSCSCSCSCSC',103), > +(4269412446747236215,'SCSCSCSCSCSCSCSC',121), > +(6341490487802728356,'6072D47E513F4A4794BBAB2200FDB67D',103), > +(6341490487802728357,'6072D47E513F4A4794BBAB2200FDB67D',121); > + > +CREATE TABLE acei ( > + id bigint(20) NOT NULL, > + aclid bigint(20) NOT NULL DEFAULT 0, > + atp smallint(6) NOT NULL DEFAULT 0, > + clus smallint(6) NOT NULL DEFAULT 0, > + PRIMARY KEY (id), > + KEY acei_aclid (aclid), > + KEY acei_clus (clus) > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > + > +insert into acei(id,aclid,atp,clus) values > +(184929059698905999,184929059698905997,0,1), > +(184929059698906000,184929059698905997,0,1), > +(184929059698906001,184929059698905997,1,1), > +(184929059698906002,184929059698905998,1,1), > +(283586039035985922,283586039035985921,1,1), > +(2216474704108064684,2216474704108064678,0,1), > +(2216474704108064685,2216474704108064678,0,1), > +(2216474704108064686,2216474704108064678,1,1), > +(2216474704108064687,2216474704108064679,1,1), > +(3080602882609775595,3080602882609775593,0,1), > +(3080602882609775596,3080602882609775593,0,1), > +(3080602882609775597,3080602882609775593,1,1), > +(3080602882609775598,3080602882609775594,1,1), > +(3080602882609776595,3080602882609776594,1,1), > +(3080602882609777596,3080602882609777595,1,1), > +(4269412446747236216,4269412446747236214,0,1), > +(4269412446747236217,4269412446747236214,0,1), > +(4269412446747236218,4269412446747236214,1,1), > +(4269412446747236219,4269412446747236215,1,1), > +(6341490487802728358,6341490487802728356,0,1), > +(6341490487802728359,6341490487802728356,0,1), > +(6341490487802728360,6341490487802728356,1,1), > +(6341490487802728361,6341490487802728357,1,1); > + > +CREATE TABLE filt ( > + id bigint(20) NOT NULL, > + aceid bigint(20) NOT NULL DEFAULT 0, > + clid smallint(6) NOT NULL DEFAULT 0, > + fh bigint(20) NOT NULL DEFAULT 0, > + PRIMARY KEY (id), > + KEY filt_aceid (aceid), > + KEY filt_clid (clid), > + KEY filt_fh (fh) > +) ENGINE=InnoDB DEFAULT CHARSET=utf8; > + > +insert into filt(id,aceid,clid,fh) values > +(184929059698905999,184929059698905999,1,8948400944397203540), > +(184929059698906000,184929059698906000,1,-3516039679025944536), > +(184929059698906001,184929059698906001,1,-3516039679025944536), > +(184929059698906002,184929059698906001,1,2965370193075218252), > +(184929059698906003,184929059698906001,1,8948400944397203540), > +(184929059698906004,184929059698906002,1,2478709353550777738), > +(283586039035985922,283586039035985922,1,5902600816362013271), > +(2216474704108064686,2216474704108064684,1,8948400944397203540), > +(2216474704108064687,2216474704108064685,1,-7244708939311117030), > +(2216474704108064688,2216474704108064686,1,-7244708939311117030), > +(2216474704108064689,2216474704108064686,1,7489060986210282479), > +(2216474704108064690,2216474704108064686,1,8948400944397203540), > +(2216474704108064691,2216474704108064687,1,-3575268945274980038), > +(3080602882609775595,3080602882609775595,1,8948400944397203540), > +(3080602882609775596,3080602882609775596,1,-5420422472375069774), > +(3080602882609775597,3080602882609775597,1,-5420422472375069774), > +(3080602882609775598,3080602882609775597,1,8518228073041491534), > +(3080602882609775599,3080602882609775597,1,8948400944397203540), > +(3080602882609775600,3080602882609775598,1,6311439873746261694), > +(3080602882609775601,3080602882609775598,1,6311439873746261694), > +(3080602882609776595,3080602882609776595,1,-661101805245999843), > +(3080602882609777596,3080602882609777596,1,-661101805245999843), > +(3080602882609777597,3080602882609777596,1,2216865386202464067), > +(4269412446747236216,4269412446747236216,1,8948400944397203540), > +(4269412446747236217,4269412446747236217,1,-1143096194892676000), > +(4269412446747236218,4269412446747236218,1,-1143096194892676000), > +(4269412446747236219,4269412446747236218,1,5313391811364818290), > +(4269412446747236220,4269412446747236218,1,8948400944397203540), > +(4269412446747236221,4269412446747236219,1,7624499822621753835), > +(6341490487802728358,6341490487802728358,1,8948400944397203540), > +(6341490487802728359,6341490487802728359,1,8141092449587136068), > +(6341490487802728360,6341490487802728360,1,8141092449587136068), > +(6341490487802728361,6341490487802728360,1,1291319099896431785), > +(6341490487802728362,6341490487802728360,1,8948400944397203540), > +(6341490487802728363,6341490487802728361,1,6701841652906431497); > + > +analyze table filt, acei, acli; > + > +let $q= > +select t.id, fi.* > +from (acli t inner join acei a on a.aclid = t.id) > + inner join filt fi on a.id = fi.aceid > + where > + t.rid = 'B5FCC8C7111E4E3CBC21AAF5012F59C2' and > + t.tp = 121 and > + a.atp = 1 and > + fi.fh in (6311439873746261694,-397087483897438286, > + 8518228073041491534,-5420422472375069774); > + > +set @save_optimizer_switch=@@optimizer_switch; > +set @save_join_cache_level=@@join_cache_level; > + > +set optimizer_switch='mrr=off'; > +set join_cache_level=2; > +eval $without_filter explain extended $q; > +eval $without_filter $q; > +eval $with_filter explain extended $q; > +eval $with_filter $q; > + > +set optimizer_switch='mrr=on'; > +set join_cache_level=6; > +eval $without_filter explain extended $q; > +eval $without_filter $q; > +eval $with_filter explain extended $q; > +eval $with_filter $q; > +--source include/analyze-format.inc > +eval $with_filter analyze format=json $q; > + > +set optimizer_switch=@save_optimizer_switch; > +set join_cache_level=@save_join_cache_level; > + > +drop table filt, acei, acli; > + > +set global innodb_stats_persistent= @stats.save; > diff --git a/sql/multi_range_read.cc b/sql/multi_range_read.cc > index 7e4c2ed..daeb53d 100644 > --- a/sql/multi_range_read.cc > +++ b/sql/multi_range_read.cc > @@ -702,7 +702,8 @@ static int rowid_cmp_reverse(void *file, uchar *a, uchar *b) > int Mrr_ordered_rndpos_reader::init(handler *h_arg, > Mrr_index_reader *index_reader_arg, > uint mode, > - Lifo_buffer *buf) > + Lifo_buffer *buf, > + Rowid_filter *filter) > { > file= h_arg; > index_reader= index_reader_arg; > @@ -710,19 +711,7 @@ int Mrr_ordered_rndpos_reader::init(handler *h_arg, > is_mrr_assoc= !MY_TEST(mode & HA_MRR_NO_ASSOCIATION); > index_reader_exhausted= FALSE; > index_reader_needs_refill= TRUE; > - > - /* > - Currently usage of a rowid filter within InnoDB engine is not supported > - if the table is accessed by the primary key. > - With optimizer switches ''mrr' and 'mrr_sort_keys' are both enabled > - any access by a secondary index is converted to the rndpos access. In > - InnoDB the rndpos access is always uses the primary key. > - Do not use pushed rowid filter if the table is accessed actually by the > - primary key. Use the rowid filter outside the engine code (see > - Mrr_ordered_rndpos_reader::refill_from_index_reader). > - */ > - if (file->pushed_rowid_filter && file->primary_key_is_clustered()) > - file->cancel_pushed_rowid_filter(); > + rowid_filter= filter; > > return 0; > } > @@ -817,10 +806,8 @@ int Mrr_ordered_rndpos_reader::refill_from_index_reader() > index_reader->position(); > > /* > - If the built rowid filter cannot be used at the engine level use it here. > + If the built rowid filter cannot be used at the engine level, use it here. > */ > - Rowid_filter *rowid_filter= > - file->get_table()->reginfo.join_tab->rowid_filter; > if (rowid_filter && !file->pushed_rowid_filter && > !rowid_filter->check((char *)index_rowid)) > continue; > @@ -967,6 +954,7 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, > handler *h_idx; > Mrr_ordered_rndpos_reader *disk_strategy= NULL; > bool do_sort_keys= FALSE; > + Rowid_filter *rowid_filter= NULL; > DBUG_ENTER("DsMrr_impl::dsmrr_init"); > /* > index_merge may invoke a scan on an object for which dsmrr_info[_const] > @@ -1015,6 +1003,21 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, > if (!(keyno == table->s->primary_key && h_idx->primary_key_is_clustered())) > { > strategy= disk_strategy= &reader_factory.ordered_rndpos_reader; > + if (h_arg->pushed_rowid_filter) > + { > + /* > + Currently usage of a rowid filter within InnoDB engine is not supported > + if the table is accessed by the primary key. > + With optimizer switches ''mrr' and 'mrr_sort_keys' are both enabled > + any access by a secondary index is converted to the rndpos access. In > + InnoDB the rndpos access is always uses the primary key. > + Do not use pushed rowid filter if the table is accessed actually by the > + primary key. Use the rowid filter outside the engine code (see > + Mrr_ordered_rndpos_reader::refill_from_index_reader). > + */ > + rowid_filter= h_arg->pushed_rowid_filter; > + h_arg->cancel_pushed_rowid_filter(); > + } > } > > full_buf= buf->buffer; > @@ -1101,7 +1104,7 @@ int DsMrr_impl::dsmrr_init(handler *h_arg, RANGE_SEQ_IF *seq_funcs, > n_ranges, mode, &keypar, key_buffer, > &buf_manager)) || > (res= disk_strategy->init(primary_file, index_strategy, mode, > - &rowid_buffer))) > + &rowid_buffer, rowid_filter))) > { > goto error; > } > diff --git a/sql/multi_range_read.h b/sql/multi_range_read.h > index 0473fef..6be9537 100644 > --- a/sql/multi_range_read.h > +++ b/sql/multi_range_read.h > @@ -364,7 +364,7 @@ class Mrr_ordered_rndpos_reader : public Mrr_reader > { > public: > int init(handler *file, Mrr_index_reader *index_reader, uint mode, > - Lifo_buffer *buf); > + Lifo_buffer *buf, Rowid_filter *filter); > int get_next(range_id_t *range_info); > int refill_buffer(bool initial); > private: > @@ -399,6 +399,9 @@ class Mrr_ordered_rndpos_reader : public Mrr_reader > /* Buffer to store (rowid, range_id) pairs */ > Lifo_buffer *rowid_buffer; > > + /* Rowid filter to be checked against (if any) */ > + Rowid_filter *rowid_filter; > + > int refill_from_index_reader(); > }; > > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index c47da28..5f034c6 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -2902,7 +2902,8 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > remove_nonrange_trees(&param, tree); > > /* Get best 'range' plan and prepare data for making other plans */ > - if ((range_trp= get_key_scans_params(&param, tree, FALSE, TRUE, > + if ((range_trp= get_key_scans_params(&param, tree, > + only_single_index_range_scan, TRUE, > best_read_time))) > { > best_trp= range_trp; > diff --git a/sql/sql_join_cache.cc b/sql/sql_join_cache.cc > index 3a509b3..e9ad538 100644 > --- a/sql/sql_join_cache.cc > +++ b/sql/sql_join_cache.cc > @@ -2248,6 +2248,8 @@ enum_nested_loop_state JOIN_CACHE::join_matching_records(bool skip_last) > if ((rc= join_tab_execution_startup(join_tab)) < 0) > goto finish2; > > + join_tab->build_range_rowid_filter_if_needed(); > + > /* Prepare to retrieve all records of the joined table */ > if (unlikely((error= join_tab_scan->open()))) > { > _______________________________________________ > 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

1 0

[Maria-developers] MDEV-7381
by Gabriel Ciciliani 11 Feb '20

11 Feb '20

Hi MariaDB developers, I want to start working on this ticket but I can't assing it to me with my current JIRA privileges. Someone higher in the chain should do it for me or I nee to get my JIRA account fixed? Thanks!

2 1