December 2014 - developers - lists.mariadb.org

[Maria-developers] JIRA ping
by Jonas Oreland 23 Dec '14

23 Dec '14

Hi Kristian, I light of recent "I did see any of your comments until by accident just now", I now mail you directly that I updated both MDEV-162 and MDEV-7257. I want (review) comments. /Jonas

2 9

[Maria-developers] Review of Debian packaging file listing?
by Otto Kekäläinen 22 Dec '14

22 Dec '14

Hello! I recently updated my build scripts to automatically generate a file listing for each commit in in the Debian packaging "pool" I have. Here is a recent one: http://labs.seravo.fi/~otto/mariadb-repo/mariadb-10.0-sid-amd64/filelist-ec… It is now very easy to review and track what files are in what package. Could somebody please quickly review that file and tell me if you spot: - any files in wrong package and should be moved? - any package that has too few files and maybe something missing? - any file attributes that might be wrong or insecure? - should any of the MariaDB plugins deserve to be shipped as a separate package? (apart from current connect and oqgraph) -- Check out our blog at http://seravo.fi/blog and follow @ottokekalainen

4 5

[Maria-developers] Report from Debian/Ubuntu MySQL packaging sprint in London in December
by Otto Kekäläinen 21 Dec '14

21 Dec '14

Hello! I thought I'd brief the readers of this mailing list about the packaging sprint in London last week. It was organized by Canonical and in addition to Canonical staff and me there where also people from Oracle and Percona. We mostly worked on syncing the MySQL/MariaDB/Percona server packaging and a little bit also on creating a unified Galera package. We made good progress and all results have been pushed to public repos. I also recently updated the pages https://wiki.debian.org/Teams/MySQL/MariaDB and https://wiki.debian.org/Teams/MySQL/Galera to reflect the current status. One big technical change that was discussed and planned is the decoupling of the config and data directories. The plan is that in the future no MySQL variant in particular would anymore own the /etc/mysql/my.cnf file but instead each variant would register their own config file at installation time by using the Debian update-alternatives mechanism. So in future there would be /etc/mysql/mariadb.conf, mysql.conf, percona.conf etc and some more snippets in /etc/mysql/mariadb.conf.d/.. etc. The plan is not complete nor tested, but there are preliminary changes in the mysql-common package to provide the config file registration hook and in a preliminary Percona Debian package to use the new config file registration facility. As the config file defines the data directory, successful decoupling of config files would make it easy to also decouple the data directories so that if the user installs/removes/upgrades any of the MySQL variants they would not mess up their data accidentally and the installation scripts would help users to dump/migrate/copy/move their data in the common scenarios. -- Check out our blog at http://seravo.fi/blog and follow @ottokekalainen

2 1

[Maria-developers] CREATE USER gotcha
by Alexander Barkov 19 Dec '14

19 Dec '14

Hi Ian, I think there is a CREATE USER related gotcha which is not described in the manual. CREATE USER allows to create multiple users at the same time: CREATE USER user1, user2; A reader of https://mariadb.com/kb/en/mariadb/documentation/sql-commands/account-manage… might think that the above query is a short synonym for: CREATE USER user1; CREATE USER user2; But in fact it is not always true. The difference can happen during replication. A "CREATE USER" statement is binlogged as a single shot, but only if at least one user was really created. Suppose (on master) user1 already exists, while user2 does not. "CREATE USER user1, user2" will be binary logged as is, while two independent "CREATE USER user1" and "CREARE USER user2" statements will be logged only as "CREATE USER user2". The CREATE statement for user1 won't be logged, because it failed on master! Now, suppose non of the users exist on slave. In case of "CREATE USER user1, user2;" both will be created on slave. In case of two independent queries, only "user2" will be created on slave.

4 6

[Maria-developers] Syntax for parallel replication
by Kristian Nielsen 18 Dec '14

18 Dec '14

