- developers - lists.mariadb.org

[Maria-developers] Review: bb-10.8-wlad-MDEV-26713
by Alexander Barkov 10 Dec '21

10 Dec '21

Hi Wlad, You've done a great piece of work. Thanks and congrats. I generally like all the changes. Have a few comments only, please see below. commit 1411d5f40ceaf2eecabbcf68588743439003f4a2 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 11:42:46 2021 +0100 Windows : incorrect handling of non-ASCIIs in get_tty_password Why no MDEV? + wchar_t wbuf[80]; + char to[80*3]; /* there is at most 3 bytes per wchar, in any codepage */ + wchar_t *pos=wbuf,*end=wbuf + array_elements(wbuf)-1; Isn't it "at most 4 bytes per char"? ############################################################# commit 7af691ca11c95a385aff4889a56870d70843e78d Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 12:03:48 2021 +0100 MDEV-27090: Windows client - ReadConsoleA does not work correctly with UTF8 codepage Corresponding Windows bug https://github.com/microsoft/terminal/issues/4551 Use ReadConsoleW instead and convert to console's input codepage, to workaround. Also, disable VT sequences in the console output, as we do not knows what type of data comes with SELECT, we do not want VT escapes there. Remove my_cgets() + while (nchars > 0) + { + if (wstrbuf[nchars - 1] != '\n' && wstrbuf[nchars - 1] != '\r') + break; + wstrbuf[--nchars]= 0; + } + + wstrbuf[nchars]= 0; The inner assignment wstrbuf[--nchars]=0 inside every iteration is not needed. I'd write it this way: for ( ; nchars > 0 ; nchars--) { if (wstrbuf[nchars - 1] != '\n' && wstrbuf[nchars - 1] != '\r') break; } wstrbuf[nchars]= 0; But the outer assignment to 0 does not seem to be needed either. Why do it? my_convert() does not need the trailing zero. It uses only sizes passed. So this should be enough: + strbuf[len]= 0; - pthread_auto_mutex_decl(my_conio_cs); - - /* lock the console for the current process*/ - if (pthread_auto_mutex_lock(my_conio_cs, GetCurrentProcessId(), INFINITE)) - { There was a mutex lock inside my_cgets(). Why was it needed before the patch and why is it not needed any more? ##################################################### commit 5c09e9ce4f922fe9674f3f85329742c0c5498a28 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 14:03:51 2021 +0100 MDEV-27092 Windows - services that have non-ASCII characters do not work with activeCodePage=UTF8 CreateServiceA, OpenServiceA, and couple of other functions do not work correctly with non-ASCII character, in the special case where application has defined activeCodePage=UTF8. ... - Copyright (c) 2011, 2012, Monty Program Ab + Copyright (c) 2011, 2021 Monty Program Ab MariaDB Corporation? This code: + AWSTRDUP(w_ServiceName,lpServiceName); + AWSTRDUP(w_DisplayName,lpDisplayName); + AWSTRDUP(w_BinaryPathName, lpBinaryPathName); + AWSTRDUP(w_LoadOrderGroup, lpLoadOrderGroup); + AWSTRDUP(w_Dependencies, lpDependencies); + AWSTRDUP(w_ServiceStartName, lpServiceStartName); + AWSTRDUP(w_Password, lpPassword); + + free(w_ServiceName); + free(w_DisplayName); + free(w_BinaryPathName); + free(w_LoadOrderGroup); + free(w_Dependencies); + free(w_ServiceStartName); + free(w_Password); duplicates two times. Consider wrapping these variables into a class and reuse the class in both cases. ########################################################## commit d4056a28ec5ae0c9a0c3144a8e90357dc574723f Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 19 14:14:38 2021 +0100 MDEV-27093 Do not pass root password in clear text from mariadb-install-db.exe to bootstrap Pass the hash instead, so that bootstrap would not misinterpret it for string with different charset. The password was passed using hex encoding. But the comment says it was clear text. Wrong comment? I did not understand the change. Reading the MDEV did not make it clearer. What was wrong? How does the patch fix the problem? Can we discuss on slack? ######################################################## commit fa29a64de918d2045df3cce1d4a546de5597c0bd Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 22 12:29:15 2021 +0100 MDEV-26713 set activeCodePage=UTF8 for windows programs - Use corresponding entry in the manifest, as described in https://docs.microsoft.com/en-us/windows/apps/design/globalizing/use-utf8-c… - Set console codepage(s) to UTF8, if ANSI codepage is UTF8 This effectively causes UTF8 to be used as default client encoding, for Windows 1903 and later - Allow UTF8 on the command line for MTR, for the affected Windows versions Looks OK for me. ######################################################## commit b88d54132369b938ba041c0d3f97a5a212210b37 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 22 12:34:10 2021 +0100 MDEV-26713 : Treat codepage 65001 as utf8mb4, not just utf8 Also, fix the "UTF8" option in MSI, which is responsible for character-set-server setting Looks OK for me. ######################################################## commit b9c38470ed6e04185f7f84a056034b7254336d51 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 22 13:22:05 2021 +0100 MDEV-26713 utf8 support on Windows, mysql_upgrade_service part Handle upgrade - on Windows that is capable of UTF8 codepage, convert entries in my.ini from ANSI to UTF8, during upgrade Reason is that paths must now be utf8. For the client , user name, database etc should be in UTF8, too, as this is now the default charset. Should not we add an UTF8 BOM to the beginning of the file, so text editors do not break it later. The patch looks too large for me. It seems it does more than the comments above tell. Looks like convenience refactoring, but it's hard to follow. It would be easier to understand changes if you split it into two parts: 1. the refactoring that adds functions like upgrade_config_file(), fix_section(), is_mariadb_section(), fix_and_check_datadir(), I guess this refactoring is a non-functional change that should not change the behaviour, right? 2. the actual conversion part on top of it ######################################################## commit 0700393b570c6b9a7b4347d3381a7cf05d1499ba Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Tue Nov 23 12:28:28 2021 +0100 mysqltest, Windows - support background execution in 'exec' command If last character in command is ampersand, do not use my_popen/my_pclose in "exec", instead do CreateProcess() without a window. I suggest you add an MDEV for this change and explain in there why we did not need this feature earlier and why we need it now and how it relates to MDEV-26713. ######################################################## commit 7d50d83b3bcab5a3be761f98722e33ed0b01ab23 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Tue Nov 23 13:05:25 2021 +0100 MDEV-26713 utf8 support on Windows , add mysql_install_db tests Add mysql_install_db test with some i18n, for data dir and root password Looks OK for me. ######################################################## commit b2cc09fe76f526bd4cd76131a52263b13b3828a5 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Wed Nov 24 10:15:11 2021 +0100 MDEV-26713 Add test for mysql_install_db creating service, with i18 Looks OK for me. ######################################################## commit 51ce069376fec45d340563a4f7630f8a155afd64 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Fri Nov 26 18:41:35 2021 +0100 MDEV-26713 Introduce --password-codepage client setting I'm not sure we need this new options. Looks like over-engineering a bit. Is the scenario it covers really deserves documenting, learning, etc? I'd better disallow non-ASCII passwords in the future, and right now just add a warning: You're using a non-ASCII password, which is not portable. If you have problems connecting to the database, please ask the administrator to change the password. ######################################################## commit b155c4036cfb3ffdaa7c272048ac9d32e3182d78 Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Mon Nov 29 19:47:36 2021 +0100 MDEV-26713 set console codepage to what user set in --default-character-set I prefer to keep the comment that you removed: - if (!fs_cset_cache) - { - char buf[10]= "cp"; - GetLocaleInfo(LOCALE_SYSTEM_DEFAULT, LOCALE_IDEFAULTANSICODEPAGE, - buf+2, sizeof(buf)-3); - /* - We cannot call get_charset_by_name here - because fs_character_set() is executed before - LOCK_THD_charset mutex initialization, which - is used inside get_charset_by_name. - As we're now interested in cp932 only, - let's just detect it using strcmp(). - */ - fs_cset_cache= - #ifdef HAVE_CHARSET_cp932 - !strcmp(buf, "cp932") ? &my_charset_cp932_japanese_ci : - #endif - &my_charset_bin; This function uses both default_charset and charset_info->cs_name.str: +static void adjust_console_codepage() +{ +#ifdef _WIN32 + const char *name= charset_info->cs_name.str; + if (my_set_console_cp(default_charset) < 0) And it seems the former is set to the latter just before the function call: default_charset= (char *)charset_info->cs_name.str; <------ HERE put_info("Charset changed", INFO_INFO); adjust_console_codepage(); I find this hard to follow. Can you please add a parameter to adjust_console_codepage() and pass either charset_info->cs_name.str (or default_charset) when calling? Also, I think this function should only be called when default_charset was some explicit name, not MYSQL_AUTODETECT_CHARSET_NAME. I think the logic should be: 1. If --default-character-set=auto, then detect it from the console. 2. If --default-character-set=csname, then set the console CP to csname. Now in case of N1 it seems to detect the character set from the console, then sets the console to what has been detected. Is the last step really needed in case of auto-detection? ######################################################## commit b38c341cc392630c798eb754f87338d37ec95cdf Author: Vladislav Vaintroub <wlad(a)mariadb.com> Date: Sun Dec 5 17:08:18 2021 +0100 Fix race condition in test. Do not change console codepage in a possibly shared console. Start new console for chcp. Can you extend the comment and tell what actually happened before this change? Greetings.

