December 2018 - developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 2da93752430: MDEV-17255: New optimizer defaults and ANALYZE TABLE
by Sergey Petrunia 06 Dec '18

06 Dec '18

Hi Varun, Nice to see that most of the input has been addressed. However, the issue of missing test coverage is still not resolved: On Sat, Nov 24, 2018 at 09:47:57PM +0300, Sergey Petrunia wrote: >> Please also add test coverage for this particular MDEV. This should be a >> testcase that shows that >> >> - with PREFERABLY, ANALYZE will collect EITS stats, SELECT will use them. >> >> - with PREFERABLY_FOR_READS : >> -- ANALYZE will not collect EITS stats >> -- ANALYZE .. PERSISTENT FOR ... will update the EITS stats. >> -- SELECT will use them. and now it is still not addressed. I made this change: @@ -104,6 +104,8 @@ Use_stat_tables_mode get_use_stat_tables_mode(THD *thd) inline bool check_eits_collection_allowed(THD *thd) { + if (get_use_stat_tables_mode(thd) == PREFERABLY_FOR_QUERIES) + DBUG_ASSERT(0); return (get_use_stat_tables_mode(thd) == COMPLEMENTARY || get_use_stat_tables_mode(thd) == PREFERABLY); } and the testsuite still passes! On Wed, Dec 05, 2018 at 07:57:06PM +0530, Varun wrote: > revision-id: 2da93752430d0dd07344c9d62f2468bba8962b77 (mariadb-10.3.6-266-g2da93752430) > parent(s): e0739064450f2c2be6d5de1d799582121747dd39 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2018-12-05 19:56:23 +0530 > message: > > MDEV-17255: New optimizer defaults and ANALYZE TABLE > > Added to new values to the server variable use_stat_tables. > The values are COMPLEMENTARY_FOR_QUERIES and PREFERABLY_FOR_QUERIES. > Both these values don't allow to collect EITS for queries like > analyze table t1; > To collect EITS we would need to use the syntax with persistent like > analyze table t1 persistent for columns (col1,col2...) index (idx1, idx2...) / ALL > > Changing the default value from NEVER to PREFERABLY_FOR_QUERIES. > > --- > mysql-test/include/default_mysqld.cnf | 1 + > mysql-test/main/stat_tables.result | 59 +++++++++++++++++++++++++++++++ > mysql-test/main/stat_tables.test | 43 ++++++++++++++++++++++ > mysql-test/main/stat_tables_innodb.result | 59 +++++++++++++++++++++++++++++++ > sql/sql_admin.cc | 2 +- > sql/sql_statistics.cc | 5 ++- > sql/sql_statistics.h | 27 ++++++++++++++ > sql/sys_vars.cc | 5 +-- > 8 files changed, 195 insertions(+), 6 deletions(-) > > diff --git a/mysql-test/include/default_mysqld.cnf b/mysql-test/include/default_mysqld.cnf > index 69a2b58288b..5ba3bdeb92c 100644 > --- a/mysql-test/include/default_mysqld.cnf > +++ b/mysql-test/include/default_mysqld.cnf > @@ -107,6 +107,7 @@ loose-performance-schema-consumer-thread-instrumentation=ON > binlog-direct-non-transactional-updates > > default-storage-engine=myisam > +use-stat-tables=preferably_for_queries > > loose-ssl-ca=(a)ENV.MYSQL_TEST_DIR/std_data/cacert.pem > loose-ssl-cert=(a)ENV.MYSQL_TEST_DIR/std_data/server-cert.pem > diff --git a/mysql-test/main/stat_tables.result b/mysql-test/main/stat_tables.result > index 308529ece47..67bf3f9237d 100644 > --- a/mysql-test/main/stat_tables.result > +++ b/mysql-test/main/stat_tables.result > @@ -605,4 +605,63 @@ SELECT MAX(pk) FROM t1; > MAX(pk) > NULL > DROP TABLE t1; > +# > +# MDEV-17255: New optimizer defaults and ANALYZE TABLE > +# > +set @save_use_stat_tables= @@use_stat_tables; > +create table t1 (a int, b int); > +insert into t1(a,b) values (1,2),(1,3),(1,4),(1,5),(2,6),(2,7),(3,8),(3,9),(3,9),(4,10); > +# > +# with use_stat_tables= PREFERABLY_FOR_QUERIES > +# analyze table t1 will not collect statistics > +# > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status OK > +select * from mysql.column_stats; > +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram > +test t1 a 1 4 0.0000 4.0000 2.5000 0 NULL NULL > +test t1 b 2 10 0.0000 4.0000 1.1111 0 NULL NULL > +analyze > +select * from t1 where a = 1 and b=3; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 10 10.00 2.78 10.00 Using where > +# > +# with use_stat_tables= PREFERABLY_FOR_QUERIES > +# analyze table t1 will collect statistics if we use PERSISTENT > +# for columns, indexes or everything > +# > +analyze table t1 persistent for columns (a) indexes (); > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status Table is already up to date > +select * from mysql.column_stats; > +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram > +test t1 a 1 4 0.0000 4.0000 2.5000 0 NULL NULL > +test t1 b 2 10 0.0000 4.0000 1.1111 0 NULL NULL > +# filtered shows that we used the data from stat tables > +analyze > +select * from t1 where a = 1 and b=3; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 10 10.00 2.78 10.00 Using where > +# > +# with use_stat_tables= PREFERABLY > +# analyze table t1 will collect statistics > +# > +set @@use_stat_tables=PREFERABLY; > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status Table is already up to date > +select * from mysql.column_stats; > +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram > +test t1 a 1 4 0.0000 4.0000 2.5000 0 NULL NULL > +test t1 b 2 10 0.0000 4.0000 1.1111 0 NULL NULL > +# filtered shows that we used the data from stat tables > +analyze > +select * from t1 where a=1 and b=3; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 10 10.00 2.78 10.00 Using where > +drop table t1; > set use_stat_tables=@save_use_stat_tables; > diff --git a/mysql-test/main/stat_tables.test b/mysql-test/main/stat_tables.test > index 19bc0fa2f46..8616f6386e1 100644 > --- a/mysql-test/main/stat_tables.test > +++ b/mysql-test/main/stat_tables.test > @@ -384,4 +384,47 @@ SELECT MAX(pk) FROM t1; > > DROP TABLE t1; > > +--echo # > +--echo # MDEV-17255: New optimizer defaults and ANALYZE TABLE > +--echo # > + > +set @save_use_stat_tables= @@use_stat_tables; > +create table t1 (a int, b int); > +insert into t1(a,b) values (1,2),(1,3),(1,4),(1,5),(2,6),(2,7),(3,8),(3,9),(3,9),(4,10); > + > +--echo # > +--echo # with use_stat_tables= PREFERABLY_FOR_QUERIES > +--echo # analyze table t1 will not collect statistics > +--echo # > + > +analyze table t1; > +select * from mysql.column_stats; > +analyze > +select * from t1 where a = 1 and b=3; > + > +--echo # > +--echo # with use_stat_tables= PREFERABLY_FOR_QUERIES > +--echo # analyze table t1 will collect statistics if we use PERSISTENT > +--echo # for columns, indexes or everything > +--echo # > + > +analyze table t1 persistent for columns (a) indexes (); > +select * from mysql.column_stats; > +--echo # filtered shows that we used the data from stat tables > +analyze > +select * from t1 where a = 1 and b=3; > + > +--echo # > +--echo # with use_stat_tables= PREFERABLY > +--echo # analyze table t1 will collect statistics > +--echo # > + > +set @@use_stat_tables=PREFERABLY; > +analyze table t1; > +select * from mysql.column_stats; > +--echo # filtered shows that we used the data from stat tables > +analyze > +select * from t1 where a=1 and b=3; > +drop table t1; > + > set use_stat_tables=@save_use_stat_tables; > diff --git a/mysql-test/main/stat_tables_innodb.result b/mysql-test/main/stat_tables_innodb.result > index 8198e94dc10..4bb8f34712c 100644 > --- a/mysql-test/main/stat_tables_innodb.result > +++ b/mysql-test/main/stat_tables_innodb.result > @@ -632,6 +632,65 @@ SELECT MAX(pk) FROM t1; > MAX(pk) > NULL > DROP TABLE t1; > +# > +# MDEV-17255: New optimizer defaults and ANALYZE TABLE > +# > +set @save_use_stat_tables= @@use_stat_tables; > +create table t1 (a int, b int); > +insert into t1(a,b) values (1,2),(1,3),(1,4),(1,5),(2,6),(2,7),(3,8),(3,9),(3,9),(4,10); > +# > +# with use_stat_tables= PREFERABLY_FOR_QUERIES > +# analyze table t1 will not collect statistics > +# > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status OK > +select * from mysql.column_stats; > +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram > +test t1 a 1 4 0.0000 4.0000 2.5000 0 NULL NULL > +test t1 b 2 10 0.0000 4.0000 1.1111 0 NULL NULL > +analyze > +select * from t1 where a = 1 and b=3; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 10 10.00 2.78 10.00 Using where > +# > +# with use_stat_tables= PREFERABLY_FOR_QUERIES > +# analyze table t1 will collect statistics if we use PERSISTENT > +# for columns, indexes or everything > +# > +analyze table t1 persistent for columns (a) indexes (); > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status OK > +select * from mysql.column_stats; > +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram > +test t1 a 1 4 0.0000 4.0000 2.5000 0 NULL NULL > +test t1 b 2 10 0.0000 4.0000 1.1111 0 NULL NULL > +# filtered shows that we used the data from stat tables > +analyze > +select * from t1 where a = 1 and b=3; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 10 10.00 2.78 10.00 Using where > +# > +# with use_stat_tables= PREFERABLY > +# analyze table t1 will collect statistics > +# > +set @@use_stat_tables=PREFERABLY; > +analyze table t1; > +Table Op Msg_type Msg_text > +test.t1 analyze status Engine-independent statistics collected > +test.t1 analyze status OK > +select * from mysql.column_stats; > +db_name table_name column_name min_value max_value nulls_ratio avg_length avg_frequency hist_size hist_type histogram > +test t1 a 1 4 0.0000 4.0000 2.5000 0 NULL NULL > +test t1 b 2 10 0.0000 4.0000 1.1111 0 NULL NULL > +# filtered shows that we used the data from stat tables > +analyze > +select * from t1 where a=1 and b=3; > +id select_type table type possible_keys key key_len ref rows r_rows filtered r_filtered Extra > +1 SIMPLE t1 ALL NULL NULL NULL NULL 10 10.00 2.78 10.00 Using where > +drop table t1; > set use_stat_tables=@save_use_stat_tables; > set optimizer_switch=@save_optimizer_switch_for_stat_tables_test; > SET SESSION STORAGE_ENGINE=DEFAULT; > diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc > index d0d959de8f9..b39103e382a 100644 > --- a/sql/sql_admin.cc > +++ b/sql/sql_admin.cc > @@ -767,7 +767,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, > } > collect_eis= > (table->table->s->table_category == TABLE_CATEGORY_USER && > - (get_use_stat_tables_mode(thd) > NEVER || > + (check_eits_collection_allowed(thd) || > lex->with_persistent_for_clause)); > > > diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc > index 04806f07b3b..8c88f7f927f 100644 > --- a/sql/sql_statistics.cc > +++ b/sql/sql_statistics.cc > @@ -3720,9 +3720,8 @@ void set_statistics_for_table(THD *thd, TABLE *table) > { > TABLE_STATISTICS_CB *stats_cb= &table->s->stats_cb; > Table_statistics *read_stats= stats_cb->table_stats; > - Use_stat_tables_mode use_stat_table_mode= get_use_stat_tables_mode(thd); > table->used_stat_records= > - (use_stat_table_mode <= COMPLEMENTARY || > + (!check_eits_preferred(thd) || > !table->stats_is_read || read_stats->cardinality_is_null) ? > table->file->stats.records : read_stats->cardinality; > KEY *key_info, *key_info_end; > @@ -3730,7 +3729,7 @@ void set_statistics_for_table(THD *thd, TABLE *table) > key_info < key_info_end; key_info++) > { > key_info->is_statistics_from_stat_tables= > - (use_stat_table_mode > COMPLEMENTARY && > + (check_eits_preferred(thd) && > table->stats_is_read && > key_info->read_stats->avg_frequency_is_inited() && > key_info->read_stats->get_avg_frequency(0) > 0.5); > diff --git a/sql/sql_statistics.h b/sql/sql_statistics.h > index 39cddf95188..8439ac8db53 100644 > --- a/sql/sql_statistics.h > +++ b/sql/sql_statistics.h > @@ -16,12 +16,26 @@ > #ifndef SQL_STATISTICS_H > #define SQL_STATISTICS_H > > +/* > + For COMPLEMENTARY_FOR_QUERIES and PREFERABLY_FOR_QUERIES they are > + similar to the COMPLEMENTARY and PREFERABLY respectively except that > + with these values we would not be collecting EITS for queries like > + ANALYZE TABLE t1; > + To collect EITS with these values, we have to use PERSISITENT FOR > + analyze table t1 persistent for > + columns (col1,col2...) index (idx1, idx2...) > + or > + analyze table t1 persistent for all > +*/ > + > typedef > enum enum_use_stat_tables_mode > { > NEVER, > COMPLEMENTARY, > PREFERABLY, > + COMPLEMENTARY_FOR_QUERIES, > + PREFERABLY_FOR_QUERIES > } Use_stat_tables_mode; > > typedef > @@ -87,6 +101,19 @@ Use_stat_tables_mode get_use_stat_tables_mode(THD *thd) > { > return (Use_stat_tables_mode) (thd->variables.use_stat_tables); > } > +inline > +bool check_eits_collection_allowed(THD *thd) > +{ > + return (get_use_stat_tables_mode(thd) == COMPLEMENTARY || > + get_use_stat_tables_mode(thd) == PREFERABLY); > +} > + > +inline > +bool check_eits_preferred(THD *thd) > +{ > + return (get_use_stat_tables_mode(thd) == PREFERABLY || > + get_use_stat_tables_mode(thd) == PREFERABLY_FOR_QUERIES); > +} > > int read_statistics_for_tables_if_needed(THD *thd, TABLE_LIST *tables); > int collect_statistics_for_table(THD *thd, TABLE *table); > diff --git a/sql/sys_vars.cc b/sql/sys_vars.cc > index 420e7feabde..b925de34db4 100644 > --- a/sql/sys_vars.cc > +++ b/sql/sys_vars.cc > @@ -5842,12 +5842,13 @@ static Sys_var_ulong Sys_progress_report_time( > VALID_RANGE(0, UINT_MAX), DEFAULT(5), BLOCK_SIZE(1)); > > const char *use_stat_tables_modes[] = > - {"NEVER", "COMPLEMENTARY", "PREFERABLY", 0}; > + {"NEVER", "COMPLEMENTARY", "PREFERABLY", > + "COMPLEMENTARY_FOR_QUERIES", "PREFERABLY_FOR_QUERIES", 0}; > static Sys_var_enum Sys_optimizer_use_stat_tables( > "use_stat_tables", > "Specifies how to use system statistics tables", > SESSION_VAR(use_stat_tables), CMD_LINE(REQUIRED_ARG), > - use_stat_tables_modes, DEFAULT(2)); > + use_stat_tables_modes, DEFAULT(4)); > > static Sys_var_ulong Sys_histogram_size( > "histogram_size", > _______________________________________________ > 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