Hi Serg, Can you help me with suggestions/comments for the following proposal for how to do configuration for MDEV-6676, speculative parallel replication? I am not too happy about how things are in 10.0. There, there is a single option --slave-parallel-threads=N. If N>0, then parallel replication is enabled, else not. One problem is that this makes it not configurable per multi-source master connection. Another problem is that there are two possible mechanisms for parallelisation, group-commit based and domain_id based; currently, one can enable none of them or both, but not just one of them. MDEV-6676 will introduce at least one other mechanism, which seems to make it essential to make a better way to configure this. Now, ideally, there would not be any configuration at all. The server would just run things in parallel when possible and to the degree desirable. However, I think there are some reasons that we need to allow the user this configurability: - Parallel replication is still a somewhat experimental feature, so it seems too risky to enable it by default. Also, it doesn't really seem possible for the server to automatically set the best number of threads to use, with current implementation (or possibly any implementation). - When replicating with non-transactional updates, or in non-gtid mode, slave state is not crash safe. This is true in non-parallel replication also, but in parallel replication, the problem seems amplified, as there may be multiple transactions in progress at the time of a crash, complicating possible manual recovery. This also suggests that parallel replication must be configurable. - When using domain-based parallel replication, the user is responsible for ensuring that independent domains are non-conflicting and can be replicated out-of-order wrt. each other. So if replication domains are used, but this property is not guaranteed, then domain-based parallel replication need to be configurable, or parallel replication cannot be used at all. - The new speculative replication feature in MDEV-6676 is not always guaranteed to be a win - in some workloads, where there are many conflicts between successive transactions, excessive rollback could cause it to be less efficient than not using it. Again, this suggests it needs to be configurable. So given this, I came up with the following idea for syntax: CHANGE MASTER TO PARALLEL_MODE=(domain,groupcommit,transactional,waiting) Each of the four keywords in the parenthesis is optional. "domain" enables domain-based parallelisation, where each replication domain is treated independently. "groupcommit" enables the non-speculative mode, where only transactions that group-committed together on the master are applied in parallel on the slave. "transactional" enables the speculative mode, where all transactional DML is optimistically tried in parallel, and then in case of conflict a rollback and retry is done. "groupcommit" and "transactional" are mutually exclusive, at most one of them can be specified. The default would be (domain,groupcommit) to be back-wards compatible with 10.0. If slave_parallel_thread=0, then no parallel apply will happen even if PARALLEL_MODE is non-empty. If slave_parallel_thread>0 but PARALLEL_MODE is empty (PARALLEL_MODE=()), then again no parallel apply will be done. The "waiting" option is not essential to add, we could remove it, I put it in because there were already a number of options so it seemed to cause no harm. The idea is that on the master, we detect if transaction T2 had to do a row lock wait on transaction T1. If so, it seems likely that a similar conflict could occur on the slave, so we will not run T2 in parallel with T1; instead we will let T2 wait for T1 to commit before T2 is started. The "waiting" option could be enabled by a user to disable this check, enabling even more aggressive parallelisation. I am not sure if it is worth it to have this configurable though, comments welcome. I have not checked how hard it will be to implement the new syntax. We do not have any similar multi-option CHANGE MASTER elements, as far as I know, but it is similar to ENUM system variables. And we already have a IGNORE_SERVER_IDS syntax with comma-separated list within parenthesis. So hopefully not too hard to do, and somewhat consistent with existing syntax. So what do you think? Is it a reasonable syntax? Any comments, or suggestions for better way to do it? Thanks, - Kristian.

5 23

Re: [Maria-developers] Rev 4383: MDEV-6916:Upgrade from MySQL to MariaDB breaks already created views
by Sergei Golubchik 17 Dec '14

17 Dec '14