1 0

Re: [Maria-developers] b95c5105e28: MDEV-17554 Auto-create new partition for system versioned tables with history partitioned by INTERVAL/LIMIT
by Sergei Golubchik 07 Dec '21

07 Dec '21

Hi, Aleksey! This is a review of diff b95c5105e2 2552bce607 This was quite good, thanks! Just a couple of questions/comments, see below. On Nov 23, Aleksey Midenkov wrote: > diff --git a/mysql-test/suite/versioning/t/partition.test b/mysql-test/suite/versioning/t/partition.test > index 006a65e1a16..a5bb8426ae4 100644 > --- a/mysql-test/suite/versioning/t/partition.test > +++ b/mysql-test/suite/versioning/t/partition.test > @@ -1070,15 +1080,572 @@ alter table t1 add partition partitions 2; ... > +--let $datadir= `select @@datadir` > +--let $dummy= $datadir/test/t1#P#p1.ibd > +--write_file $dummy > +EOF > + > +set timestamp= unix_timestamp('2000-01-01 01:00:00'); > +--error ER_GET_ERRNO > +update t1 set x= x + 2; That's a good one! > +show warnings; > +--remove_file $dummy > + > diff --git a/sql/table.h b/sql/table.h > index 2e074abcea0..d861db261bf 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -910,6 +911,11 @@ struct TABLE_SHARE > vers_kind_t versioned; > period_info_t vers; > period_info_t period; > + /* > + Protect multiple threads from repeating partition auto-create over > + single share. could you please add here (to the comment) TODO remove it when partitioning metadata will be in TABLE_SHARE > + */ > + bool vers_skip_auto_create; > > bool init_period_from_extra2(period_info_t *period, const uchar *data, > const uchar *end); > @@ -2557,6 +2567,11 @@ struct TABLE_LIST > bool merged; > bool merged_for_insert; > bool sequence; /* Part of NEXTVAL/CURVAL/LASTVAL */ > + /* > + Protect single thread from repeating partition auto-create over > + multiple share instances (as the share is closed on backoff action). > + */ > + bool vers_skip_create; I don't understand it. What does it mean "multiple share instances"? Can there be multiple TABLE_SHARE objects for the same table? How? > > /* > Items created by create_view_field and collected to change them in case > diff --git a/sql/sql_partition.cc b/sql/sql_partition.cc > index d7bb02bbd4e..56d5a68efd0 100644 > --- a/sql/sql_partition.cc > +++ b/sql/sql_partition.cc > @@ -2587,11 +2587,15 @@ char *generate_partition_syntax(THD *thd, partition_info *part_info, > err+= str.append(ctime, ctime_len); > err+= str.append('\''); > } > + if (vers_info->auto_hist) > + err+= str.append(STRING_WITH_LEN(" AUTO")); > } > - if (vers_info->limit) > + else if (vers_info->limit) > { > err+= str.append(STRING_WITH_LEN("LIMIT ")); > err+= str.append_ulonglong(vers_info->limit); > + if (vers_info->auto_hist) > + err+= str.append(STRING_WITH_LEN(" AUTO")); Can you do this str.appent("AUTO") once, after the if()? Not in both branches? > } > } > else if (part_info->part_expr) > diff --git a/sql/partition_info.cc b/sql/partition_info.cc > index fd92e437cac..ddcc6f413ba 100644 > --- a/sql/partition_info.cc > +++ b/sql/partition_info.cc > @@ -865,9 +870,167 @@ void partition_info::vers_set_hist_part(THD *thd) > { > vers_info->hist_part= next; > if (next->range_value > thd->query_start()) > - return; > + { > + error= false; > + break; > + } > + } > + if (error) > + { > + if (auto_hist) > + { > + *create_count= 0; > + const my_time_t hist_end= (my_time_t) vers_info->hist_part->range_value; > + DBUG_ASSERT(thd->query_start() >= hist_end); > + MYSQL_TIME h0, q0; > + my_tz_OFFSET0->gmt_sec_to_TIME(&h0, hist_end); > + my_tz_OFFSET0->gmt_sec_to_TIME(&q0, thd->query_start()); > + longlong q= pack_time(&q0); > + longlong h= pack_time(&h0); > + while (h <= q) > + { > + if (date_add_interval(thd, &h0, vers_info->interval.type, > + vers_info->interval.step)) > + return true; > + h= pack_time(&h0); > + ++*create_count; > + if (*create_count == MAX_PARTITIONS - 2) > + { > + my_error(ER_TOO_MANY_PARTITIONS_ERROR, MYF(ME_WARNING)); > + my_error(ER_VERS_HIST_PART_FAILED, MYF(0), > + table->s->db.str, table->s->table_name.str); > + return true; > + } > + } > + } > + else > + { > + DBUG_ASSERT(!vers_info->auto_hist); ^^^ This assert fails for me in the update-big test > + my_error(WARN_VERS_PART_FULL, MYF(ME_WARNING|ME_ERROR_LOG), > + table->s->db.str, table->s->table_name.str, > + vers_info->hist_part->partition_name, "INTERVAL"); > + } > + } > + } > + > + return false; > +} > + > + > diff --git a/sql/sql_base.cc b/sql/sql_base.cc > index ac22a63bdba..459edb614f7 100644 > --- a/sql/sql_base.cc > +++ b/sql/sql_base.cc > @@ -4203,6 +4444,15 @@ bool open_tables(THD *thd, const DDL_options_st &options, > } > > thd->current_tablenr= 0; > + > +#ifdef WITH_PARTITION_STORAGE_ENGINE > + if (!thd->stmt_arena->is_conventional()) > + { > + for (tables= *start; tables; tables= tables->next_global) > + tables->vers_skip_create= false; > + } This looks a bit like an overkill, you do it for every prepare, while auto-adding a partition is a rare event, as you said. May be you can do instead in TABLE::vers_switch_partition uint *create_count= !thd->stmt_arena->is_conventional() && table_list->vers_skip_create ? > +#endif > + > restart: > /* > Close HANDLER tables which are marked for flush or against which there Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

[Maria-developers] some questions for the community
by 杨凡 26 Nov '21

26 Nov '21

Hello, I am a student of Xi'an University of Posts and Telecommunications. My name is yangfan.I want to ask something about community code sharing and GSOC. I am now in my sophomore year, and I am very interested in databases, especially your marria database. I am now familiar with the c/c++ language and master the basic Linux system programming and network programming. Currently learning C++ multi-threaded programming. I can use basic SQL statements, databases, basic Linux commands, and some knowledge about operating systems and networks. I joined the xiyou linux group. Now, I want to learn quickly and contribute code to your community. Can you tell me what to do? And I want to participate in GSOC in 2022. If you can give me some help, I will be very happy. I only learned about open source culture from university. After a long period of study and thinking, I feel that open source is a very mea ningful thing. It not only helps personal growth but also helps a product iteration. The slogan of our group is free sharing and openness. At the same time, I continue to promote open source culture. I am fascinated by this culture. I also want to join a community and make my contribution to you. Although my current ability is not very strong, but I will continue to work hard. I am very interested in programming, and I want to focus on distributed and database work now and in the future. Maybe my English is not very good, I hope you can be more tolerant.

1 0

Re: [Maria-developers] a5fca9a6e30: MENT-651 [6/8] store cache managers in a list
by Sergei Golubchik 26 Nov '21

26 Nov '21