Re: [Maria-developers] af6a8ea0f79: MDEV-16266 : New command FLUSH SSL ro reload SSL acceptor context.
by Sergei Golubchik 06 Dec '18

06 Dec '18

Hi, Vladislav! Again, that's quite ok. I don't like a new lock around sslaccept(), but I couldn't find any other solution for it :( See few comments below. On Dec 03, Vladislav Vaintroub wrote: > revision-id: af6a8ea0f79d628384a61986482dec7d3ff751dd (mariadb-10.4.0-55-gaf6a8ea0f79) > parent(s): 757530b83ca1cc550127311dc6edb47152ba0c6a > author: Vladislav Vaintroub <wlad(a)mariadb.com> > committer: Vladislav Vaintroub <wlad(a)mariadb.com> > timestamp: 2018-12-02 01:25:04 +0100 > message: > > MDEV-16266 : New command FLUSH SSL ro reload SSL acceptor context. > > Reloads certificate, key, CA, CRL. > > diff --git a/mysql-test/main/flush_ssl.test b/mysql-test/main/flush_ssl.test > new file mode 100644 > index 00000000000..24dbb4719f3 > --- /dev/null > +++ b/mysql-test/main/flush_ssl.test > @@ -0,0 +1,24 @@ > +source include/have_ssl_communication.inc; > + > +connect ssl_con,localhost,root,,,,,SSL; > +SELECT VARIABLE_VALUE != '0' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='SSL_ACCEPTS'; > +FLUSH SSL; > + > +# Check if SSL_ACCEPTS was flushed (SSL_ACCEPTS = 0) > +SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='SSL_ACCEPTS'; How about the test that a certificate was actually reloaded? That's the main functionality, right? Nobody wanted a new command just to reset SSL_ACCEPTS Note, that if you want to generate a new certificate for mysql-test, don't forget to extend mysql-test/lib/generate-ssl-certs.sh > + > +CREATE USER u; > +connect ssl_con2,localhost,u,,,,,SSL; > + > +# Check SSL_ACCEPTS increased to 1 > +SELECT VARIABLE_VALUE FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME='SSL_ACCEPTS'; > + > +# Check that FLUSH SSL does not work for unprivileged user > +error ER_SPECIFIC_ACCESS_DENIED_ERROR; > +FLUSH SSL; > + > +disconnect ssl_con2; > +disconnect ssl_con; > +connection default; > +DROP USER u; > + > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index 9ff47dc1ff1..13aad0a0197 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -4766,6 +4771,56 @@ static void openssl_lock(int mode, openssl_lock_t *lock, const char *file, > } > #endif /* HAVE_OPENSSL10 */ > > + > +struct SSL_ACCEPTOR_STATS > +{ > + long accept; > + long accept_good; > + long cache_size; > + long verify_mode; > + long verify_depth; > + const char *session_cache_mode; > + long zero; > + SSL_ACCEPTOR_STATS() > + { > + session_cache_mode = "NONE"; are you relying on everything else being zero'ed by default because SSL_ACCEPTOR_STATS is instantiated statically? May be, still, add a bzero? > + } > + void init() > + { > + DBUG_ASSERT(ssl_acceptor_fd); > + accept = 0; > + accept_good = 0; > + verify_mode = SSL_CTX_get_verify_mode(ssl_acceptor_fd->ssl_context); > + verify_depth = SSL_CTX_get_verify_depth(ssl_acceptor_fd->ssl_context); > + cache_size = SSL_CTX_sess_get_cache_size(ssl_acceptor_fd->ssl_context); > + switch (SSL_CTX_get_session_cache_mode(ssl_acceptor_fd->ssl_context)) > + { > + case SSL_SESS_CACHE_OFF: > + session_cache_mode = "OFF"; break; > + case SSL_SESS_CACHE_CLIENT: > + session_cache_mode = "CLIENT"; break; > + case SSL_SESS_CACHE_SERVER: > + session_cache_mode = "SERVER"; break; > + case SSL_SESS_CACHE_BOTH: > + session_cache_mode = "BOTH"; break; > + case SSL_SESS_CACHE_NO_AUTO_CLEAR: > + session_cache_mode = "NO_AUTO_CLEAR"; break; > + case SSL_SESS_CACHE_NO_INTERNAL_LOOKUP: > + session_cache_mode = "NO_INTERNAL_LOOKUP"; break; > + default: > + session_cache_mode = "Unknown"; break; > + } > + } > +}; > + > +static SSL_ACCEPTOR_STATS ssl_acceptor_stats; > +void ssl_acceptor_stats_update(int sslaccept_ret) > +{ > + ssl_acceptor_stats.accept++; > + if (!sslaccept_ret) > + ssl_acceptor_stats.accept_good++; use statistic_increment here > +} > + > static void init_ssl() > { > #if defined(HAVE_OPENSSL) && !defined(EMBEDDED_LIBRARY) > @@ -4804,6 +4862,38 @@ static void init_ssl() > #endif /* HAVE_OPENSSL && ! EMBEDDED_LIBRARY */ > } > > +/* Reinitialize SSL (FLUSH SSL) */ > +int reinit_ssl() > +{ > +#if defined(HAVE_OPENSSL) && !defined(EMBEDDED_LIBRARY) > + if (!opt_use_ssl) > + return 0; > +#ifndef DBUG_OFF > + char *old_ssl_cert; it's generally prefered to use __attribute__((unused)) instead of #ifdef > +#endif > + > + DBUG_EXECUTE_IF("simulate_bad_ssl_cert", old_ssl_cert= opt_ssl_cert; opt_ssl_cert=const_cast<char *>("");); > + > + enum enum_ssl_init_error error = SSL_INITERR_NOERROR; > + st_VioSSLFd *new_fd = new_VioSSLAcceptorFd(opt_ssl_key, opt_ssl_cert, > + opt_ssl_ca, opt_ssl_capath, opt_ssl_cipher, &error, opt_ssl_crl, opt_ssl_crlpath); > + > + DBUG_EXECUTE_IF("simulate_bad_ssl_cert", opt_ssl_cert= old_ssl_cert;); > + > + if (!new_fd) > + { > + my_printf_error(ER_UNKNOWN_ERROR, "Failed to refresh SSL, error: %s", MYF(0), > + sslGetErrString(error)); > + return 1; > + } > + mysql_rwlock_wrlock(&LOCK_ssl_refresh); > + free_vio_ssl_acceptor_fd(ssl_acceptor_fd); > + ssl_acceptor_fd= new_fd; > + ssl_acceptor_stats.init(); > + mysql_rwlock_unlock(&LOCK_ssl_refresh); > + return 0; > +#endif > +} > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MDEV-16188: Rowid filtering: EXPLAIN FORMAT=JSON support
by Sergey Petrunia 05 Dec '18

05 Dec '18

Hello Igor, I have pushed into 10.4-mdev16188 tree a patch that adds EXPLAIN FORMAT=JSON support. There is no ANALYZE support, yet. main.rowid_filter has an example of EXPLAIN FORMAT=JSON at the bottom. The filter is described like so: + "rowid_filter": { + "range": { + "key": "i_l_shipdate", + "used_key_parts": ["l_shipDATE"] + }, + "rows": 8, + "selectivity_pct": 0.1332 + }, Here "rows" is how many rows we will get from the quick select that is used to build the filter. selectivity_pct is the selectivity of the filter. You will also notice this change from (1%) to (0%): -1 SIMPLE lineitem range|filter PRIMARY,i_l_shipdate,i_l_receiptdate,i_l_orderkey,i_l_orderkey_quantity i_l_receiptdate|i_l_shipdate 4|4 NULL 6 (1%) Using index condition; Using where; Using filter +1 SIMPLE lineitem range|filter PRIMARY,i_l_shipdate,i_l_receiptdate,i_l_orderkey,i_l_orderkey_quantity i_l_receiptdate|i_l_shipdate 4|4 NULL 6 (0%) Using index condition; Using where; Using filter This is because the original code had a piece of logic in JOIN_TAB::save_filter_explain_data: print "1%" if it was "0%". I'm wondering, is there any reason we cannot print the number with greater precision in this case? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] 2397c00086c: MDEV-15649 Speedup search in acl_users and acl_dbs array,
by Sergei Golubchik 03 Dec '18

03 Dec '18

Hi, Vladislav! Okay. Frankly, that's not how I'd done it, but it'll work. Only a few minor comments below, then ok to push. On Dec 03, Vladislav Vaintroub wrote: > revision-id: 2397c00086c4d9df5e2bbad70803e293f5a47984 (mariadb-10.4.0-37-g2397c00086c) > parent(s): 46a411088c5abbacda4e52e89f4dbcf52a451f7a > author: Vladislav Vaintroub <wlad(a)mariadb.com> > committer: Vladislav Vaintroub <wlad(a)mariadb.com> > timestamp: 2018-11-30 18:14:00 +0100 > message: > > MDEV-15649 Speedup search in acl_users and acl_dbs array, > sorting them by usernames first, and then by get_sort() value. > > Search functions now use binary search to find the the first entry with > given name. Then, linear search is done, until the first match. > > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index 052c5ada3a2..932c4189e3f 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -2326,6 +2328,156 @@ static int acl_compare(ACL_ACCESS *a,ACL_ACCESS *b) > + > + > +static inline const char *get_username(const ACL_DB& acl_db) > +{ > + return acl_db.user; > +} > + > + > +static inline const char *get_username(const ACL_USER& acl_user) > +{ > + return acl_user.user.str; > +} I'd do them as methods in ACL_DB and ACL_USER > + > +/* > + Return index of the first entry with given user in the array, > + or UINT_MAX if not found. > + > + Assumes the array is sorted by get_username > +*/ > +template<typename T> uint find_first_user(T* arr, uint len, const char *user) > +{ > + uint low= 0; > + uint high= len - 1; > + uint mid; > + if(!len) > + return UINT_MAX; > + > +#ifndef DBUG_OFF > + for (uint i = 0; i < len - 1; i++) > + DBUG_ASSERT(strcmp(get_username(arr[i]), get_username(arr[i + 1])) <= 0); > +#endif > + while (low <= high) > + { > + mid = low + (high - low) / 2; > + int cmp = strcmp(get_username(arr[mid]), user); > + if (cmp == 0) > + { > + /* > + We are looking for the left-most element in the array with the given value. > + Thus, even if value is found, we still have to check the left neighbor, > + and if it has the same value, continue the search. > + */ > + if (mid > 0 && !strcmp(get_username(arr[mid - 1]), user)) > + cmp = 1; hmm, I thought, normally it's done not by checking the left neighbor, but by `if (cmp <=0 )` and continuing the search until low>high > + } > + > + if (cmp > 0) > + { > + if (mid == 0) > + break; > + high= mid - 1; > + } > + else if (cmp < 0) > + low= mid + 1; > + else > + return mid; > + } > + return UINT_MAX; > +} > + > +static ACL_DB *acl_db_find(const char *db, const char *user, const char *host, const char *ip, my_bool db_is_pattern) > +{ > + uint i; > + ACL_DB *ret = NULL; > + > + // Check databases with matching user name. > + uint start= acl_find_db_by_username(user); > + for (i= start; i < acl_dbs.elements; i++) > + { > + ACL_DB *acl_db= dynamic_element(&acl_dbs, i, ACL_DB*); > + if (i > start && strcmp(user, acl_db->user)) > + break; > + > + if(acl_db_matches(acl_db, db, host, ip,db_is_pattern)) > + { > + ret= acl_db; > + break; > + } > + } > + > + // Look also for anonymous user (at the start of array due to sort by name). > + for (i = 0; i < acl_dbs.elements; i++) > + { > + ACL_DB *acl_db= dynamic_element(&acl_dbs, i, ACL_DB*); > + if (*acl_db->user || (ret && acl_compare(acl_db, ret) >= 0)) > + break; > + if (acl_db_matches(acl_db, db, host, ip, db_is_pattern)) > + { > + ret= acl_db; > + break; > + } > + } > + return ret; > +} that's the same logic as in find_user_or_anon(). May be you could template that out too? Another thought - this is a pretty weird-looking search logic. Please add a comment that its goal is to emulate historical behavior, where entries were found by a linear search in an array sorted by the get_sort() value. > @@ -3437,10 +3579,14 @@ static ACL_USER * find_user_wild(const char *host, const char *user, const char > { > mysql_mutex_assert_owner(&acl_cache->lock); > > - for (uint i=0 ; i < acl_users.elements ; i++) > + uint start = acl_find_user_by_name(user); > + > + for (uint i= start; i < acl_users.elements; i++) > { > ACL_USER *acl_user=dynamic_element(&acl_users,i,ACL_USER*); > - if (acl_user->wild_eq(user, host, ip)) This makes wild_eq method kind of pointless. Remove it? > + if (i > start && strcmp(acl_user->user.str, user)) > + break; > + if (compare_hostname(&acl_user->host, host, ip ? ip : host)) > return acl_user; > } > return 0; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0