Hi, Sanja! Comments about the code, see below. Comments about the whole approach - see my other email. On Dec 07, sanja(a)askmonty.org wrote: > At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6916/ > > ------------------------------------------------------------ > revno: 4383 > revision-id: sanja(a)askmonty.org-20141207094825-tx163unk3mm4gkql > parent: sanja(a)askmonty.org-20141203234414-63zkpeq9eb0n9lc1 > committer: sanja(a)askmonty.org > branch nick: work-maria-5.5-MDEV-6916 > timestamp: Sun 2014-12-07 10:48:25 +0100 > message: > MDEV-6916:Upgrade from MySQL to MariaDB breaks already created views > (SQL-level commands to support view update) > === modified file 'mysql-test/r/view.result' > --- a/mysql-test/r/view.result 2014-12-03 22:37:59 +0000 > +++ b/mysql-test/r/view.result 2014-12-07 09:48:25 +0000 > @@ -5398,6 +5398,29 @@ DROP VIEW v1; > DROP TABLE t1, t2; > create view v1 as select 1; > drop view v1; > +# > +#MDEV-6916:Upgrade from MySQL to MariaDB breaks already created views > +# (SQL command for upgrade check) > +# > +create table t1 (a int); > +create algorithm=temptable view v1 as select a from t1; > +show create view v1; > +View Create View character_set_client collation_connection > +v1 CREATE ALGORITHM=TEMPTABLE DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `t1`.`a` AS `a` from `t1` latin1 latin1_swedish_ci > +check table v1 force mariadb upgrade; > +Table Op Msg_type Msg_text > +test.v1 check note View algorithm swap > +show create view v1; > +View Create View character_set_client collation_connection > +v1 CREATE ALGORITHM=MERGE DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `t1`.`a` AS `a` from `t1` latin1 latin1_swedish_ci No, I don't like that. FORCE or not, upgrading of a view should not blindly change the alorithm. This functionality is created to solve a specific problem. So when a view has mariadb-version field we know with certainty that the algorithm is correct and it should *not* be changed with or without FORCE. In the test you'll need to copy mysql-created view frm (from std_data). Addendum: ok, I've read till the end. See my comment in view_check() function. > +check table v1 force mariadb upgrade; Also, I don't like that CHECK changes the algorithm of the view. For tables CHECK only performs the *check* (and updates the version in the frm). It's REPAIR that does the change. To be consistent with the behavior for tables, CHECK ... FOR UPGRADE should only report an error, like "view needs upgrading" and a user should then use REPAIR to actually fix the view. > +Table Op Msg_type Msg_text > +test.v1 check note View algorithm swap > +show create view v1; > +View Create View character_set_client collation_connection > +v1 CREATE ALGORITHM=TEMPTABLE DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `t1`.`a` AS `a` from `t1` latin1 latin1_swedish_ci > +drop view v1; > +drop table t1; > # ----------------------------------------------------------------- > # -- End of 5.5 tests. > # ----------------------------------------------------------------- > > === modified file 'sql/share/errmsg-utf8.txt' > --- a/sql/share/errmsg-utf8.txt 2014-05-27 06:45:01 +0000 > +++ b/sql/share/errmsg-utf8.txt 2014-12-07 09:48:25 +0000 > @@ -6565,3 +6565,5 @@ ER_QUERY_EXCEEDED_ROWS_EXAMINED_LIMIT > ER_NO_SUCH_TABLE_IN_ENGINE 42S02 > eng "Table '%-.192s.%-.192s' doesn't exist in engine" > swe "Det finns ingen tabell som heter '%-.192s.%-.192s' i handlern" > +ER_VIEW_UPGRADE_IS_DONE > + eng "View algorithm swap" No way, you cannot add a new error message in 5.5 because the next major branch (10.0) is already GA. > === modified file 'sql/sql_admin.cc' > --- a/sql/sql_admin.cc 2014-05-03 16:12:17 +0000 > +++ b/sql/sql_admin.cc 2014-12-07 09:48:25 +0000 > @@ -314,7 +314,8 @@ static bool mysql_admin_table(THD* thd, > HA_CHECK_OPT *), > int (handler::*operator_func)(THD *, > HA_CHECK_OPT *), > - int (view_operator_func)(THD *, TABLE_LIST*)) > + int (view_operator_func)(THD *, TABLE_LIST*, > + HA_CHECK_OPT *)) > { > TABLE_LIST *table; > SELECT_LEX *select= &thd->lex->select_lex; > @@ -521,7 +522,7 @@ static bool mysql_admin_table(THD* thd, > ER_CHECK_NO_SUCH_TABLE, ER(ER_CHECK_NO_SUCH_TABLE)); > /* if it was a view will check md5 sum */ > if (table->view && > - view_checksum(thd, table) == HA_ADMIN_WRONG_CHECKSUM) > + view_check(thd, table, check_opt) == HA_ADMIN_WRONG_CHECKSUM) > push_warning(thd, MYSQL_ERROR::WARN_LEVEL_WARN, > ER_VIEW_CHECKSUM, ER(ER_VIEW_CHECKSUM)); > if (thd->stmt_da->is_error() && > @@ -536,7 +537,7 @@ static bool mysql_admin_table(THD* thd, > if (table->view) > { > DBUG_PRINT("admin", ("calling view_operator_func")); > - result_code= (*view_operator_func)(thd, table); > + result_code= (*view_operator_func)(thd, table, check_opt); > goto send_result; > } > > @@ -849,6 +850,14 @@ send_result_message: > system_charset_info); > break; > } > + case HA_ADMIN_VIEW_UPGRADE_IS_DONE: > + { > + protocol->store(STRING_WITH_LEN("note"), system_charset_info); > + protocol->store(ER(ER_VIEW_UPGRADE_IS_DONE), > + strlen(ER(ER_VIEW_UPGRADE_IS_DONE)), > + system_charset_info); > + break; > + } Don't do that. First, you cannot have ER_VIEW_UPGRADE_IS_DONE. Second, it's simply unnecessary. Return HA_ADMIN_OK from the view_operator_func() and use push_warning inside view_operator_func() to say that the algorithm was changed. > case HA_ADMIN_NEEDS_UPGRADE: > case HA_ADMIN_NEEDS_ALTER: > @@ -1071,7 +1080,7 @@ bool Check_table_statement::execute(THD > > res= mysql_admin_table(thd, first_table, &m_lex->check_opt, "check", > lock_type, 0, 0, HA_OPEN_FOR_REPAIR, 0, > - &handler::ha_check, &view_checksum); > + &handler::ha_check, &view_check); > > m_lex->select_lex.table_list.first= first_table; > m_lex->query_tables= first_table; > > === modified file 'sql/sql_view.cc' > --- a/sql/sql_view.cc 2014-12-03 23:44:14 +0000 > +++ b/sql/sql_view.cc 2014-12-07 09:48:25 +0000 > @@ -729,9 +729,29 @@ err: > DBUG_RETURN(res || thd->is_error()); > } > > +static void make_view_filename(LEX_STRING *dir, char *dir_buff, > + size_t dir_buff_len, > + LEX_STRING *path, char *path_buff, > + size_t path_buff_len, > + LEX_STRING *file, > + TABLE_LIST *view) > +{ > + /* print file name */ > + dir->length= build_table_filename(dir_buff, dir_buff_len - 1, > + view->db, "", "", 0); > + dir->str= dir_buff; > + > + path->length= build_table_filename(path_buff, path_buff_len - 1, > + view->db, view->table_name, reg_ext, 0); > + path->str= path_buff; > + > + file->str= path->str + dir->length; > + file->length= path->length - dir->length; > +} > + > > /* number of required parameters for making view */ > -static const int required_view_parameters= 15; > +static const int required_view_parameters= 16; eh? why? > /* > table of VIEW .frm field descriptors > @@ -795,6 +815,65 @@ static File_option view_parameters[]= > static LEX_STRING view_file_type[]= {{(char*) STRING_WITH_LEN("VIEW") }}; > > > +int mariadb_fix_view(THD *thd, TABLE_LIST *view) > +{ > + char dir_buff[FN_REFLEN + 1], path_buff[FN_REFLEN + 1]; > + LEX_STRING dir, file, path; > + DBUG_ENTER("mariadb_fix_view"); > + > + if (view->algorithm == VIEW_ALGORITHM_UNDEFINED) > + DBUG_RETURN(HA_ADMIN_OK); > + > + make_view_filename(&dir, dir_buff, sizeof(dir_buff), > + &path, path_buff, sizeof(path_buff), > + &file, view); > + /* init timestamp */ > + if (!view->timestamp.str) > + view->timestamp.str= view->timestamp_buffer; > + > + /* check old .frm */ > + { > + char path_buff[FN_REFLEN]; > + LEX_STRING path; > + File_parser *parser; > + > + path.str= path_buff; > + fn_format(path_buff, file.str, dir.str, "", MY_UNPACK_FILENAME); > + path.length= strlen(path_buff); > + if (access(path.str, F_OK)) > + DBUG_RETURN(HA_ADMIN_INVALID); > + > + if (!(parser= sql_parse_prepare(&path, thd->mem_root, 0))) > + DBUG_RETURN(HA_ADMIN_INTERNAL_ERROR); > + > + if (!parser->ok() || !is_equal(&view_type, parser->type())) > + DBUG_RETURN(HA_ADMIN_INVALID); > + } > + > + DBUG_ASSERT(view->algorithm == VIEW_ALGORITHM_MERGE || > + view->algorithm == VIEW_ALGORITHM_TMPTABLE); if() instead of an assert, please. while unlikely, it's possible that the file would be changed between two opens. > + if (view->algorithm == VIEW_ALGORITHM_MERGE) > + view->algorithm= VIEW_ALGORITHM_TMPTABLE; > + else > + view->algorithm= VIEW_ALGORITHM_MERGE; > + > + if (sql_create_definition_file(&dir, &file, view_file_type, > + (uchar*)view, view_parameters)) > + { > + sql_print_error("View '%-.192s'.'%-.192s': algorithm swap error.", "View %`s.%`s: cannot change the algorithm from %s to %s" And push_warning_printf() with the same. Or, rather, only push_warning_printf without sql_print_information. > + view->db, view->table_name); > + DBUG_RETURN(HA_ADMIN_INTERNAL_ERROR); > + } > + sql_print_information("View '%-.192s'.'%-.192s': algorithm swapped to '%s'", "View %`s.%`s: algorithm changed from %s to %s" And push_warning_printf() with the same. Or, rather, only push_warning_printf without sql_print_information. > + view->db, view->table_name, > + (view->algorithm == VIEW_ALGORITHM_MERGE)? > + "MERGE":"TEMPTABLE"); > + > + > + DBUG_RETURN(HA_ADMIN_VIEW_UPGRADE_IS_DONE); > +} > + > + > /* > Register VIEW (write .frm & process .frm's history backups) > > @@ -931,16 +1010,9 @@ static int mysql_register_view(THD *thd, > } > loop_out: > /* print file name */ > - dir.length= build_table_filename(dir_buff, sizeof(dir_buff) - 1, > - view->db, "", "", 0); > - dir.str= dir_buff; > - > - path.length= build_table_filename(path_buff, sizeof(path_buff) - 1, > - view->db, view->table_name, reg_ext, 0); > - path.str= path_buff; > - > - file.str= path.str + dir.length; > - file.length= path.length - dir.length; > + make_view_filename(&dir, dir_buff, sizeof(dir_buff), > + &path, path_buff, sizeof(path_buff), > + &file, view); > > /* init timestamp */ > if (!view->timestamp.str) > @@ -1953,6 +2025,33 @@ int view_checksum(THD *thd, TABLE_LIST * > HA_ADMIN_OK); > } you can make view_checksum() static now. > > +/** > + Check view > + > + @param thd thread handle > + @param view view for check > + @param check_opt check options > + > + @retval HA_ADMIN_OK OK > + @retval HA_ADMIN_NOT_IMPLEMENTED it is not VIEW > + @retval HA_ADMIN_WRONG_CHECKSUM check sum is wrong > +*/ > +int view_check(THD *thd, TABLE_LIST *view, HA_CHECK_OPT *check_opt) > +{ > + int res; > + DBUG_ENTER("view_check"); > + if ((res= view_checksum(thd, view)) != HA_ADMIN_OK) > + DBUG_RETURN(res); > + if (((check_opt->sql_flags & TT_MARIADB_UPGRADE) && > + !view->mariadb_version) || > + ((check_opt->sql_flags & (TT_MARIADB_UPGRADE | TT_FORCE)))) What is the point? Why would one ever want to change the algorithm of the provably created-in-mariadb view and pretend that it's an "upgrade"? To change the algorithm one should use ALTER. Fixing the view after an upgrade is not altering it, it's *fixing after an upgrade*. And views that were created in mariadb don't need to be fixed after an upgrade. No FORCE please. > + { > + DBUG_PRINT("XXX", ("Upgrade")); please either remove this DBUG_PRINT or change it to use a reasonable keyword. > + DBUG_RETURN(mariadb_fix_view(thd, view)); > + } > + DBUG_RETURN(HA_ADMIN_OK); > +} > + > === modified file 'sql/sql_yacc.yy' > --- a/sql/sql_yacc.yy 2014-11-08 18:54:42 +0000 > +++ b/sql/sql_yacc.yy 2014-12-07 09:48:25 +0000 > @@ -7299,6 +7300,10 @@ mi_check_type: > | EXTENDED_SYM { Lex->check_opt.flags|= T_EXTEND; } > | CHANGED { Lex->check_opt.flags|= T_CHECK_ONLY_CHANGED; } > | FOR_SYM UPGRADE_SYM { Lex->check_opt.sql_flags|= TT_FOR_UPGRADE; } > + | MARIADB_SYM UPGRADE_SYM > + { Lex->check_opt.sql_flags|= TT_MARIADB_UPGRADE; } > + | FORCE_SYM MARIADB_SYM UPGRADE_SYM > + { Lex->check_opt.sql_flags|= (TT_MARIADB_UPGRADE | TT_FORCE); } > ; > > optimize: Regards, Sergei