Hi, Nikita! On Nov 14, Nikita Malyavin wrote: > revision-id: a5fca9a6e30 (mariadb-10.5.2-478-ga5fca9a6e30) > parent(s): ad6e7b87107 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-27 17:28:05 +1000 > message: > > MENT-651 [6/8] store cache managers in a list again, a couple of lines with a description would be good here > diff --git a/mysql-test/suite/binlog/t/online_alter.test b/mysql-test/suite/binlog/t/online_alter.test > index 804e847d008..4ed2db74bd6 100644 > --- a/mysql-test/suite/binlog/t/online_alter.test > +++ b/mysql-test/suite/binlog/t/online_alter.test > @@ -218,7 +219,7 @@ set autocommit = 0; > start transaction; > update t1 set b= 7007 where a = 7; > --error ER_DUP_ENTRY > -update t1 set a= 8 where a = 8 or a = 9; > +update t1 set a= 8, b= 8008 where a = 8 or a = 9 order by a; why did you change tests for what is, as far as I understand, just an optimization? > commit; > set autocommit = 1; > > diff --git a/sql/handler.cc b/sql/handler.cc > index 8b95653e69f..8b22167e729 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -6660,7 +6660,16 @@ static int binlog_log_row_online_alter(TABLE* table, > THD *thd= table->in_use; > > if (!table->online_alter_cache) > - table->online_alter_cache= thd->binlog_setup_cache_data(); > + { > + auto *cache_mngr= online_alter_binlog_get_cache_mngr(thd, table); > + // Use transaction cache directly, if it is not multi-transaction mode > + table->online_alter_cache= binlog_get_cache_data(cache_mngr, > + !thd->in_multi_stmt_transaction_mode()); > + > + trans_register_ha(thd, false, binlog_hton, 0); > + if (thd->in_multi_stmt_transaction_mode()) > + trans_register_ha(thd, true, binlog_hton, 0); I still don't understand that. Why do you need a handlerton, why do you fake a transaction? And why do you need to search a list in THD, instead of storing cache_mngr in TABLE_SHARE? > + } > > // We need to log all columns for the case if alter table changes primary key. > table->use_all_columns(); > diff --git a/sql/log.cc b/sql/log.cc > index 6c678624230..c58096e6d9a 100644 > --- a/sql/log.cc > +++ b/sql/log.cc > @@ -2258,14 +2298,18 @@ static int binlog_rollback(handlerton *hton, THD *thd, bool all) > { > DBUG_ENTER("binlog_rollback"); > > - for (TABLE *table= thd->open_tables; table; table= table->next) > - { > - if (!table->online_alter_cache) > - continue; > - table->online_alter_cache->reset(); > - delete table->online_alter_cache; > - table->online_alter_cache= NULL; > - } > + bool is_ending_trans= ending_trans(thd, all); > + > + /* > + This is a crucial moment that we are running through > + thd->online_alter_cache_list, and not through thd->open_tables to cleanup > + stmt cache, though both have it. The reason is that the tables can be closed > + to that moment in case of an error. > + The same reason applies to the fact we don't store cache_mngr in the table > + itself -- because it can happen to be not existing. > + Still in case if tables are left opened > + */ > + binlog_online_alter_cleanup(thd->online_alter_cache_list, is_ending_trans); still don't understand that comment either :( Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

[Maria-developers] Squash aaed3436 MENT-651 [4/8] ALTER ONLINE TABLE
by Nikita Malyavin 25 Nov '21

25 Nov '21

Sergei, I think all the commits starting from [4/8] ALTER ONLINE TABLE should be squashed. I will also add a better description in the commit message covering our latest discussions. I would also embed a reference to the original unsquashed branch. It seems to me as a good idea in sense of helping the reader investigate the code -- Yours truly, Nikita Malyavin

2 5

Re: [Maria-developers] 4af7a583802: Refactor MYSQL_BIN_LOG: extract Event_log ancestor
by Sergei Golubchik 24 Nov '21

24 Nov '21

Hi, Nikita! On Nov 12, Nikita Malyavin wrote: > revision-id: 4af7a583802 (mariadb-10.5.2-474-g4af7a583802) > parent(s): 2da3dc5d422 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-08 20:59:09 +1000 > message: > > Refactor MYSQL_BIN_LOG: extract Event_log ancestor Would be good to have some explanation here. I understand that you'll use this refactoring in following commits, but conceptually, what Event_log is? I mean, like Event_log is a log where Log_event_xxx events are written, but not _something else that MYSQL_BIN_LOG is and Event_log isn't_ or Event_log is a log where Log_event_xxx events are written. While MYSQL_BIN_LOG is Event_log that also does .... > diff --git a/sql/log.cc b/sql/log.cc > index 827a34c2883..6f4e3191588 100644 > --- a/sql/log.cc > +++ b/sql/log.cc > @@ -3701,64 +3765,25 @@ bool MYSQL_BIN_LOG::open(const char *log_name, > write_file_name_to_index_file= 1; > } a bit strange that MYSQL_BIN_LOG::open doesn't use Event_log::open > diff --git a/sql/log.h b/sql/log.h > index 73cd66d5a4a..74c409e1ac7 100644 > --- a/sql/log.h > +++ b/sql/log.h > @@ -327,6 +327,7 @@ class MYSQL_LOG > bool strip_ext, char *buff); > virtual int generate_new_name(char *new_name, const char *log_name, > ulong next_log_number); > + inline mysql_mutex_t* get_log_lock() { return &LOCK_log; } > protected: > /* LOCK_log is inited by init_pthread_objects() */ > mysql_mutex_t LOCK_log; > @@ -376,6 +377,64 @@ struct Rows_event_factory > } > }; > > +class Event_log: public MYSQL_LOG > +{ > +public: > +#if !defined(MYSQL_CLIENT) > + Rows_log_event* > + prepare_pending_rows_event(THD *thd, TABLE* table, > + binlog_cache_data *cache_data, > + uint32 serv_id, size_t needed, > + bool is_transactional, > + Rows_event_factory event_factory); forgot to delete the same from MYSQL_BIN_LOG > +#endif > + > + bool flush_and_sync(bool *synced); same > + void cleanup() > + { > + if (inited) > + mysql_mutex_destroy(&LOCK_binlog_end_pos); > + > + MYSQL_LOG::cleanup(); > + } > + void init_pthread_objects() > + { > + MYSQL_LOG::init_pthread_objects(); > + > + mysql_mutex_init(m_key_LOCK_binlog_end_pos, &LOCK_binlog_end_pos, > + MY_MUTEX_INIT_SLOW); > + } > + > + bool open( > + const char *log_name, > + enum_log_type log_type, > + const char *new_name, ulong next_file_number, > + enum cache_type io_cache_type_arg); > + IO_CACHE *get_log_file() { return &log_file; } > + > + int write_description_event(enum_binlog_checksum_alg checksum_alg, > + bool encrypt, bool dont_set_created); > + > + bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file); > +}; > + > /* Tell the io thread if we can delay the master info sync. */ > #define SEMI_SYNC_SLAVE_DELAY_SYNC 1 > /* Tell the io thread if the current event needs a ack. */ > @@ -451,7 +510,7 @@ class binlog_cache_data; > struct rpl_gtid; > struct wait_for_commit; > > -class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG > +class MYSQL_BIN_LOG: public TC_LOG, private Event_log > { > /** The instrumentation key to use for @ LOCK_index. */ > PSI_mutex_key m_key_LOCK_index; > @@ -849,15 +901,13 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG > > - bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file); > + bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file) > + { return Event_log::write_event(ev, data, file); } Perhaps using Event_log::write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file); ? > bool write_event(Log_event *ev) { return write_event(ev, 0, &log_file); } > > bool write_event_buffer(uchar* buf,uint len); > @@ -918,7 +968,6 @@ class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG > uint next_file_id(); > inline char* get_index_fname() { return index_file_name;} > inline char* get_log_fname() { return log_file_name; } > - inline char* get_name() { return name; } > inline mysql_mutex_t* get_log_lock() { return &LOCK_log; } should remove that too, I reckon > inline mysql_cond_t* get_bin_log_cond() { return &COND_bin_log_updated; } > inline IO_CACHE* get_log_file() { return &log_file; } Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] f3603cbfaeb: report progress
by Sergei Golubchik 24 Nov '21

24 Nov '21

Hi, Nikita! On Nov 15, Nikita Malyavin wrote: > revision-id: f3603cbfaeb (mariadb-10.5.2-483-gf3603cbfaeb) > parent(s): 3b6038bee88 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-29 03:47:52 +1000 > message: > > report progress Perhaps, squash this with your commit "test progress reporting" ? > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 29d0f487303..87dbe226c2e 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -11300,6 +11300,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, > > IO_CACHE *log_file= log->flip(); > > + thd_progress_report(thd, 0, my_b_write_tell(log_file) > + / rgi->tables_to_lock->m_conv_table->s->reclength); 1. why do you do this "/ rgi->...->s->reclength" ? 2. How does it work with your "cache flipping"? While data is copied between tables, concurrent connections write into an IO_CACHE. Then you flip, they what, write into the second IO_CACHE? What file length will you use here in thd_progress_report()? > + > do > { > const auto *descr_event= rgi->rli->relay_log.description_event_for_exec; > @@ -11319,6 +11322,9 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, > free_root(&event_mem_root, MYF(MY_KEEP_PREALLOC)); > if (ev != rgi->rli->relay_log.description_event_for_exec) > delete ev; > + thd_progress_report(thd, my_b_tell(log_file) > + / rgi->tables_to_lock->m_conv_table->s->reclength, > + thd->progress.max_counter); > } > while(!error); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] 9277b838aad: Forbid savepoints setup while ONLINE ALTER goes on
by Sergei Golubchik 24 Nov '21

24 Nov '21

Hi, Nikita! On Nov 15, Nikita Malyavin wrote: > revision-id: 9277b838aad (mariadb-10.5.2-485-g9277b838aad) > parent(s): 37b6f5cb8ef > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-29 03:47:52 +1000 > message: > > Forbid savepoints setup while ONLINE ALTER goes on Why is that? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] 903ae1c03ae: Fix running without binlog
by Sergei Golubchik 24 Nov '21

