- developers - lists.mariadb.org

[Maria-developers] Non-bloking API problem on non-x86 (MDEV-9112)
by Kristian Nielsen 01 Feb '16

01 Feb '16

Hi Georg, I stumbled upon the problem in the server source tree that ucontext check is broken, causing the non-blocking library to be completely disabled on all non-x86 platforms. After creating my own patch, I noticed that you alrady made a patch for this in MDEV-9112, but it does not appear to be pushed yet. Do you still want to push your patch? The patch looks fine (you can take this as review), it handles a couple things I forgot in my own patch. I think this should definitely go in 10.0 (and 10.1) - but do you think it should go in 5.5 also (not sure)? - Kristian. PS. I supplied a patch in CONC-153 for the similar problem in mariadb-connector-c, but maybe you are already aware of that as well. It was BTW cool to see that an application of mine (originally written against libmysqlclient) worked without changes on mariadb-connector-c.

1 1

Re: [Maria-developers] [Commits] 2a181ea: MDEV-8713 Add continuous binary log backup to mysqlbinlog.
by Sergei Golubchik 31 Jan '16

31 Jan '16

Hi, Alexey! On Nov 27, Alexey Botchkov wrote: > revision-id: 2a181ea7b16dcbc6dd30af5a2396760261d7d500 (mariadb-10.1.8-81-g2a181ea) > parent(s): 9d5c9379a6fd3662d4388cda177a3b1ea7b070cc > committer: Alexey Botchkov > timestamp: 2015-11-27 16:33:23 +0400 > message: > > MDEV-8713 Add continuous binary log backup to mysqlbinlog. > diff --git a/client/client_priv.h b/client/client_priv.h > index 656c8fc..44f0b21 100644 > --- a/client/client_priv.h > +++ b/client/client_priv.h > @@ -92,6 +92,7 @@ enum options_client > OPT_REPORT_PROGRESS, > OPT_SKIP_ANNOTATE_ROWS_EVENTS, > OPT_SSL_CRL, OPT_SSL_CRLPATH, > + OPT_RAW_OUTPUT, OPT_WAIT_SERVER_ID, OPT_STOP_NEVER, 1. There's no need to create named constants for OPT_RAW_OUTPUT and OPT_WAIT_SERVER_ID - they are never used anywhere. Simply put 0 in the my_options structure instead. 2. OPT_STOP_NEVER is used in the switch, so it needs a named constant. but it still doesn't have to be in client_priv.h, it's not an option common to all clients. Define it in mysqlbinlog.cc (see how mysqltest.cc and mysql_upgrade.c do it) > OPT_MAX_CLIENT_OPTION /* should be always the last */ > }; > > diff --git a/client/mysqlbinlog.cc b/client/mysqlbinlog.cc > index 5b07232..a2509b7 100644 > --- a/client/mysqlbinlog.cc > +++ b/client/mysqlbinlog.cc > @@ -96,6 +98,8 @@ static char* database= 0; > static my_bool force_opt= 0, short_form= 0, remote_opt= 0; > static my_bool debug_info_flag, debug_check_flag; > static my_bool force_if_open_opt= 1; > +static my_bool opt_raw_mode= 0, opt_stop_never= 0; > +static int64 opt_stop_never_slave_server_id= -1; longlong, not int64. Because 1) you've used GET_LL for it, and 2) we generally don't use int64 (it's only used in special cases to mean exactly "signed 64-bit integer"). > static my_bool opt_verify_binlog_checksum= 1; > static ulonglong offset = 0; > static char* host = 0; > @@ -1368,8 +1373,14 @@ static struct my_option my_options[] = > {"read-from-remote-server", 'R', "Read binary logs from a MySQL server.", > &remote_opt, &remote_opt, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, > 0, 0}, > - {"result-file", 'r', "Direct output to a given file.", 0, 0, 0, GET_STR, > - REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, > + {"raw", OPT_RAW_OUTPUT, "Requires -R. Output raw binlog data instead of SQL " > + "statements and write it to files corresponding to server logs.", What does that mean? "write it to files corresponding to server logs"? Perhaps you could clarify the help text? > + &opt_raw_mode, &opt_raw_mode, 0, GET_BOOL, NO_ARG, 0, 0, 0, 0, > + 0, 0}, > + {"result-file", 'r', "Direct output to a given file. With --raw this is a " > + "prefix for the file names.", > + &result_file_name, &result_file_name, 0, GET_STR, REQUIRED_ARG, > + 0, 0, 0, 0, 0, 0}, > {"server-id", 0, > "Extract only binlog entries created by the server having the given id.", > &server_id, &server_id, 0, GET_ULONG, > @@ -1418,6 +1429,16 @@ static struct my_option my_options[] = > "(you should probably use quotes for your shell to set it properly).", > &stop_datetime_str, &stop_datetime_str, > 0, GET_STR_ALLOC, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, > + {"stop-never", OPT_STOP_NEVER, "Wait for more data from the server " > + "instead of stopping at the end of the last log. Implicitly sets " > + "--to-last-log but instead of stopping at the end of the last log " > + "it continues to wait till the server disconnects.", I don't know why you needed to say the same thing twice. What about: Wait for more data from the server instead of stopping at the end of the last log. Implies --to-last-log. > + &opt_stop_never, &opt_stop_never, 0, > + GET_BOOL, NO_ARG, 0, 0, 0, 0, 0, 0}, > + {"stop-never-slave-server-id", OPT_WAIT_SERVER_ID, > + "The slave server_id used for --read-from-remote-server --stop-never.", > + &opt_stop_never_slave_server_id, &opt_stop_never_slave_server_id, 0, > + GET_LL, REQUIRED_ARG, -1, -1, 0xFFFFFFFFLL, 0, 0, 0}, GET_LL, really? So, negative server_id's are allowed? Either do it like the existing --server-id option (GET_ULONG, 0 means "not set"), or, if you want to allow 0 as a valid server-id value, you can do, like case OPT_WAIT_SERVER_ID: if (argument == disabled_my_option) opt_stop_never_slave_server_id= -1; break; but really I wouldn't bother and just do like --server-id does. > {"stop-position", OPT_STOP_POSITION, > "Stop reading the binlog at position N. Applies to the last binlog " > "passed on the command line.", > @@ -1701,6 +1718,9 @@ get_one_option(int optid, const struct my_option *opt __attribute__((unused)), > print_version(); > opt_version= 1; > break; > + case OPT_STOP_NEVER: > + to_last_remote_log= 1; > + break; This is wrong, it allows someone to do mysqlbinlog --stop-never --to-last-remote-log=0 and you get incorrect configuration. Alternatively one can do mysqlbinlog --stop-never=1 --stop-never=0 and to_last_remote_log will be magically enabled. Heck, one can even do mysqlbinlog --stop-never=0 and that'll enable to_last_remote_log. The correct approach is to do if (opt_stop_never) to_last_remote_log= TRUE; and only *after* all options are read. Note that in this case you won't need OPT_STOP_NEVER constant either (see my comment earlier in this review) > case '?': > usage(); > opt_version= 1; > @@ -1922,6 +1943,240 @@ static Exit_status check_master_version() > } > > + > + > +static char out_file_name[FN_REFLEN + 1]; > + > +static Exit_status handle_event_raw_mode(PRINT_EVENT_INFO *print_event_info, > + ulong *len, > + const char* logname, uint logname_len) > +{ I'm not reviewing the logic of handle_event_raw_mode, as far as I can see you've copied it from mysqlbinlog in MySQL. Did you use the latest version of mysqlbinlog or the one from the original patch? Use the latest one, it might have bugfixes. > + const char *error_msg; > + const unsigned char *read_pos= mysql->net.read_pos + 1; > + Log_event_type type; > + DBUG_ENTER("handle_event_raw_mode"); > + DBUG_ASSERT(opt_raw_mode && remote_opt); > + > + type= (Log_event_type) read_pos[EVENT_TYPE_OFFSET]; > + > + if (type == HEARTBEAT_LOG_EVENT) > + DBUG_RETURN(OK_CONTINUE); > + > + if (type == ROTATE_EVENT || type == FORMAT_DESCRIPTION_EVENT) > + { > + Log_event *ev; > + if (!(ev= Log_event::read_log_event((const char*) read_pos , > + *len - 1, &error_msg, > + glob_description_event, > + opt_verify_binlog_checksum))) > + { > + error("Could not construct annotate event object: %s", error_msg); may be: "%s", type == ROTATE_EVENT ? "rotate":"annotate" > + DBUG_RETURN(ERROR_STOP); > + } > + /* > + If reading from a remote host, ensure the temp_buf for the > + Log_event class is pointing to the incoming stream. > + */ > + ev->register_temp_buf((char *) read_pos, FALSE); > + > @@ -2000,113 +2264,23 @@ static Exit_status dump_remote_log_entries(PRINT_EVENT_INFO *print_event_info, > break; // end of data > DBUG_PRINT("info",( "len: %lu net->read_pos[5]: %d\n", > len, net->read_pos[5])); > - if (net->read_pos[5] == ANNOTATE_ROWS_EVENT) > + if (opt_raw_mode) > { > - if (!(ev= read_remote_annotate_event(net->read_pos + 1, len - 1, > - &error_msg))) > - { > - error("Could not construct annotate event object: %s", error_msg); > - DBUG_RETURN(ERROR_STOP); > - } > + retval= handle_event_raw_mode(print_event_info, &len, > + logname, logname_len); > } > else > { > - if (!(ev= Log_event::read_log_event((const char*) net->read_pos + 1 , > - len - 1, &error_msg, > - glob_description_event, > - opt_verify_binlog_checksum))) > - { > - error("Could not construct log event object: %s", error_msg); > - DBUG_RETURN(ERROR_STOP); > - } > - /* > - If reading from a remote host, ensure the temp_buf for the > - Log_event class is pointing to the incoming stream. > - */ > - ev->register_temp_buf((char *) net->read_pos + 1, FALSE); > + retval= handle_event_text_mode(print_event_info, &len, > + logname, logname_len, old_off); > } > - > - Log_event_type type= ev->get_type_code(); > - if (glob_description_event->binlog_version >= 3 || > - (type != LOAD_EVENT && type != CREATE_FILE_EVENT)) > + if (retval != OK_CONTINUE) > { > - /* > - If this is a Rotate event, maybe it's the end of the requested binlog; > - in this case we are done (stop transfer). > - This is suitable for binlogs, not relay logs (but for now we don't read > - relay logs remotely because the server is not able to do that). If one > - day we read relay logs remotely, then we will have a problem with the > - detection below: relay logs contain Rotate events which are about the > - binlogs, so which would trigger the end-detection below. > - */ > - if (type == ROTATE_EVENT) > - { > - Rotate_log_event *rev= (Rotate_log_event *)ev; > - /* > - If this is a fake Rotate event, and not about our log, we can stop > - transfer. If this a real Rotate event (so it's not about our log, > - it's in our log describing the next log), we print it (because it's > - part of our log) and then we will stop when we receive the fake one > - soon. > - */ > - if (rev->when == 0) > - { > - if (!to_last_remote_log) > - { > - if ((rev->ident_len != logname_len) || > - memcmp(rev->new_log_ident, logname, logname_len)) > - { > - DBUG_RETURN(OK_CONTINUE); > - } > - /* > - Otherwise, this is a fake Rotate for our log, at the very > - beginning for sure. Skip it, because it was not in the original > - log. If we are running with to_last_remote_log, we print it, > - because it serves as a useful marker between binlogs then. > - */ > - continue; > - } > - len= 1; // fake Rotate, so don't increment old_off > - } > - } > - else if (type == FORMAT_DESCRIPTION_EVENT) > - { > - /* > - This could be an fake Format_description_log_event that server > - (5.0+) automatically sends to a slave on connect, before sending > - a first event at the requested position. If this is the case, > - don't increment old_off. Real Format_description_log_event always > - starts from BIN_LOG_HEADER_SIZE position. > - */ > - if (old_off != BIN_LOG_HEADER_SIZE) > - len= 1; // fake event, don't increment old_off > - } > - Exit_status retval= process_event(print_event_info, ev, old_off, logname); > - if (retval != OK_CONTINUE) > - DBUG_RETURN(retval); > + if (retval == OK_EOF) > + break; > + DBUG_RETURN(retval); > } > - else > - { > - Load_log_event *le= (Load_log_event*)ev; > - const char *old_fname= le->fname; > - uint old_len= le->fname_len; > - File file; > - Exit_status retval; > > - if ((file= load_processor.prepare_new_file_for_old_format(le,fname)) < 0) > - DBUG_RETURN(ERROR_STOP); > - > - retval= process_event(print_event_info, ev, old_off, logname); > - if (retval != OK_CONTINUE) > - { > - my_close(file,MYF(MY_WME)); > - DBUG_RETURN(retval); > - } > - retval= load_processor.load_old_format_file(net,old_fname,old_len,file); > - my_close(file,MYF(MY_WME)); > - if (retval != OK_CONTINUE) > - DBUG_RETURN(retval); > - } Hmm, when you moved all that code above into handle_event_text_mode(), you seem to have changed the behavior for to_last_remote_log. Was (comments removed for brevity): if (!to_last_remote_log) { if ((rev->ident_len != logname_len) || memcmp(rev->new_log_ident, logname, logname_len)) { DBUG_RETURN(OK_CONTINUE); } continue; } Now instead of 'continue' you return OK_CONTINUE from handle_event_text_mode() and it does not skip the code below (after the comment Let's adjust), like the old 'continue' did. > /* > Let's adjust offset for remote log as for local log to produce > similar text and to have --stop-position to work identically. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Aliasing slave_parallel_workers/slave_parallel_threads
by Colin Charles 29 Jan '16

29 Jan '16

Hi Kristian, I am referencing an old thread from March 2014 here where you don’t have an objection, but maybe this has slipped: https://lists.launchpad.net/maria-developers/msg06923.html MySQL 5.7 has been released and I’m sure you realise that it uses the variable slave_parallel_workers (http://dev.mysql.com/doc/refman/5.7/en/replication-options-slave.html#sysva…) In MariaDB Server 10.1 we use slave_parallel_threads (https://mariadb.com/kb/en/mariadb/replication-and-binary-log-server-system-…) From a DBA standpoint, and also from people using both databases in testing/production, this is confusing and scripts now have to have unnecessary added logic for if MariaDB or else MySQL. It would be wise for us to alias this, so that its easier from a DBA usability standpoint IMHO I planned to open up a Jira request, but since this was discussed on the list before I thought I would ask again to revive an old thread. cheers, -colin -- Colin Charles, http://bytebot.net/blog/ twitter: @bytebot | skype: colincharles "First they ignore you, then they laugh at you, then they fight you, then you win." -- Mohandas Gandhi

4 6

Re: [Maria-developers] [Commits] 56ae997: [MDEV-7978] Added show create user implementation.
by Sergei Golubchik 29 Jan '16

29 Jan '16

Hi, Vicentiu! Below are my comments on the combined patch: > diff --git a/mysql-test/r/alter_user.result b/mysql-test/r/alter_user.result > new file mode 100644 > index 0000000..2acc249 > --- /dev/null > +++ b/mysql-test/r/alter_user.result > @@ -0,0 +1,79 @@ > +select * from mysql.user where user = 'root' and host = 'localhost'; > +Host User Password Select_priv Insert_priv Update_priv Delete_priv Create_priv Drop_priv Reload_priv Shutdown_priv Process_priv File_priv Grant_priv References_priv Index_priv Alter_priv Show_db_priv Super_priv Create_tmp_table_priv Lock_tables_priv Execute_priv Repl_slave_priv Repl_client_priv Create_view_priv Show_view_priv Create_routine_priv Alter_routine_priv Create_user_priv Event_priv Trigger_priv Create_tablespace_priv ssl_type ssl_cipher x509_issuer x509_subject max_questions max_updates max_connections max_user_connections plugin authentication_string password_expired is_role default_role max_statement_time > +localhost root Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y 0 0 0 0 N N 0.000000 > +# Test syntax > +# > +# These 2 selects should have no changes from the first one. > +alter user CURRENT_USER; I'm not sure this should be a valid statement. After all ALTER TABLE t1; is not valid. There must be something after the user name > +select * from mysql.user where user = 'root' and host = 'localhost'; > +Host User Password Select_priv Insert_priv Update_priv Delete_priv Create_priv Drop_priv Reload_priv Shutdown_priv Process_priv File_priv Grant_priv References_priv Index_priv Alter_priv Show_db_priv Super_priv Create_tmp_table_priv Lock_tables_priv Execute_priv Repl_slave_priv Repl_client_priv Create_view_priv Show_view_priv Create_routine_priv Alter_routine_priv Create_user_priv Event_priv Trigger_priv Create_tablespace_priv ssl_type ssl_cipher x509_issuer x509_subject max_questions max_updates max_connections max_user_connections plugin authentication_string password_expired is_role default_role max_statement_time > +localhost root Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y 0 0 0 0 N N 0.000000 > +alter user CURRENT_USER(); > +select * from mysql.user where user = 'root' and host = 'localhost'; > +Host User Password Select_priv Insert_priv Update_priv Delete_priv Create_priv Drop_priv Reload_priv Shutdown_priv Process_priv File_priv Grant_priv References_priv Index_priv Alter_priv Show_db_priv Super_priv Create_tmp_table_priv Lock_tables_priv Execute_priv Repl_slave_priv Repl_client_priv Create_view_priv Show_view_priv Create_routine_priv Alter_routine_priv Create_user_priv Event_priv Trigger_priv Create_tablespace_priv ssl_type ssl_cipher x509_issuer x509_subject max_questions max_updates max_connections max_user_connections plugin authentication_string password_expired is_role default_role max_statement_time > +localhost root Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y Y 0 0 0 0 N N 0.000000 > +create user foo; > +select * from mysql.user where user = 'foo'; > +Host User Password Select_priv Insert_priv Update_priv Delete_priv Create_priv Drop_priv Reload_priv Shutdown_priv Process_priv File_priv Grant_priv References_priv Index_priv Alter_priv Show_db_priv Super_priv Create_tmp_table_priv Lock_tables_priv Execute_priv Repl_slave_priv Repl_client_priv Create_view_priv Show_view_priv Create_routine_priv Alter_routine_priv Create_user_priv Event_priv Trigger_priv Create_tablespace_priv ssl_type ssl_cipher x509_issuer x509_subject max_questions max_updates max_connections max_user_connections plugin authentication_string password_expired is_role default_role max_statement_time > +% foo N N N N N N N N N N N N N N N N N N N N N N N N N N N N N 0 0 0 0 N N 0.000000 > diff --git a/mysql-test/t/alter_user.test b/mysql-test/t/alter_user.test > new file mode 100644 > index 0000000..3a3a7d7 > --- /dev/null > +++ b/mysql-test/t/alter_user.test > @@ -0,0 +1,71 @@ > +--source include/not_embedded.inc > +--enable_connect_log > + > + > +select * from mysql.user where user = 'root' and host = 'localhost'; > +--echo # Test syntax > +--echo # > +--echo # These 2 selects should have no changes from the first one. > +alter user CURRENT_USER; > +select * from mysql.user where user = 'root' and host = 'localhost'; > +alter user CURRENT_USER(); > +select * from mysql.user where user = 'root' and host = 'localhost'; > + > +create user foo; > +select * from mysql.user where user = 'foo'; > +alter user foo; > +select * from mysql.user where user = 'foo'; > + > +--echo # Test super privilege works correctly with a read only database. > +SET @start_read_only = @@global.read_only; > +SET GLOBAL read_only=1; > +grant create user on *.* to foo; > + > +--echo # Currently no super privileges. > +connect (a, localhost, foo); > +select @@global.read_only; > + > +--error ER_OPTION_PREVENTS_STATEMENT > +alter user foo; > + > +--echo # Grant super privilege to the user. > +connection default; > +grant super on *.* to foo; > + > +--echo # We now have super privilege. We should be able to run alter user. > +connect (b, localhost, foo); > +alter user foo; > + > +connection default; > +SET GLOBAL read_only = @start_read_only; > + > +--echo # Test inexistant user. > +--error ER_CANNOT_USER > +alter user boo; > +--echo #--warning ER_CANNOT_USER > +alter if exists user boo; > + > +--echo # Test SSL related altering. eh? I don't see any SSL related altering here > +alter user foo identified by 'something'; > +select * from mysql.user where user = 'foo'; > + > +alter user foo identified by 'something2'; > +select * from mysql.user where user = 'foo'; > + > +alter user foo identified by password '*88C89BE093D4ECF72D039F62EBB7477EA1FD4D63'; > +select * from mysql.user where user = 'foo'; > + > +alter user foo identified with 'somecoolplugin'; > +select * from mysql.user where user = 'foo'; > + > +alter user foo identified with 'somecoolplugin' using 'somecoolpassphrase'; > +select * from mysql.user where user = 'foo'; > + > +--echo # Test resource limits altering. > +alter user foo with MAX_QUERIES_PER_HOUR 10 > + MAX_UPDATES_PER_HOUR 20 > + MAX_CONNECTIONS_PER_HOUR 30 > + MAX_USER_CONNECTIONS 40; > +select * from mysql.user where user = 'foo'; > +drop user foo; > +--disable_connect_log > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 96985aa..9fcd505 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -4635,7 +4640,11 @@ mysql_execute_command(THD *thd) > break; > /* Conditionally writes to binlog */ > WSREP_TO_ISOLATION_BEGIN(WSREP_MYSQL_DB, NULL, NULL) > - if (!(res= mysql_rename_user(thd, lex->users_list))) > + if (lex->sql_command == SQLCOM_ALTER_USER) > + res= mysql_alter_user(thd, lex->users_list); I don't think ALTER USER needs same privileges as RENAME USER. A user should be able to ALTER herself. We have added a check_alter_user() helper exactly for that, remember? :) > + else > + res= mysql_rename_user(thd, lex->users_list); > + if (!res) > my_ok(thd); > break; > } > diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc > index a6c6859..1285165 100644 > --- a/sql/sql_acl.cc > +++ b/sql/sql_acl.cc > @@ -7710,6 +7798,78 @@ static bool print_grants_for_role(THD *thd, ACL_ROLE * role) > } > > > +bool mysql_show_create_user(THD *thd, LEX_USER *lex_user) > +{ > + const char *username = safe_str(lex_user->user.str); > + const char *hostname = safe_str(lex_user->host.str); > + Protocol *protocol= thd->protocol; > + bool error= false; > + ACL_USER *acl_user; > + DBUG_ENTER("mysql_show_create_user"); > + > + // Check if the command specifies a username or not. > + if (lex_user->user.str == current_user.str) > + { > + username= thd->security_ctx->priv_user; > + hostname= thd->security_ctx->priv_host; > + } > + > + char buff[1024]; //Show create user should not take more than 1024 bytes. > + String field_name(buff, sizeof(buff), system_charset_info); There's a helper for that: StringBuffer<1024> field_name; on the other hand... you don't use field_name anywhere. > + List<Item> field_list; > + strxmov(buff, "CREATE USER for ", username, "@", hostname, NullS); > + Item_string *field = new (thd->mem_root) Item_string_ascii(thd, "", 0); > + if (!field) > + { > + my_error(ER_OUTOFMEMORY, MYF(0)); > + DBUG_RETURN(true); > + } > + Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] 57bf586: MDEV-8615: Assertion `m_cpp_buf <= begin_ptr && begin_ptr <= m_cpp_buf + m_buf_length' failed in Lex_input_stream::body_utf8_start
by Sergei Golubchik 27 Jan '16

27 Jan '16

Hi, Sanja! On Jan 27, OleksandrByelkin wrote: > revision-id: 57bf58668877552ee43f4ed86689b8bbb3821c9c (mariadb-10.1.10-23-g57bf586) > parent(s): 825f51d1aab51d363dc07ec9fe0829af33063883 > committer: Oleksandr Byelkin > timestamp: 2016-01-27 00:38:36 +0100 > message: > > MDEV-8615: Assertion `m_cpp_buf <= begin_ptr && begin_ptr <= m_cpp_buf + m_buf_length' failed in Lex_input_stream::body_utf8_start > > Nothing should be done before any keyword recognized. Good! Just two comments: 1. May be it's better to rename s/pop_cs_label/pop_sp_label/ and s/pop_cs_empty_label/pop_sp_empty_label/ ? first, pop_sp_empty_label applies to all SPs, not only to compound statements, and second, pop_cs_label does not apply to compound statements at all :) 2. Your process_control_statement_label() makes no sense here. There is no need to do an if() - because in both branches you always know whether you have a label or not. Please split process_control_statement_label() in two different functions (or two yacc rules, whatever). > diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy > index 302c2fd..3c2d386 100644 > --- a/sql/sql_yacc.yy > +++ b/sql/sql_yacc.yy > @@ -248,6 +248,36 @@ static bool maybe_start_compound_statement(THD *thd) > return 0; > } > > +static bool process_control_statement_label(THD *thd, LEX_STRING label) > +{ > + if (label.str != NULL) > + { > + sp_pcontext *ctx= thd->lex->spcont; > + sp_label *lab= ctx->find_label(label); > + > + if (lab) > + { > + my_error(ER_SP_LABEL_REDEFINE, MYF(0), label.str); > + return 1; > + } > + else > + { > + lab= thd->lex->spcont->push_label(thd, label, > + thd->lex->sphead->instructions()); > + lab->type= sp_label::ITERATION; > + } > + } > + else > + { > + if (maybe_start_compound_statement(thd)) > + return 1; > + /* Unlabeled controls get an empty label. */ > + thd->lex->spcont->push_label(thd, empty_lex_str, > + thd->lex->sphead->instructions()); > + } > + return 0; > +} > + > /** > Helper action for a case expression statement (the expr in 'CASE expr'). > This helper is used for 'searched' cases only. > @@ -4389,6 +4374,84 @@ sp_control_content: > } > ; > > +pop_cs_label: > + sp_opt_label > + { > + sp_label *lab; > + Lex->sphead->backpatch(lab= Lex->spcont->pop_label()); > + if ($1.str) > + { > + if (my_strcasecmp(system_charset_info, $1.str, > + lab->name.str) != 0) > + { > + my_error(ER_SP_LABEL_MISMATCH, MYF(0), $1.str); > + MYSQL_YYABORT; > + } > + } > + } > + ; > + > +pop_cs_empty_label: > + { > + sp_label *lab; > + Lex->sphead->backpatch(lab= Lex->spcont->pop_label()); > + DBUG_ASSERT(lab->name.length == 0); > + } > + ; > + > +sp_labeled_control: > + label_ident ':' LOOP_SYM > + { > + if (process_control_statement_label(thd, $1)) > + MYSQL_YYABORT; > + } > + loop_body pop_cs_label > + { } > + | label_ident ':' WHILE_SYM > + { > + if (process_control_statement_label(thd, $1)) > + MYSQL_YYABORT; > + Lex->sphead->reset_lex(thd); > + } > + while_body pop_cs_label > + { } > + | label_ident ':' REPEAT_SYM > + { > + if (process_control_statement_label(thd, $1)) > + MYSQL_YYABORT; > + } > + repeat_body pop_cs_label > + { } > + ; > + > +sp_unlabeled_control: > + LOOP_SYM > + { > + if (process_control_statement_label(thd, null_lex_str)) > + MYSQL_YYABORT; > + } > + loop_body > + pop_cs_empty_label > + { } > + | WHILE_SYM > + { > + if (process_control_statement_label(thd, null_lex_str)) > + MYSQL_YYABORT; > + Lex->sphead->reset_lex(thd); > + } > + while_body > + pop_cs_empty_label > + { } > + | REPEAT_SYM > + { > + if (process_control_statement_label(thd, null_lex_str)) > + MYSQL_YYABORT; > + } > + repeat_body > + pop_cs_empty_label > + { } > + ; > + > trg_action_time: > BEFORE_SYM > { Lex->trg_chistics.action_time= TRG_ACTION_BEFORE; } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] MDEV-5513 merge audit api changes from 5.5.34.
by Alexey Botchkov 27 Jan '16

27 Jan '16

Hello Sergei, all. I didn't quite understand what exact changes in 5.5.34 are important to us, so in order to begin i'm going to mention shortly what is new in MySQL 5.7 audit plugin API. 1. New event classes added: MYSQL_AUDIT_PARSE_CLASS = 2, MYSQL_AUDIT_AUTHORIZATION_CLASS = 3, MYSQL_AUDIT_TABLE_ACCESS_CLASS = 4, MYSQL_AUDIT_GLOBAL_VARIABLE_CLASS = 5, MYSQL_AUDIT_SERVER_STARTUP_CLASS = 6, MYSQL_AUDIT_SERVER_SHUTDOWN_CLASS = 7, MYSQL_AUDIT_COMMAND_CLASS = 8, MYSQL_AUDIT_QUERY_CLASS = 9, MYSQL_AUDIT_STORED_PROGRAM_CLASS = 10, These new notifications are sent from various parts of the code. I think that part can be most difficult to keep similar between Maria and My. New data structures are declared for these classes to send data inside notify(). In my opinion these data structures only contain little information, not sufficient for many tasks. 2. Plugin can specify the CLASS/SUBCLASS filter in the st_mysql_audit structure. To implement that nubers of subclasses were changed (i mean previously known subclasses). 3. notify() function returns 'int' (it used to be 'void'). I think 2. and 3. are the reasons MySQL doesn't load plugins with older interface. 4. Notification function can affect server. - Nonzero return from the notify() breaks the query execution in some cases. - Query can be modified inside the 'notify' function in some cases. 5. Sending notification looks quite crazy in the code. All notification-sending functions share the exactly same name 'mysql_audit_notify'. Naturally the number and types of parameters are different. So that i have complications if i want to find all the places where for instance the MYSQL_AUDIT_PARSE_CLASS notifications are sent. 'grep' isn't much help anymore. 6. The code is immature. The MYSQL_AUDIT_TABLE_ACCESS_CLASS just is not supported. st_mysql_audit::class_mask doesn't always work as it's supposed to. Now my proposals for our audit API. The 'radical' proposal : make the notify() function return 'int' freeze the AUDIT_INTERFACE_VERSION forever. notify() expects the 'event_class' as a parameter. It does something with the classes it is aware of. if it gets the class it doesn't know about - it should just return. notify() at the moment is expected to get the server data from the 'const void *event' sent to it as a paramter, Which is in my opinion inconvenient for everybody. My idea is to provide a set of functions like 'ap_get_query_string(MYSQL_THD thd, const void *event)', or 'ap_get_connection_id(MYSQL_THD thd, const void *event)'. That set can be extended in newer versions of the server, but old functions never change. If a plugin connects to the old server where that new possibilities are not implemented for some reason, these function still available, just do nothing. Plugin can check if some functions are available with the server it connected to. We provide the plugin developer with an 'audit_plugin_adaptor.c' file, that is built inside the plugin. It checks the version of the server that loads the plugin and links or implements the 'ap_*' functions respectively. Also it provides proper structures for the connecting server and interceipts notification's call to prepare it for the developer's notify() function. That way the plugin once built can work with variety of servers. And getting newer 'audit_plugin_adaptor.c' is going to make your plugin working with the newer versions of a server (be it MariaDB or MySQL). Less radical proposition is to mimic MySQL's API, to make the plugin compilable both in MySQL and MariaDB environment. So we get rid of the 'maria_declare_plugin', leaving only 'mysql_declare_plugin'. The present idea of having both doesn't work well. Last option i see - get rid of the 'mysql_declare_plugin', leaving only 'maria_declare_plugin' and forget about the idea that same plugin is used in MySQL and MariaDB at the same time. In this case i'd provide the set of the beforementioned 'ap_*' functions for the plugin to get server data. As before i belive it'd be convenient, we'd never need to check the plugin API version, and once built the plugin can work with many versions of MariaDB. And we don't need the 'adaptor.c' file as the proper .h file seem to be enough. Best regards. HF

1 0

[Maria-developers] convert_tz unexpectedly returning fractional seconds
by Yitzchak Scott-Thoennes 27 Jan '16

27 Jan '16

Using the ubuntu 14.04 mariadb (5.5.46), convert_tz unexpectedly returns a time with fractional seconds if its first argument is an expression using a column. This doesn't happen with mysqldb 5.5.46. Example: select convert_tz(concat('2006-05-25 17:45:00',''),'+00:00','+00:00') no_columns_used, convert_tz(concat('2006-05-25 17:45:00',blank),'+00:00','+00:00') column_used from (select '' blank) foo \G *************************** 1. row *************************** no_columns_used: 2006-05-25 17:45:00 column_used: 2006-05-25 17:45:00.000000

2 1

[Maria-developers] MDEV-9181 (NULLIF(count(table.col)), 0) gives wrong result on 10.1.x
by Alexander Barkov 26 Jan '16

26 Jan '16

Hi, Please review a new version of a patch for MDEV-9181. Thanks.

1 0

[Maria-developers] MDEV-9332 Bug after upgrade to 10.1.10
by Alexander Barkov 26 Jan '16

26 Jan '16

Hi Sergei, Please review a patch form MDEV-9332. It does the following: - Changes the behaviour of copy_if_not_alloced in case from->Alloced_length==0 and from_length==0 to return "to" instead of "from", because "from" can point to a constant string. - Rewrites the code to make it more readable (I hope), adding comments. Thanks.

2 2

Re: [Maria-developers] [Commits] e58e69e: MDEV-6421 SQL_ERROR_LOG doesn't log comments in Events.
by Sergei Golubchik 25 Jan '16

25 Jan '16

Hi, Alexey! On Jan 25, Alexey Botchkov wrote: > revision-id: e58e69e2feed36cdbffbae18f75faf0aea028401 (mariadb-10.1.10-24-ge58e69e) > parent(s): ff8d4009a7743b99c1c13831172b08041a4b272e > committer: Alexey Botchkov > timestamp: 2016-01-25 11:28:58 +0400 > message: > > MDEV-6421 SQL_ERROR_LOG doesn't log comments in Events. > Change parser so it saves all the query line to the ';' in the > sp_instr::m_query. Looks ok to push, thanks! Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0