1 0

Re: [Maria-developers] Rev 4384: MDEV-6916: Upgrade from MySQL to MariaDB breaks already created views
by Sergei Golubchik 17 Dec '14

17 Dec '14

Hi, Sanja! Ok, I've seen all four patches. Specific comments about the code are below and in other emails. But the main question is - why are you doing it, what problem does it solve. As you said, the use case is - a user upgrades *from MySQL* and *runs mysql_upgrade*. To cover this use case mysql_upgrade needs to 1) detect whether it's an upgrade from MySQL, and to 2) run a command to change the algorithm of all views. You've done a lot of changes that aren't related to that. If this is, indeed, the use case, than you only need to: 1. add MySQL detection in mysql_upgrade. You have it, is_mysql() function. 2. add the code to toggle the algorithm of all views. See? No command line options, no FORCE, no CHECK, nothing. Now, let's consider extensions to this minimal functionality. 3. mariadb-version in the view frm file. this is a good idea, makes views a lot more future-proof. ok. I don't see why other extensions might be needed. Now, how to toggle the algorithm. The most natural solution would be to use ALTER VIEW. Like ALTER VIEW v1 ALGORITHM=MERGE. Unfortunately, ALTER VIEW doesn't work that way, I don't know why it was implemented in that crazy way in MySQL, but it's mostly useless. CHECK should not change its arguments. REPAIR? May be. I'd still prefer ALTER, but I don't know whether it can be extended in 5.5 in a safe way. If not then REPAIR ... AFTER UPGRADE is probably the second best. Or even REPAIR VIEW ... AFTER UPGRADE. It won't conflict with anything, doesn't add new keywords. Drawback - we'll be stuck with that hack for quite a while :( Even better would be to make mysql_upgrade to scan the data directory and rewrite view frms. No hacks or new grammar in the server. No new features in 5.5 GA. Nothing to maintain for years. Almost perfect. But it requires mysql_upgrade to have write access to the datadir. It kind of needs it already, but it's optional. With this approach it'll be less optional than before. So, I don't know... The most reasonable and future-proof solution would be to use ALTER TABLE. But I don't know whether we'll be able to do it in 5.5, looks a bit risky. Then there are reasonably safe hacks like REPAIR VIEW. And most safe and quite clean mysql_upgrade only approach, that needs write access to the datadir. Opinions? Anyway, here are comments about the code, below: On Dec 11, sanja(a)askmonty.org wrote: > At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6916/ > > ------------------------------------------------------------ > revno: 4384 > revision-id: sanja(a)askmonty.org-20141211180458-j8ajlibnvvydmj40 > parent: sanja(a)askmonty.org-20141207094825-tx163unk3mm4gkql > committer: sanja(a)askmonty.org > branch nick: work-maria-5.5-MDEV-6916 > timestamp: Thu 2014-12-11 19:04:58 +0100 > message: > MDEV-6916: Upgrade from MySQL to MariaDB breaks already created views > > mysql_upgrade now fix views from mysql. > > (+ fix new view detection) > === modified file 'client/mysql_upgrade.c' > --- a/client/mysql_upgrade.c 2014-03-27 21:26:58 +0000 > +++ b/client/mysql_upgrade.c 2014-12-11 18:04:58 +0000 > @@ -40,7 +40,8 @@ static char mysql_path[FN_REFLEN]; > static char mysqlcheck_path[FN_REFLEN]; > > static my_bool opt_force, opt_verbose, debug_info_flag, debug_check_flag, > - opt_systables_only, opt_version_check; > + opt_systables_only, opt_version_check, > + opt_mysql_upgrade= 0, opt_skip_mysql_upgrade= 0; > static my_bool opt_not_used, opt_silent; > static uint my_end_arg= 0; > static char *opt_user= (char*)"root"; > @@ -150,6 +151,14 @@ static struct my_option my_long_options[ > &opt_not_used, &opt_not_used, 0, GET_BOOL, NO_ARG, 1, 0, 0, 0, 0, 0}, > {"version", 'V', "Output version information and exit.", 0, 0, 0, > GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, > + {"mysql-upgrade", 'y', > + "Skip automatic detection MySQL and assume that we upgrade it", > + &opt_mysql_upgrade, &opt_mysql_upgrade, 0, GET_BOOL, NO_ARG, > + 0, 0, 0, 0, 0, 0}, > + {"skip-mysql-upgrade", 'Y', > + "Skip view algorithm upgrade from MySQL", > + &opt_skip_mysql_upgrade, &opt_skip_mysql_upgrade, 0, GET_BOOL, NO_ARG, > + 0, 0, 0, 0, 0, 0}, Those two are very bad option names. First, $ mysql_upgrade --mysql-upgrade looks plain weird. $ mysql_upgrade --skip-mysql-upgrade is even worse. And what about $ mysql_upgrade --mysql-upgrade --skip-mysql-upgrade Second, "skip" is the prefix automatically recognized by my_getopt. Normally, --skip-xxx is the same as --xxx=0. But now you're introducing --skip-mysql-upgrade which is very different from --skip-mysql-upgrade. While simultaneously adding support for --skip-skip-mysql-upgrade. A better option would be --fix-view-algorithm = { AUTO | ALWAYS | NEVER } > {"version-check", 'k', "Run this program only if its \'server version\' " > "matches the version of the server to which it's connecting, (enabled by " > "default); use --skip-version-check to avoid this check. Note: the \'server " > @@ -754,6 +771,23 @@ static int run_mysqlcheck_upgrade(void) > NULL); > } > > +static int run_mysqlcheck_views(void) > +{ > + if (!opt_mysql_upgrade) > + return 0; > + verbose("Phase 0: Fixing views"); No "Phase 0" please, renumber all phases to start from 1. > + print_conn_args("mysqlcheck"); > + return run_tool(mysqlcheck_path, > + NULL, /* Send output from mysqlcheck directly to screen */ > + "--no-defaults", > + ds_args.str, > + "--all-databases", > + "--view-upgrade", > + opt_verbose ? "--verbose": "", > + opt_silent ? "--silent": "", > + "2>&1", > + NULL); > +} > > === modified file 'client/mysqlcheck.c' > --- a/client/mysqlcheck.c 2014-02-17 10:00:51 +0000 > +++ b/client/mysqlcheck.c 2014-12-11 18:04:58 +0000 > @@ -42,7 +42,8 @@ static my_bool opt_alldbs = 0, opt_check > opt_medium_check = 0, opt_quick = 0, opt_all_in_1 = 0, > opt_silent = 0, opt_auto_repair = 0, ignore_errors = 0, > tty_password= 0, opt_frm= 0, debug_info_flag= 0, debug_check_flag= 0, > - opt_fix_table_names= 0, opt_fix_db_names= 0, opt_upgrade= 0; > + opt_fix_table_names= 0, opt_fix_db_names= 0, opt_upgrade= 0, > + opt_mariadb_upgrade= 0, opt_f_mariadb_upgrade= 0; > static my_bool opt_write_binlog= 1, opt_flush_tables= 0; > static uint verbose = 0, opt_mysql_port=0; > static int my_end_arg; > @@ -196,6 +197,12 @@ static struct my_option my_long_options[ > NO_ARG, 0, 0, 0, 0, 0, 0}, > {"version", 'V', "Output version information and exit.", 0, 0, 0, GET_NO_ARG, > NO_ARG, 0, 0, 0, 0, 0, 0}, > + {"view-upgrade", 'f', > + "Fix view algorithm view field if it is not new MariaDB view.", "Repair algorithm value for views created by MySQL (or MariaDB before 5.5.41)" > + 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, > + {"force-view-upgrade", 'F', > + "Swap view algorithm view field.", "Always change view algorithm" > + 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}, > {0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0} > }; Regards, Sergei