24 Nov '21

Hi, Nikita! On Nov 15, Nikita Malyavin wrote: > revision-id: 903ae1c03ae (mariadb-10.5.2-479-g903ae1c03ae) > parent(s): a5fca9a6e30 > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-27 17:28:05 +1000 > message: > > Fix running without binlog Strange. With such a comment I'd expect you add a test for ALTER TABLE with disabled binlog. Instead you change an error to a warning in one of the existing tests. This test change doesn't match the commit comment. > diff --git a/mysql-test/suite/binlog/r/online_alter.result b/mysql-test/suite/binlog/r/online_alter.result > index 9bca94b8a87..bb3fa85e76c 100644 > --- a/mysql-test/suite/binlog/r/online_alter.result > +++ b/mysql-test/suite/binlog/r/online_alter.result > @@ -34,9 +34,9 @@ insert into t1 values (123), (456), (789); > set debug_sync= 'now SIGNAL end'; > connection default; > Warnings: > -Error 1364 Field 'b' doesn't have a default value > -Error 1364 Field 'b' doesn't have a default value > -Error 1364 Field 'b' doesn't have a default value > +Warning 1364 Field 'b' doesn't have a default value > +Warning 1364 Field 'b' doesn't have a default value > +Warning 1364 Field 'b' doesn't have a default value > select * from t1; > a b > 5 0 > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index f0d1167ad61..e348056646d 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -11308,9 +11308,12 @@ static int online_alter_read_from_binlog(THD *thd, rpl_group_info *rgi, > break; > > ev->thd= thd; > + bool abort_on_warning= thd->abort_on_warning; > + thd->abort_on_warning= false; > thd->set_n_backup_active_arena(&event_arena, &backup_arena); > error= ev->apply_event(rgi); > thd->restore_active_arena(&event_arena, &backup_arena); > + thd->abort_on_warning= abort_on_warning; > > event_arena.free_items(); > free_root(&event_mem_root, MYF(MY_KEEP_PREALLOC)); > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

[Maria-developers] Review for: MDEV-27036: Add asserts to detect duplicate JSON keys
by Sergey Petrunia 24 Nov '21

24 Nov '21

Hi Sergei, Please find my input below. > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 0dfe95e81b0..fa07608177a 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -7910,28 +7910,27 @@ best_access_path(JOIN *join, > can make an adjustment is a special case of the criteria used > in ReuseRangeEstimateForRef-3. > */ > - if (table->opt_range_keys.is_set(key) && > + auto use_range_estimates= table->opt_range_keys.is_set(key) && > (const_part & What is the reason for this change? Please restore the old code. > (((key_part_map)1 << table->opt_range[key].key_parts)-1)) == > (((key_part_map)1 << table->opt_range[key].key_parts)-1) && > table->opt_range[key].ranges == 1 && > - records > (double) table->opt_range[key].rows) > + records > (double) table->opt_range[key].rows; > + if (use_range_estimates) > { > records= (double) table->opt_range[key].rows; > trace_access_idx.add("used_range_estimates", "clipped down"); > } > else > { > + trace_access_idx.add("used_range_estimates",false); > if (table->opt_range_keys.is_set(key)) > { > - trace_access_idx.add("used_range_estimates",false) > - .add("cause", > - "not better than ref estimates"); > + cause= "not better than ref estimates"; As we have already discussed: Do not assign this to "cause". To avoid duplicate names, change the name above from "cause" to e.g. "reason". > } > else > { > - trace_access_idx.add("used_range_estimates", false) > - .add("cause", "not available"); > + cause= "not available"; > } The same as above. > } > } > @@ -8217,9 +8215,10 @@ best_access_path(JOIN *join, > best_uses_jbuf= TRUE; > best_filter= 0; > best_type= JT_HASH; > + Json_writer_object trace_access_hash(thd); > trace_access_hash.add("type", "hash"); > trace_access_hash.add("index", "hj-key"); > - trace_access_hash.add("cost", rnd_records); > + trace_access_hash.add("est_records_cost", rnd_records); Please use "rnd_records" instead of "est_records_cost". > trace_access_hash.add("cost", best); > trace_access_hash.add("chosen", true); > } > commit 0e8b82dbcbd8ae96e0758114c87e48061d603c5f > Author: Sergei Krivonos <sergei.krivonos(a)mariadb.com> > Date: Fri Nov 12 03:36:10 2021 +0200 > > MDEV-27036: add assert to detect duplicated JSON keys > > diff --git a/sql/my_json_writer.cc b/sql/my_json_writer.cc > index 9470ba57855..501f0b72318 100644 > --- a/sql/my_json_writer.cc > +++ b/sql/my_json_writer.cc > @@ -39,6 +41,9 @@ inline void Json_writer::on_start_object() > #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) > if(!fmt_helper.is_making_writer_calls()) > { > + if(got_name != named_item_expected()){ > + std::cout <<"got_name["<<got_name<<"] != named_item_expected["<<named_item_expected()<<']'<<std::endl; > + } First: as I've already mentioned, please don't write anything to to stderr or stdout directly. Use the sql_print_error() function. Second: the message is not informative. The above will print something like: got_name[1] != named_item_expected[0] which is not at all informative. What's "got_name[1]" ? Please change the printout to be something like: Json_writer: attempt to write invalid JSON. Last data in JSON: %s here %s should print last 256 chars in the JSON document. > VALIDITY_ASSERT(got_name == named_item_expected()); > named_items_expectation.push_back(true); > } > @@ -140,7 +150,19 @@ Json_writer& Json_writer::add_member(const char *name, size_t len) > } > #if !defined(NDEBUG) || defined(JSON_WRITER_UNIT_TEST) > if (!fmt_helper.is_making_writer_calls()) > + { > + VALIDITY_ASSERT(!got_name); > got_name= true; > + VALIDITY_ASSERT(named_items.size()); > + auto& named_items_keys= named_items.top(); > + auto emplaced= named_items_keys.emplace(name, len); > + auto is_uniq_key= emplaced.second; > + if(!is_uniq_key) > + { > + std::cerr << "Duplicated key: " << *emplaced.first << std::endl; > + VALIDITY_ASSERT(is_uniq_key); Same input as above: use sql_print_error and make the error message helpful. > + } > + } > #endif > return *this; > } > commit 57e8f68955954ff96104c8356733bfa73feae849 (HEAD -> bb-10.8-MDEV-27036, origin/bb-10.8-MDEV-27036) > Author: Sergei Krivonos <sergei.krivonos(a)mariadb.com> > Date: Mon Nov 15 05:57:25 2021 +0200 > > MDEV-27036: unittest JSON object member name collision > > diff --git a/unittest/sql/my_json_writer-t.cc b/unittest/sql/my_json_writer-t.cc > index 6398a589c33..7d20802527d 100644 > --- a/unittest/sql/my_json_writer-t.cc > +++ b/unittest/sql/my_json_writer-t.cc > @@ -119,7 +119,14 @@ int main(int args, char **argv) > ok(w.invalid_json, "JSON array end of object"); > } > > - > + { > + Json_writer w; > + w.start_object(); > + w.add_member("name").add_ll(1); > + w.add_member("name").add_ll(2); > + w.end_object(); > + ok(w.invalid_json, "JSON object member name collision"); > + } Please add more unit test coverage. Check that this is accepted: { "foo": { "foo": {} } } Check that this is not accepted: { "foo": { "bar": {} }, "foo": "something else" } BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] 37b6f5cb8ef: decouple commit/rollback code from binlog hton
by Sergei Golubchik 23 Nov '21

23 Nov '21

Hi, Nikita! On Nov 15, Nikita Malyavin wrote: > revision-id: 37b6f5cb8ef (mariadb-10.5.2-484-g37b6f5cb8ef) > parent(s): f3603cbfaeb > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-29 03:47:52 +1000 > message: > > decouple commit/rollback code from binlog hton let's squash it with "[6/8] store cache managers in a list" Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] ad6e7b87107: introduce cache flipping
by Sergei Golubchik 23 Nov '21

23 Nov '21

Hi, Nikita! Interesting trick. On Nov 13, Nikita Malyavin wrote: > revision-id: ad6e7b87107 (mariadb-10.5.2-477-gad6e7b87107) > parent(s): 85fbd867b4c > author: Nikita Malyavin > committer: Nikita Malyavin > timestamp: 2021-01-27 14:17:39 +1000 > message: > > introduce cache flipping Could you add few lines from the Cache_flip_event_log comment here too please? > diff --git a/sql/log.h b/sql/log.h > index 74c409e1ac7..1e422ae4524 100644 > --- a/sql/log.h > +++ b/sql/log.h > @@ -424,10 +424,9 @@ class Event_log: public MYSQL_LOG > > bool open( > const char *log_name, > - enum_log_type log_type, > const char *new_name, ulong next_file_number, > enum cache_type io_cache_type_arg); > - IO_CACHE *get_log_file() { return &log_file; } > + virtual IO_CACHE *get_log_file() { return &log_file; } > > int write_description_event(enum_binlog_checksum_alg checksum_alg, > bool encrypt, bool dont_set_created); > @@ -435,6 +434,73 @@ class Event_log: public MYSQL_LOG > bool write_event(Log_event *ev, binlog_cache_data *data, IO_CACHE *file); > }; > > +/** > + A single-reader, multiple-writer non-blocking layer for Event_log. > + Provides IO_CACHE for writing and IO_CACHE for reading.\ > + > + Writers use an overrided get_log_file version for their writes, while readers > + should use flip() to initiate reading. > + flip() swaps pointers to allow non-blocking reads. > + > + Writers can block other writers and a reader with a mutex, but a reader only > + swaps two pointers under a lock, so it won't block writers. > + > + TODO should be unnecessary after MDEV-24676 is done > + */ > +class Cache_flip_event_log: public Event_log { > + IO_CACHE alt_buf; > + IO_CACHE *current, *alt; > +public: > + > + Cache_flip_event_log() : Event_log(), alt_buf{}, > + current(&log_file), alt(&alt_buf) {} > + bool open(enum cache_type io_cache_type_arg) > + { > + log_file.dir= mysql_tmpdir; > + alt_buf.dir= log_file.dir; > + bool res= Event_log::open(NULL, NULL, 0, io_cache_type_arg); > + if (res) > + return res; > + > + name= my_strdup(key_memory_MYSQL_LOG_name, "online-alter-binlog", > + MYF(MY_WME)); > + if (!name) > + return false; > + > + res= init_io_cache(&alt_buf, -1, LOG_BIN_IO_SIZE, io_cache_type_arg, 0, 0, > + MYF(MY_WME | MY_NABP | MY_WAIT_IF_FULL)) != 0; > + return res; > + } > + > + /** > + Swaps current and alt_log. Can be called only from the reader thread. > + @return a new IO_CACHE pointer to read from. > + */ > + IO_CACHE *flip() > + { > + IO_CACHE *tmp= current; > + reinit_io_cache(alt, WRITE_CACHE, 0, 0, 0); > + mysql_mutex_lock(get_log_lock()); > + reinit_io_cache(current, READ_CACHE, 0, 0, 0); > + current= alt; > + mysql_mutex_unlock(get_log_lock()); > + alt= tmp; > + > + return alt; > + } > + > + IO_CACHE *get_log_file() override > + { > + return current; may be mysql_mutex_assert_owner(get_log_lock()) ? > + } > + > + void cleanup() > + { > + end_io_cache(&alt_buf); > + Event_log::cleanup(); > + } > +}; > + Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] c80991c79f7: MDEV-25785 Add support for OpenSSL 3.0
by Sergei Golubchik 21 Nov '21