1 0

[Maria-developers] Invisible Indexes and .frm Change?
by James Briggs 16 Dec '14

16 Dec '14

Hi list. I'm looking into adding invisible indexes. There's 3 "scopes" I've thought of: 1) global and persistent (create table, alter table, create index) 2) global temporary (alter table t1 change index invisible global) 3) session temporary (alter table t1 change index invisible session) #1 likely means changing the .frm format, which would cause backwardcompatibility issues. Are there any upcoming changes to .frm planned? Any comments or suggestions? Thanks, James Briggs. -- Cassandra/MySQL DBA. Available in San Jose area or remote. cass_top: https://github.com/jamesbriggs/cassandra-top

2 1

Re: [Maria-developers] Rev 4350: MDEV-6985: MariaDB crashes on stored procedure call
by Sergei Golubchik 16 Dec '14

16 Dec '14

Hi, Sanja! On Nov 17, sanja(a)askmonty.org wrote: > ------------------------------------------------------------ > revno: 4350 > revision-id: sanja(a)askmonty.org-20141117132635-gf281ipiomifw1oc > parent: psergey(a)askmonty.org-20141113111559-cuhsyfhor141qo6a > committer: sanja(a)askmonty.org > branch nick: work-maria-5.5-MDEV-6985 > timestamp: Mon 2014-11-17 14:26:35 +0100 > message: > MDEV-6985: MariaDB crashes on stored procedure call > > Item_ident fixed to allow double cleanup(). > === modified file 'sql/item.cc' > --- a/sql/item.cc 2014-10-06 17:53:55 +0000 > +++ b/sql/item.cc 2014-11-17 13:26:35 +0000 > @@ -862,12 +862,20 @@ void Item_ident::cleanup() > field_name ? field_name : "(null)", > orig_field_name ? orig_field_name : "(null)")); > #endif Before pushing, please remove the above #ifdef, in a separate commit. thanks! > + bool was_fixed= fixed; > Item::cleanup(); > db_name= orig_db_name; > table_name= orig_table_name; > field_name= orig_field_name; > /* Store if this Item was depended */ > - can_be_depended= test(depended_from); > + if (was_fixed) > + { > + /* > + We can trust that depended_from set correctly only if this item > + was fixed > + */ > + can_be_depended= test(depended_from); > + } Are double cleanups generally normal? Why this item is cleaned up twice? > DBUG_VOID_RETURN; > } Regards, Sergei

2 2

Re: [Maria-developers] Rev 4382: Added creation time to the view frm
by Sergei Golubchik 16 Dec '14

16 Dec '14

Hi, Sanja! On Dec 03, sanja(a)askmonty.org wrote: > At file:///home/bell/maria/bzr/work-maria-5.5-MDEV-6916/ > > ------------------------------------------------------------ > revno: 4382 > revision-id: sanja(a)askmonty.org-20141203234414-63zkpeq9eb0n9lc1 > parent: sanja(a)askmonty.org-20141203223759-316k8y35qdb4cgyq > committer: sanja(a)askmonty.org > branch nick: work-maria-5.5-MDEV-6916 > timestamp: Thu 2014-12-04 00:44:14 +0100 > message: > Added creation time to the view frm. > ALTER VIEW saves it. CREATE VIEW & CREATE OR REPLACE VIEW reset it. Please no. This has nothing to do with MDEV-6916, it's a separate task and should be reviewed and discussed separately, not sneaked in under the MDEV-6916 cover. Regards, Sergei

1 0