21 Nov '21

Hi, Vladislav! few questions below: On Nov 18, Vladislav Vaintroub wrote: > revision-id: c80991c79f7 (mariadb-10.6.1-213-gc80991c79f7) > parent(s): cee33f1ab7c > author: Vladislav Vaintroub > committer: Vladislav Vaintroub > timestamp: 2021-11-09 02:04:22 +0100 > message: > > MDEV-25785 Add support for OpenSSL 3.0 > > Summary of changes > > - MD_CTX_SIZE is increased > > - EVP_CIPHER_CTX_buf_noconst(ctx) does not work anymore, points > to nobody knows where. The assumption made previously was that > (since the function does not seem to be documented) > was that it points to the last partial source block. > Add own partial block buffer for NOPAD encryption instead > > - SECLEVEL in CipherString in openssl.cnf > had been downgraded to 0, from 1, to make TLSv1.0 and TLSv1.1 possible The definition of SECLEVEL is: Level 0 Everything is permitted. This retains compatibility with previous versions of OpenSSL. Level 1 The security level corresponds to a minimum of 80 bits of security. Any parameters offering below 80 bits of security are excluded. As a result RSA, DSA and DH keys shorter than 1024 bits and ECC keys shorter than 160 bits are prohibited. All export cipher suites are prohibited since they all offer less than 80 bits of security. SSL version 2 is prohibited. Any cipher suite using MD5 for the MAC is also prohibited. Note that signatures using SHA1 and MD5 are also forbidden at this level as they have less than 80 security bits. Only at level 3 does it say "TLS versions below 1.1 are not permitted." I don't see why you had to change from 1 to 0. Do we have "signatures using SHA1 and MD5"? > - ctx_buf buffer now must be aligned to 16 bytes with openssl( > previously with WolfSSL only), ot crashes will happen > > - updated aes-t , to be better debuggable > using function, rather than a huge multiline macro > added test that does "nopad" encryption piece-wise, to test > replacement of EVP_CIPHER_CTX_buf_noconst > > diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake > index 7c2488be8bd..64c93ff9b4f 100644 > --- a/cmake/ssl.cmake > +++ b/cmake/ssl.cmake > @@ -139,9 +139,20 @@ MACRO (MYSQL_CHECK_SSL) > SET(SSL_INTERNAL_INCLUDE_DIRS "") > SET(SSL_DEFINES "-DHAVE_OPENSSL") > > + FOREACH(x INCLUDES LIBRARIES DEFINITIONS) > + SET(SAVE_CMAKE_REQUIRED_${x} ${CMAKE_REQUIRED_${x}}) > + ENDFOREACH() why do you set/restore them? > + # Silence "deprecated in OpenSSL 3.0" > + IF((NOT OPENSSL_VERSION) # 3.0 not determined by older cmake > + OR NOT(OPENSSL_VERSION VERSION_LESS "3.0.0")) > + SET(SSL_DEFINES "${SSL_DEFINES} -DOPENSSL_API_COMPAT=0x10100000L") > + SET(CMAKE_REQUIRED_DEFINITIONS -DOPENSSL_API_COMPAT=0x10100000L) > + ENDIF() This is just postponing the inevitable. They'll drop the old API eventually. As far as I understand the internals were changed in a way that doesn't fit the old API. Is there some safe subset of OpenSSL API that works both in 1.0 and in 3.0 ? It might be more future proof to use only that. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

3 4

Re: [Maria-developers] 4492c869f31: MDEV-26238: Remove inconsistent behaviour of --default-* options in my_print_defaults
by Sergei Golubchik 18 Nov '21

18 Nov '21

Hi, Rucha! Thanks! Looks good now. One comment about tests: On Nov 18, Rucha Deodhar wrote: > revision-id: 4492c869f31 (mariadb-10.6.1-212-g4492c869f31) > parent(s): 3f3ec40c91b > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-11-08 16:51:33 +0530 > message: > > MDEV-26238: Remove inconsistent behaviour of --default-* options > in my_print_defaults ... > diff --git a/mysql-test/main/my_print_defaults.test b/mysql-test/main/my_print_defaults.test > index bfd4e563826..b52deaf09ff 100644 > --- a/mysql-test/main/my_print_defaults.test > +++ b/mysql-test/main/my_print_defaults.test > @@ -87,20 +85,74 @@ read_buffer_size=1M ... > +--echo # checking that --defaults* option only works when mentioned at beginning > + > +--echo # Testing --defaults-file at beginning only > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld > +--remove_file $MYSQLTEST_VARDIR/tmp/tmp3.cnf How does it test that "--defaults* option only works at beginning"? You test that --defaults-file works at the beginning all right. This is redundant, as there are lots of tests in this file that have --defaults-file, but it doesn't hurt. But you don't test the "only" part. You don't test anything that you've changed - I suspect your new test would happily pass without any changes in my_print_defaults.c To test that --defaults-file works *only* at the beginning, you need to verify that it fails not at the beginning. Like error 1 exec $MYSQL_MY_PRINT_DEFAULTS --myslqd --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf also with -c error 1 exec $MYSQL_MY_PRINT_DEFAULTS -c $MYSQLTEST_VARDIR/tmp/tmp1.cnf --mysqld This should fail even at the beginning. The point is - you need to test the behavior that you've changed. As your test has introduced failures - you need to test for these failures. But all your tests succeed. > +--echo # Testing --defaults-extra-file works at beginning only > +--exec $MYSQL_MY_PRINT_DEFAULTS --defaults-file=$MYSQLTEST_VARDIR/tmp/tmp1.cnf --defaults-extra-file=$MYSQLTEST_VARDIR/tmp/tmp2.cnf --mysqld Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-27036: do not output empty subqueries in traces
by Sergey Petrunia 16 Nov '21

16 Nov '21

Hi Sergei, > commit eb7d6304a2aa168e09d48f6884dadf8c9f868feb > Author: Sergei Krivonos <sergei.krivonos(a)mariadb.com> > Date: Mon Nov 15 01:37:04 2021 +0200 > > do not output empty subqueries in traces This reads as if the trace can have an empty clause describing a subquery. This is very confusing. Actually, the patch removes potential empty "subqueries" arrays. Do you have a testcase for this? I'm trying to find one with grep -rn -A1 '"subqueries"' mysql-test/{main,suite} and I can't. > diff --git a/sql/sql_explain.cc b/sql/sql_explain.cc > index 499b6dbc97a..9a9bf786d62 100644 > --- a/sql/sql_explain.cc > +++ b/sql/sql_explain.cc > @@ -25,6 +25,8 @@ > #include "opt_range.h" > #include "sql_expression_cache.h" > > +#include <algorithm> > + Is this needed? It seems to work without it. > + bool Explain_node::has_json_printable_connection_objects(Explain_query *query) const > + { > + auto n= children.elements(); > + if (n) > + { > + for (; n--;) > + { Why such a convoluted loop inside an if statement? Can you make the code simpler by using for (n=0; n < children.elements(); n++) { if (is_connection_printable_in_json(children.at(i)->connection_type)) return true; } return false; ? > + auto node= query->get_node(children.at(n)); > + /* Derived tables are printed inside Explain_table_access objects */ > + > + if (!is_connection_printable_in_json(node->connection_type)) > + continue; > + > + return true; > + } > + } > + return false; > + } BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] MDEV-27036: duplicated keys in best_access_path
by Sergey Petrunia 16 Nov '21

16 Nov '21

Hi Sergei, > commit d2b4782200ccfd0850f36d25c75082974e4b86e1 > Author: Sergei Krivonos <sergei.krivonos(a)mariadb.com> > Date: Sun Nov 14 10:37:20 2021 +0200 > > MDEV-27036: duplicated keys in best_access_path > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 3272f8b7f65..29e9c0efe36 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -7611,16 +7611,14 @@ best_access_path(JOIN *join, > } > else > { > + trace_access_idx.add("used_range_estimates", false); > if (table->quick_keys.is_set(key)) > { > - trace_access_idx.add("used_range_estimates",false) > - .add("cause", > - "not better than ref estimates"); > + cause= "not better than ref estimates"; Issue #1: you make this change here, in the branch marked as ReuseRangeEstimateForRef-2. But there's very similar code right above, in ReuseRangeEstimateForRef-1 branch: /* quick_range couldn't use key! */ records= (double) s->records/rec; trace_access_idx.add("used_range_estimates", false) .add("cause", "not available"); Issue #2: this patch causes some information to be removed from the trace! How about an alternative solution: change the name from "cause" here to "reason"? > } > else > { > - trace_access_idx.add("used_range_estimates", false) > - .add("cause", "not available"); > + cause= "not available"; > } > } > } BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

[Maria-developers] MDEV-26929: Please enable optimizer trace run in buildbot JSON
by Sergey Petrunia 09 Nov '21

09 Nov '21

Hi Elena, The patch for https://jira.mariadb.org/browse/MDEV-26929 has been merged from 10.4 into all versions up to 10.8. Could you enable the optimizer trace run in the buildbot please? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

2 2

Re: [Maria-developers] 80749146b7d: MDEV-26238: Remove inconsistent behaviour of --default-* options in my_print_defaults
by Sergei Golubchik 07 Nov '21

07 Nov '21

Hi, Rucha! On Nov 05, Rucha Deodhar wrote: > > On Nov 04, Rucha Deodhar wrote: > > > revision-id: 80749146b7d (mariadb-10.6.1-77-g80749146b7d) > > > parent(s): 76149650764 > > > author: Rucha Deodhar > > > committer: Rucha Deodhar > > > timestamp: 2021-09-12 02:11:19 +0530 > > > message: > > > > > > MDEV-26238: Remove inconsistent behaviour of --default-* options > > > in my_print_defaults > > > > > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > > > index b7f52382721..607509a309e 100644 > > > --- a/extra/my_print_defaults.c > > > +++ b/extra/my_print_defaults.c > > > @@ -141,50 +124,39 @@ static int get_options(int *argc,char ***argv) > > ... > > > + /* > > > + We already process --defaults* options at the beginning in > > > + get_defaults_options(). So skip --defaults* options and > > > + pass remaining options to handle_options(). > > > + */ > > > + if (is_prefix(org_argv[1], "--no-defaults")) > > > + args_used--; > > > > I'm sorry, I don't understand. Why do you need that? > > I made it to keep the output consistent with previous version. > > If --no-defaults is present in the command line, then it is counted in > args_used when get_defaults_options() is called. Later we skip > "args_used" number of arguments so that only non-default options > mentioned after --default-* options are passed to get_options(). > > The above code decreases this count when --no-defaults is present so > that we don't skip it unlike --defaults-* options and pass it to > get_options(). It is required to pass --no-defaults to get_options() > because if it is not passed then it produces output which looks like > --help ( if no other options are there in addition to --no-defaults). > If it is passed to get_options(), then --no-defaults produces no > output like it does in 10.6. But it's correct, isn't it? If you run my_print_defaults with no groups, it'll print the help text. Try, for example, my_print_defaults --defaults-file=var/my.cnf (presuming you're in mysql-test dir). You'll get the help text, because you didn't specify any groups. Or my_print_defaults --defaults-extra-file=var/my.cnf But my_print_defaults --no-defaults doesn't do it. Doesn't behave like other *defaults* options. We can easily preserve the inconsisent "if --no-defaults, no group is required" behavior with, like if (my_no_defaults) cleanup_and_exit(0); (and add my_no_defaults to my_default.h). But I don't think it's necessary. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 50f43d46299: MDEV-14938 make buildbot to include galera into bintars
by Sergei Golubchik 05 Nov '21

05 Nov '21

Hi, Alexey! On Nov 05, Alexey Bychko wrote: > revision-id: 50f43d46299 (mariadb-10.2.40-106-g50f43d46299) > parent(s): 05c3dced861 > author: Alexey Bychko > committer: Alexey Bychko > timestamp: 2021-10-22 16:30:56 +0700 > message: > > MDEV-14938 make buildbot to include galera into bintars > > this commit adds cmake module to process external directory with built > galera to include galera library and binaries into server build. > it introduces new cmake variable EXT_GALERA_PATH representing relative > or absolute path to compiled galera tree. No compiled galera tree. This is useless. MDEV description says Currently bintars that buildbot produces don't contain libgalera_smm.so. Publishing scripts repackage bintars to add it. We want to produce correct packages in buildbot and release exactly what buildbot builds. It means, buildbot need to produce *exactly* what `prep` script was generating. `prep` script does not build galera. This is a completely separate step, `prep` only packages pre-existing galera files. It's enough to assume that there's ${EXT_GALERA_PATH}/garbd and ${EXT_GALERA_PATH}/libgalera_smm.so. Or if you want it to be really universal, make two different variables, cmake -DBINTAR_GARBD=/path/to/garbd -DBINTAR_LIBGALERA=/path/to/libgalera_smm.so This will work with a compiled galera tree, with installed galera package, with just garbd and libgalera_smm.so in $HOME, with everything. > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 67216e0e443..1107013860c 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -153,6 +153,7 @@ INCLUDE(mysql_version) > INCLUDE(cpack_source_ignore_files) > INCLUDE(install_layout) > INCLUDE(wsrep) > +INCLUDE(galera_external) > > # Add macros > INCLUDE(character_sets) > diff --git a/cmake/galera_external.cmake b/cmake/galera_external.cmake > new file mode 100644 > index 00000000000..64ddfc9c498 > --- /dev/null > +++ b/cmake/galera_external.cmake > @@ -0,0 +1,75 @@ > +# EXT_GALERA_PATH > +# a path to built galera git tree > +# or a path to unpacked binary tarball > + > +IF(WIN32 OR NOT EXT_GALERA_PATH) why do you need WIN32 check here? > + RETURN() > +ENDIF() > + > +IF(NOT EXISTS ${EXT_GALERA_PATH}) > + MESSAGE(FATAL_ERROR "Galera dir ${EXT_GALERA_PATH} does not exist!") > +ENDIF() > + > +SET(GALERA_PATH ${CMAKE_SOURCE_DIR}/GALERASYM) > +IF(EXISTS ${GALERA_PATH}) > + FILE({REMOVE ${GALERA_PATH}) > +ENDIF() > +EXECUTE_PROCESS(COMMAND ${CMAKE_COMMAND} -E create_symlink ${EXT_GALERA_PATH} ${GALERA_PATH}) Why do you need this symlink? > + > +FIND_LIBRARY(GALERA_LIB > + NAMES galera_smm galera_enterprise_smm > + PATHS ${GALERA_PATH} > + NO_DEFAULT_PATH > + ) > + > +IF(NOT GALERA_LIB) > + MESSAGE(FATAL_ERROR "Galera library not found in ${GALERA_PATH}") > +ENDIF() > + > +INSTALL(PROGRAMS > + ${GALERA_LIB} > + DESTINATION lib > + RENAME libgalera_smm.so > + COMPONENT Server > + ) > + > +INSTALL(PROGRAMS > + ${GALERA_PATH}/garb/garbd > + DESTINATION bin > + COMPONENT Server > + ) > + > +INSTALL(PROGRAMS > + ${GALERA_PATH}/garb/files/garb-systemd > + DESTINATION bin > + COMPONENT Server > + ) > + > +INSTALL(FILES > + ${GALERA_PATH}/garb/files/garb.cnf > + DESTINATION docs/galera > + COMPONENT Server > + ) MDEV description says Currently bintars that buildbot produces don't contain libgalera_smm.so. Publishing scripts repackage bintars to add it. We want to produce correct packages in buildbot and release exactly what buildbot builds. It means, buildbot need to produce *exactly* what `prep` script was generating. Meaning three galera files. Like: -rwxr-xr-x mariadb-10.2.40-linux-glibc_214-x86_64/bin/garbd -rw-r--r-- mariadb-10.2.40-linux-glibc_214-x86_64/lib/libgalera_smm.so lrwxrwxrwx mariadb-10.2.40-linux-glibc_214-x86_64/lib/galera/libgalera_smm.so -> ../libgalera_smm.so Nothing else. Also, please, verify that the generated directory inside the tarball matches the pattern above. And that the tarbal name does too. > + > +INSTALL(FILES > + ${GALERA_PATH}/AUTHORS > + ${GALERA_PATH}/COPYING > + ${GALERA_PATH}/README > + DESTINATION docs/galera/doc > + COMPONENT Server > + ) > + > +INSTALL(FILES > + ${GALERA_PATH}/asio/LICENSE_1_0.txt > + DESTINATION docs/galera/doc > + RENAME LICENSE.asio > + COMPONENT Server > + ) > + > +INSTALL(FILES > + ${GALERA_PATH}/chromium/LICENSE > + DESTINATION docs/galera/doc > + RENAME LICENSE.chromium > + COMPONENT Server > + ) > + > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-23766: Optimizer tracing code is prone to producing invalid JSON
by Sergey Petrunia 05 Nov '21

05 Nov '21

Hi Serg, On Tue, Nov 02, 2021 at 02:19:05AM +0200, Sergei Krivonos wrote: > Applied fixes by your tests patch 638930f85f… <https://github.com/MariaDB/server/commit/638930f85ff40bd39354da81e53e7278a4…> > #1: Please check out the commit adding unit tests: http://lists.askmonty.org/pipermail/commits/2021-November/014773.html It turns out it is not that hard to add them. Any comments/ways to improve this? #2: In the above commit, note that two last unit tests show a scenario that is not currently checked. #3: What is the point of functions Json_writer::on_add_str(), Json_writer::on_start_array, Json_writer::on_start_object() ? I would understand if all the checking code was isolated in these functions. But it isn't. Please either remove the functions or explain the reasoning. #4 See the input below about the commit comment. Please change (feel free to force-push into the branch after that). I think this is all the input, and the patch will be ready for pushing once all of this is addressed. > > Item #2: please improve the commit comment as was requested in my previous > > e-mail: > > > > On Thu, Oct 28, 2021 at 04:58:35PM +0300, Sergey Petrunia wrote: > >> First, input on the commit comment. Please consider it as a request applying > >> to ALL further commits to the MariaDB codebase. > >> > >> > >>> MDEV-23766: implemented requested debug checks > >> > >> Imagine somebody looking at this in a few years. Will they know what checks > >> were requested? They might get a clue by looking at the Jira text, but the > >> idea to make the commit comments concise and self-contained descriptions. > >> > >> Something like this: > >> > >> Line #1: one-line description of the patch: > >> > >>>> MDEV-23766: Make attempts to write invalid JSON cause assertion failures > >> > >> Subsequent lines: > >> > >>>> Make JSON writing API (class Json_writer) check the produced JSON document > >>>> is valid. The following checks are made: > >>>> - JSON objects must contain named members > >>>> - JSON arrays must contain unnamed members. > >>>> - (TODO: add other restrictions we're enforcing). BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] 80749146b7d: MDEV-26238: Remove inconsistent behaviour of --default-* options in my_print_defaults
by Sergei Golubchik 04 Nov '21

04 Nov '21

Hi, Rucha! On Nov 04, Rucha Deodhar wrote: > revision-id: 80749146b7d (mariadb-10.6.1-77-g80749146b7d) > parent(s): 76149650764 > author: Rucha Deodhar > committer: Rucha Deodhar > timestamp: 2021-09-12 02:11:19 +0530 > message: > > MDEV-26238: Remove inconsistent behaviour of --default-* options > in my_print_defaults > > Analysis: --defaults* option is recognized anywhere in the commandline > instead of only at the beginning because handle_options() recognizes > options in any order. > Fix: use get_defaults_options() which recognizes --defaults* options only at > the beginning. After this is done, we only want to recognize other options > given in any order which can be done using handle_options(). So only skip > --defaults* options and pass rest of them to handle_options(). > Also, removed -e, -g and -c because only my_print_defaults supports them. > > diff --git a/extra/my_print_defaults.c b/extra/my_print_defaults.c > index b7f52382721..607509a309e 100644 > --- a/extra/my_print_defaults.c > +++ b/extra/my_print_defaults.c > @@ -141,50 +124,39 @@ static int get_options(int *argc,char ***argv) ... > int main(int argc, char **argv) > { > - int count= 0, error, no_defaults= 0; > + int count, error, args_used; > char **load_default_groups= 0, *tmp_arguments[6]; > char **argument, **arguments, **org_argv; > int nargs, i= 0; > MY_INIT(argv[0]); > > org_argv= argv; > - if (*argv && !strcmp(*argv, "--no-defaults")) > - { > - argv++; > - ++count; > - no_defaults= 1; > - } > - /* Copy program name and --no-defaults if present*/ > + args_used= get_defaults_options(argv); > + > + /* Copy defaults-xxx arguments & program name */ > + count= args_used; > arguments= tmp_arguments; > - memcpy((char*) arguments, (char*) org_argv, (++count)*sizeof(*org_argv)); > + memcpy((char*) arguments, (char*) org_argv, count*sizeof(*org_argv)); > arguments[count]= 0; I still cannot understand why you're doing it. Why do you need a copy of argv? > > + /* > + We already process --defaults* options at the beginning in > + get_defaults_options(). So skip --defaults* options and > + pass remaining options to handle_options(). > + */ > + if (is_prefix(org_argv[1], "--no-defaults")) > + args_used--; I'm sorry, I don't understand. Why do you need that? all *defaults* options were handled in get_defaults_options(). > + why do you need org_argv? First you do org_argv= argv; then after some code that doesn't change either argv or org_argv, you copy the vallue back: > + argv=org_argv; > + argv+=args_used-1; > + argc-=args_used-1; > + > /* Check out the args */ > if (get_options(&argc,&argv)) > cleanup_and_exit(1); > > - if (!no_defaults) > - { > - if (opt_defaults_file_used) > - arguments[count++]= make_args("--defaults-file=", config_file); > - if (my_defaults_extra_file) > - arguments[count++]= make_args("--defaults-extra-file=", > - my_defaults_extra_file); > - if (my_defaults_group_suffix) > - arguments[count++]= make_args("--defaults-group-suffix=", > - my_defaults_group_suffix); > - arguments[count]= 0; > - } > - > nargs= argc + 1; > if (opt_mysqld) > nargs+= array_elements(mysqld_groups); Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

[Maria-developers] [MDEV-10654] Review request
by Manohar K B 04 Nov '21

04 Nov '21

Hi All, Good day. I have fixed ticket MDEV-10654 and raised PR. Can someone kindly review it? This is the first time I am contributing to MariaDB, so please guide me if I missed anything. Best regards, Manohar.

2 1

Re: [Maria-developers] bb1737b0482: MDEV-12459: The information_schema tables for getting temporary tables info is missing, at least for innodb there is no INNODB_TEMP_TABLE_INFO
by Sergei Golubchik 01 Nov '21

01 Nov '21

Hi, Anel! On Nov 01, Anel Husakovic wrote: > revision-id: bb1737b0482 (mariadb-10.6.1-73-gbb1737b0482) > parent(s): 1bc82aaf0a7 > author: Anel Husakovic > committer: Anel Husakovic > timestamp: 2021-09-06 08:58:46 +0200 > message: > > MDEV-12459: The information_schema tables for getting temporary tables info is missing, at least for innodb there is no INNODB_TEMP_TABLE_INFO > > diff --git a/sql/sql_show.cc b/sql/sql_show.cc > index 6b351726320..5444de8c1ca 100644 > --- a/sql/sql_show.cc > +++ b/sql/sql_show.cc > @@ -69,6 +69,8 @@ > > #include "lex_symbol.h" > #define KEYWORD_SIZE 64 > +#define IS_USER_TEMP_TABLE(A) ((A->tmp_table == TRANSACTIONAL_TMP_TABLE) || \ > + (A->tmp_table == NON_TRANSACTIONAL_TMP_TABLE)) > > extern SYMBOL symbols[]; > extern size_t symbols_length; > @@ -151,6 +153,8 @@ static int show_create_sequence(THD *thd, TABLE_LIST *table_list, > static const LEX_CSTRING *view_algorithm(TABLE_LIST *table); > > bool get_lookup_field_values(THD *, COND *, TABLE_LIST *, LOOKUP_FIELD_VALUES *); > +void process_i_s_table_temporary_tables(THD *thd, TABLE * table, LEX_CSTRING *db_name, > + LEX_CSTRING *table_name, TABLE *tmp_tbl); > > /** > Try to lock a mutex, but give up after a short while to not cause deadlocks > @@ -5161,6 +5166,10 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > uint table_open_method= tables->table_open_method; > bool can_deadlock; > MEM_ROOT tmp_mem_root; > + Dynamic_array<LEX_CSTRING> system_tables(PSI_INSTRUMENT_MEM); > + All_tmp_tables_list *temp_tables= NULL; > + // Scan for temporary tables > + TMP_TABLE_SHARE *share_temp; > DBUG_ENTER("get_all_tables"); > > bzero(&tmp_mem_root, sizeof(tmp_mem_root)); > @@ -5223,6 +5232,12 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > init_alloc_root(PSI_INSTRUMENT_ME, &tmp_mem_root, SHOW_ALLOC_BLOCK_SIZE, > SHOW_ALLOC_BLOCK_SIZE, MY_THREAD_SPECIFIC); > > + system_tables.push(INFORMATION_SCHEMA_NAME); > + system_tables.push(PERFORMANCE_SCHEMA_DB_NAME); > + system_tables.push(MYSQL_SCHEMA_NAME); > + system_tables.push(SYS_SCHEMA_NAME); > + system_tables.push(MTR_SCHEMA_NAME); > + > for (size_t i=0; i < db_names.elements(); i++) > { > LEX_CSTRING *db_name= db_names.at(i); > @@ -5234,6 +5249,36 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) > sctx->master_access & (DB_ACLS | SHOW_DB_ACL) || > acl_get(sctx->host, sctx->ip, sctx->priv_user, db_name->str, 0)) > #endif > + > + // Only if there is IS.tables allow temporary tables to be shown > + if (schema_table_idx == SCH_TABLES && !temp_tables) > + { > + for (size_t k=0; k < system_tables.elements(); k++) > + { > + if (db_name != &system_tables.at(k)) > + { > + temp_tables= open_tables_state_backup.temporary_tables; > + if (temp_tables) > + { > + All_tmp_tables_list::Iterator it(*open_tables_state_backup.temporary_tables); > + while ((share_temp= it++)) > + { > + All_share_tables_list::Iterator it2(share_temp->all_tmp_tables); > + while (TABLE *tbl= it2++) > + { > + if (IS_USER_TEMP_TABLE(share_temp)) > + { > + // Now we have the data and we should process_table() manually > + process_i_s_table_temporary_tables(thd, table, db_name, > + &share_temp->table_name, tbl); > + } > + } > + } > + } > + break; > + } > + } > + } hmmmm. What do you do here? I understand, basically, nothing here. Could you explain, line by line? > { > Dynamic_array<LEX_CSTRING*> table_names(PSI_INSTRUMENT_MEM); > int res= make_table_name_list(thd, &table_names, lex, > @@ -5495,6 +5540,8 @@ static int get_schema_tables_record(THD *thd, TABLE_LIST *tables, > table->field[3]->store(STRING_WITH_LEN("SYSTEM VIEW"), cs); > else if (share->table_type == TABLE_TYPE_SEQUENCE) > table->field[3]->store(STRING_WITH_LEN("SEQUENCE"), cs); > + else if (IS_USER_TEMP_TABLE(share)) > + table->field[3]->store(STRING_WITH_LEN("TEMPORARY"), cs); what about INTERNAL_TMP_TABLE and SYSTEM_TMP_TABLE? > else > { > DBUG_ASSERT(share->tmp_table == NO_TMP_TABLE); > @@ -9032,7 +9096,6 @@ ST_FIELD_INFO tables_fields_info[]= > NOT_NULL, "Comment", OPEN_FRM_ONLY), > Column("MAX_INDEX_LENGTH",ULonglong(), NULLABLE, "Max_index_length", > OPEN_FULL_TABLE), > - Column("TEMPORARY", Varchar(1), NULLABLE, "Temporary", OPEN_FRM_ONLY), > CEnd() > }; Let's remove it in a separate commit. After you implement "TEMPORARY" for table type. Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-23766:Optimizer tracing code is prone to producing invalid JSON
by Sergey Petrunia 31 Oct '21

31 Oct '21

Hi Serg, Ok the patch moving in the right direction but it's not there, yet. Please find the input below. First, input on the commit comment. Please consider it as a request applying to ALL further commits to the MariaDB codebase. > MDEV-23766: implemented requested debug checks Imagine somebody looking at this in a few years. Will they know what checks were requested? They might get a clue by looking at the Jira text, but the idea to make the commit comments concise and self-contained descriptions. Something like this: Line #1: one-line description of the patch: >> MDEV-23766: Make attempts to write invalid JSON cause assertion failures Subsequent lines: >> Make JSON writing API (class Json_writer) check the produced JSON document >> is valid. The following checks are made: >> - JSON objects must contain named members >> - JSON arrays must contain unnamed members. >> - (TODO: add other restrictions we're enforcing). It is not clear how the checker interacts with Single_line_formatting_helper. The code like one the one I'm quoting below shows some non-trivial overlap: > @@ -28,7 +36,16 @@ void Json_writer::append_indent() > > void Json_writer::start_object() > { > +#ifndef NDEBUG > + DBUG_ASSERT(got_name == named_item_expected()); > + named_items_expectation.push_back(true); > + size_t depth= named_items_expectation.size(); > +#endif > fmt_helper.on_start_object(); > +#ifndef NDEBUG > + if (depth != named_items_expectation.size()) > + named_items_expectation.pop_back(); > +#endif > > if (!element_started) > start_element(); This looks very cryptic. Can you elaborate about this? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 1

Re: [Maria-developers] 85aa3f8cf66: MDEV-26102: dgcov: add support for *.gcda.gcov.json.gz files of gcov 9.1+
by Sergei Golubchik 30 Oct '21

30 Oct '21

Hi, Anel! Looks ok. Except that you have that pointless $gcc_version=~ line that I wrote about in a previous review. Did you verify that this script actually delivers meaningful results? On Oct 30, Anel Husakovic wrote: > revision-id: 85aa3f8cf66 (mariadb-10.2.39-96-g85aa3f8cf66) > parent(s): afe00bb7cce > author: Anel Husakovic > committer: Anel Husakovic > timestamp: 2021-07-29 12:42:44 +0200 > message: > > MDEV-26102: dgcov: add support for *.gcda.gcov.json.gz files of gcov 9.1+ > > Reviewed by: serg(a)mariadb.com > > --- > mysql-test/dgcov.pl | 55 +++++++++++++++++++++++++++++++++++------------------ > 1 file changed, 37 insertions(+), 18 deletions(-) > > diff --git a/mysql-test/dgcov.pl b/mysql-test/dgcov.pl > index 2c00c64d1ff..543099c1b4c 100755 > --- a/mysql-test/dgcov.pl > +++ b/mysql-test/dgcov.pl > @@ -62,12 +62,15 @@ my $res; > + > +my $gcc_version= `gcc -dumpversion`; > +$gcc_version=~ s/(\d).*$/$1/; > > find(\&gcov_one_file, $root); > find(\&write_coverage, $root) if $opt_generate; Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0