- developers - lists.mariadb.org

Re: [Maria-developers] 29e99a7: MDEV-717 LP:1003679 - Wrong binlog order on concurrent DROP schema and
by Sergei Golubchik 21 Mar '16

21 Mar '16

Hi, Alexey! In general the patch is pretty good. But I've had few comments, mostly about bad choice of error messages. It's a historical behavoir that you decided not to change. If you'd prefer to leave it as is, that's ok, but then, please, create a new MDEV about these error messages. And add test cases :) On Mar 20, Alexey Botchkov wrote: > revision-id: 29e99a701817da56499df9126830d9533e4d66f3 (mariadb-10.1.12-26-g29e99a7) > parent(s): 7cb16dc2a369058b31df1b3999989f6beff94d07 > committer: Alexey Botchkov > timestamp: 2016-03-20 23:57:26 +0400 > message: > > MDEV-717 LP:1003679 - Wrong binlog order on concurrent DROP schema and > CREATE function. > The cause of the issue is when DROP DATABASE takes > metadata lock and is in progress through it's > execution, a concurrently running CREATE FUNCTION checks > for the existence of database which it succeeds and then it > waits on the metadata lock. Once DROP DATABASE writes to > BINLOG and finally releases the metadata lock on schema > object, the CREATE FUNCTION waiting on metadata lock gets > in it's code path and succeeds and writes to binlog. Please structure the commit comment as <first line - the subject> <empty line> <the rest - the body> That's the convention and various tools, for example, github itself, use it to parse and display the commit subject and body. > --- > sql/events.cc | 29 ++++++++++++++------ > sql/sp.cc | 81 +++++++++++++++++++++++++++++++++++++------------------- > sql/sp.h | 2 +- > sql/sql_parse.cc | 56 +++++++-------------------------------- What, no test cases at all? Please, add tests for everything you've fixed: - create event - update event with moving to a new database > diff --git a/sql/sp.cc b/sql/sp.cc > index 6ec5914..11ba186 100644 > --- a/sql/sp.cc > +++ b/sql/sp.cc > @@ -1040,7 +1045,22 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp) > > /* Grab an exclusive MDL lock. */ > if (lock_object_name(thd, mdl_type, sp->m_db.str, sp->m_name.str)) > - DBUG_RETURN(SP_OPEN_TABLE_FAILED); > + { > + my_error(ER_SP_STORE_FAILED, MYF(0), SP_TYPE_STRING(type), sp->m_name.str); No better error message here? May be ER_BAD_DB_ERROR? I understand that the old behavior is ER_SP_STORE_FAILED here. > + DBUG_RETURN(TRUE); > + } > + > + /* > + Check that a database directory with this name > + exists. Design note: This won't work on virtual databases > + like information_schema. > + */ > + if (check_db_dir_existence(sp->m_db.str)) > + { > + my_error(ER_BAD_DB_ERROR, MYF(0), sp->m_db.str); > + DBUG_RETURN(TRUE); > + } > + > > /* Reset sql_mode during data dictionary operations. */ > thd->variables.sql_mode= 0; > @@ -1049,7 +1069,9 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp) > thd->count_cuted_fields= CHECK_FIELD_WARN; > > if (!(table= open_proc_table_for_update(thd))) > - ret= SP_OPEN_TABLE_FAILED; > + { > + my_error(ER_SP_STORE_FAILED, MYF(0), SP_TYPE_STRING(type),sp->m_name.str); better error message, may be? > + } > else > { > /* Checking if the routine already exists */ > @@ -1268,7 +1289,8 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp) > &(thd->lex->definer->host), > saved_mode)) > { > - ret= SP_INTERNAL_ERROR; > + my_error(ER_SP_STORE_FAILED, MYF(0), > + SP_TYPE_STRING(type), sp->m_name.str); That's OOM, can be a better error message too. Btw, could you reverse the return value of show_create_sp() please? > goto done; > } > /* restore sql_mode when binloging */ > @@ -1277,9 +1299,14 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp) > if (thd->binlog_query(THD::STMT_QUERY_TYPE, > log_query.ptr(), log_query.length(), > FALSE, FALSE, FALSE, 0)) > - ret= SP_INTERNAL_ERROR; > + { > + my_error(ER_SP_STORE_FAILED, MYF(0), > + SP_TYPE_STRING(type), sp->m_name.str); that's very confusing. "Failed to CREATE %s %s" when the creation has actually succeeded. The same applies to the OOM error above. > + goto done; > + } > thd->variables.sql_mode= 0; > } > + ret= FALSE; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

[Maria-developers] Randomness in TokuDB builds
by Otto Kekäläinen 21 Mar '16

21 Mar '16

Hello TokuDB experts! There is some randomness, maybe some unique build id in every TokuDB build that breaks the reproducable builds of it in Debian. Check out: https://tests.reproducible-builds.org/dbd/unstable/amd64/mariadb-10.0_10.0.… I can't find any mentions of a build id or similar in the sources. Can somebody help me figure out where the randomness stems from, and how I should patch the source to make the build deterministic? - Otto

2 1

[Maria-developers] Please review MDEV-9516 type error when setting session variable
by Alexander Barkov 21 Mar '16

21 Mar '16

Hello Sergei, Please review a patch for MDEV-9516. Thanks.

1 0

Re: [Maria-developers] 8d675d5: MDEV-6058 MySQL Bug #11766693: LOG-SLOW-ADMIN-STATEMENTS AND
by Sergei Golubchik 21 Mar '16

21 Mar '16

Hi, Alexey! On Mar 21, Alexey Botchkov wrote: > revision-id: 8d675d5764e7d8ecad19fafdd167bee9a41d1dc7 (mariadb-10.1.12-27-g8d675d5) > parent(s): 94768542115289272996f6450b7d3cafa75dcead > committer: Alexey Botchkov > timestamp: 2016-03-21 13:02:09 +0400 > message: > > MDEV-6058 MySQL Bug #11766693: LOG-SLOW-ADMIN-STATEMENTS AND > LOG-SLOW-SLAVE-STATEMENTS NOT DISPLAYED. > These parameters were moved from the command line options to the > system variables section. Treatment of the > opt_log_slow_slave_statements changed to let the > dynamic change of the variable. Thanks! Looks good, just one question: > diff --git a/mysql-test/suite/rpl/r/rpl_slow_query_log.result b/mysql-test/suite/rpl/r/rpl_slow_query_log.result > index 79b83b4..871722e 100644 > --- a/mysql-test/suite/rpl/r/rpl_slow_query_log.result > +++ b/mysql-test/suite/rpl/r/rpl_slow_query_log.result > @@ -76,6 +76,18 @@ ALTER TABLE t1 ADD INDEX id1(a); > INSERT INTO t1 values(1, sleep(3)); > ### Assertion is good. Both Master and Slave exhibit the > ### same number of queries in slow log: 1 > +******************************************************************** > +**** TRUNCATE the slow log then check whether runtime changes of > +**** log_slow_slave_statements work without slave restart. > +******************************************************************** > +SET @old_log_slow_slave_statements= @@global.log_slow_slave_statements; > +SET @@global.log_slow_slave_statements = off; > +TRUNCATE mysql.slow_log; > +INSERT INTO t1 values(1, sleep(3));; > +0 > +SET @@global.log_slow_slave_statements = on; > +INSERT INTO t1 values(1, sleep(3));; > +1 > SET @@global.log_output= @old_log_output; > SET @@global.long_query_time= @old_long_query_time; > DROP TABLE t1; > diff --git a/mysql-test/suite/rpl/t/rpl_slow_query_log.test b/mysql-test/suite/rpl/t/rpl_slow_query_log.test > index faf037a..3505883 100644 > --- a/mysql-test/suite/rpl/t/rpl_slow_query_log.test > +++ b/mysql-test/suite/rpl/t/rpl_slow_query_log.test > @@ -299,6 +299,39 @@ if ($master_slow_query == $slave_slow_query) > -- echo ### same number of queries in slow log: $master_slow_query > } > > +-- echo ******************************************************************** > +-- echo **** TRUNCATE the slow log then check whether runtime changes of > +-- echo **** log_slow_slave_statements work without slave restart. > +-- echo ******************************************************************** > + > +SET @old_log_slow_slave_statements= @@global.log_slow_slave_statements; > +SET @@global.log_slow_slave_statements = off; > +TRUNCATE mysql.slow_log; > + > +-- connection master > + > +--disable_warnings > +-- eval $slow_query; > +--enable_warnings > +sync_slave_with_master; > + > +-- connection slave > + > +eval SELECT sql_text FROM mysql.slow_log WHERE sql_text like '$slow_query'; > + > +SET @@global.log_slow_slave_statements = on; > + > +-- connection master > + > +--disable_warnings > +-- eval $slow_query; > +--enable_warnings > +sync_slave_with_master; > + > +-- connection slave > + > +eval SELECT sql_text FROM mysql.slow_log WHERE sql_text like '$slow_query'; Did you run this test? It seems the result doesn't match the test. The result looks like as if you'd have eval SELECT COUNT(*) FROM ... Please, run the full test suite before pushing. > + > -- connection master > SET @@global.log_output= @old_log_output; > SET @@global.long_query_time= @old_long_query_time; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] MDEV-9369 IN operator with ( num, NULL ) gives inconsistent result
by Sergei Golubchik 21 Mar '16

21 Mar '16

Hi, Alexander! On Feb 08, Alexander Barkov wrote: > Hello Sergei, > > Please review a patch for MDEV-9369. > > Briefly, it fixes cmp_item::cmp() to return three possible values: > FALSE, TRUE, UNKNOWN. These values are then recursively pulled to > the very top level, to the owner Item_xxx. > > The owner Item_func_in (as well as Item_func_case and Item_func_equal) > then further "translate" these tree-values logic into the traditional > val_int()+null_value combination using some additional information. > > For example, Item_func_in::val_int() uses "negated" as an additional > information. > > The patch looks Ok for me. Looks ok to me too. A couple of stylistic comments, see below. ok to push afterwards. > diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc > index d5f5087..39003de 100644 > --- a/sql/item_cmpfunc.cc > +++ b/sql/item_cmpfunc.cc > @@ -3837,15 +3843,15 @@ void cmp_item_decimal::store_value(Item *item) > /* val may be zero if item is nnull */ > if (val && val != &value) > my_decimal2decimal(val, &value); > + set_null_value(item->null_value); > } > > > int cmp_item_decimal::cmp(Item *arg) > { > my_decimal tmp_buf, *tmp= arg->val_decimal(&tmp_buf); > - if (arg->null_value) > - return 1; > - return my_decimal_cmp(&value, tmp); > + return (m_null_value || arg->null_value) ? if you access m_null_value directly, you don't need a setter for it. it'll be less confusing if you replace set_null_value(X) with m_null_value=X. alternatively, you can add a getter is_null_value(). But I personally wouldn't do it, cmp_xxx is a small set of simple classes, no need to get too enterprisey there. > + UNKNOWN : (my_decimal_cmp(&value, tmp) != 0); > } > > > @@ -3892,10 +3900,10 @@ cmp_item *cmp_item_datetime::make_same() > } > > > -bool Item_func_in::nulls_in_row() > +bool Item_func_in::list_contains_null() I found the old name clear enough. But ok, whatever... > { > Item **arg,**arg_end; > - for (arg= args+1, arg_end= args+arg_count; arg != arg_end ; arg++) > + for (arg= args + 1, arg_end= args+arg_count; arg != arg_end ; arg++) > { > if ((*arg)->null_inside()) > return 1; > @@ -4010,6 +4018,32 @@ void Item_func_in::fix_length_and_dec() > } > } > > + /* > + First conditions for bisection to be possible: > + 1. All types are similar, and > + 2. All expressions in <in value list> are const > + */ > + bool bisection_possible= > + type_cnt == 1 && // 1 > + const_itm; // 2 > + if (bisection_possible) > + { > + /* > + In the presence of NULLs, the correct result of evaluating this item > + must be UNKNOWN or FALSE. To achieve that: > + - If type is scalar, we can use bisection and the "have_null" boolean. > + - If type is ROW, we will need to scan all of <in value list> when > + searching, so bisection is impossible. Unless: > + 3. UNKNOWN and FALSE are equivalent results > + 4. Neither left expression nor <in value list> contain any NULL value > + */ > + > + if (cmp_type == ROW_RESULT && > + !((is_top_level_item() && !negated) || // 3 > + (!list_contains_null() && !args[0]->maybe_null))) // 4 > + bisection_possible= false; I think the condition will be easier to understand if you remove the negation of that complex condition in parentheses: if (cmp_type == ROW_RESULT && ((!is_top_level_item() || negated) && // 3 (list_contains_null() || args[0]->maybe_null))) // 4 I find this ^^^ much easier to understand. > + } > + > if (type_cnt == 1) > { > if (cmp_type == STRING_RESULT && Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 1

[Maria-developers] Google Summer of Code [MySql -> MsSql]
by Bo Zhu 20 Mar '16

20 Mar '16

Hi, everyone. I am Bo, a second year master student in Clemson university, SC. I had ever implemented a small relational database and java compiler. I am really interested in the MaxScale filter to translate the Mysql query to MsSql one. In this task, we need to write some C++ code as a MaxScale's filter, right ? I think it is about syntax translation which is a compiler front-end problem. I have shared my proposal draft and am really looking forward to get some feedback.

1 0

[Maria-developers] MaxScale filter to real Microsoft SQL Server syntax
by Wepngong Benaiah 20 Mar '16

20 Mar '16

> > Hello everyone, I am Wepngong Ngeh Benaiah, a 4th year Computer Engineering student at the University of Buea, Cameroon. I have been looking at gsoc projects on https://mariadb.com/kb/en/mariadb/google-summer-of-code-2016/ and got interested in *MaxScale filter to real Microsoft SQL Server syntax [https://jira.mariadb.org/browse/MXS-1 <https://jira.mariadb.org/browse/MXS-1>]. * I will love to contribute to MariaDB by means of GSOC2016. I have drafted a proposal and submitted to google and will love to get feedback on this Thanks in advance -- Buea,Cameroon University of Buea Faculty of Engineering and Technology Computer Engineering Software Engineering * Wepngong Ngeh Benaiah* *https://github.com/bwepngong <https://github.com/bwepngong>*

1 0

[Maria-developers] GSoC 2016 CONC-125
by Christian Massey 19 Mar '16

19 Mar '16

Hi everyone, I am Christian Massey a computer science student at York University and I wish to work on Providing import and export functions for popular data formats like JSON, XML , PHP, ... for Connector/C and MariaDB Server. I have been inspecting the code to understand how the dynamic column packed string is encoded and I see kind on big endian encoding( Correct me if I am wrong please). Also I see that the old format already work like arrays and it uses 3 bits for encoding type which is the main reason why not all types in the enum are supported in this old format. DBUG_ASSERT(((type - 1) & (~7)) == 0); /* fit in 3 bits */ (Again please correct me if I am wrong. The encoding is not easy to understand) My Question is do we need to create a completely new type to support arrays or do we change the old format which already work like arrays in a way but lacks support for some types(I don't know which of the enums yet). Please, I wish to write a proposal for this project and hope to discuss more with mentors for more clarification on how we are going to implement this as it seems to be a very important if not the most critical part of this project. Cheers, Kris.

2 2

[Maria-developers] Aggregate stored functions [MDEV-7773]
by Varun Gupta 19 Mar '16

19 Mar '16

Well i have checked out the syntax for the databases for aggregate functions. I thought using the syntax similar to that of HSQL would be good. In it if we are having N tuples then we are computing the values for the N rows and then after that we make another call and return the value for the function. Well this seems very reasonable to do. Using PostgreSQL <http://www.postgresql.org/docs/8.3/static/sql-createaggregate.html> we need to have two regular functions but if we have an aggregate function like Count(column_name) , then this would be done in just a single function , I guess we would not need two functions for it . Regards, Varun

2 11

Re: [Maria-developers] b1a1e5b: MDEV-6058 MySQL Bug #11766693: LOG-SLOW-ADMIN-STATEMENTS AND LOG-SLOW-SLAVE-STATEMENTS NOT DISPLAYED.
by Sergei Golubchik 18 Mar '16

18 Mar '16

Hi, Alexey! On Mar 18, Alexey Botchkov wrote: > revision-id: b1a1e5b3d3d5ab95c596580de07daf38a3efa039 (mariadb-10.1.12-19-gb1a1e5b) > parent(s): 4aac51db9a55a11574cbfa60484d4329d35b6f2c > committer: Alexey Botchkov > timestamp: 2016-03-18 11:47:05 +0400 > message: > > MDEV-6058 MySQL Bug #11766693: LOG-SLOW-ADMIN-STATEMENTS AND LOG-SLOW-SLAVE-STATEMENTS NOT DISPLAYED. > These parameters were moved from the command line options to the > system variables section. Treatment of the > opt_log_slow_slave_statements changed to let the dynamic change > of the variable. > > --- > mysql-test/r/variables.result | 10 ++ > mysql-test/suite/rpl/r/rpl_slow_query_log.result | 10 ++ > mysql-test/suite/rpl/t/rpl_slow_query_log.test | 43 ++++++++ > .../r/log_slow_admin_statements_basic.result | 76 ++++++++++++++ > .../r/log_slow_admin_statements_func.result | 46 +++++++++ > .../r/log_slow_slave_statements_basic.result | 76 ++++++++++++++ > .../t/log_slow_admin_statements_basic.test | 110 +++++++++++++++++++++ > .../sys_vars/t/log_slow_admin_statements_func.test | 61 ++++++++++++ > .../t/log_slow_slave_statements_basic.test | 110 +++++++++++++++++++++ > mysql-test/t/variables.test | 11 +++ I suppose you didn't run the full test suite. Otherwise you would've noticed that sysvar_* results were changed. Also, because we have sysvar_* tests, we no longer create <varname>_basic tests for every new variable, they are no longer useful. Starting from 10.1 you only need to test the functionality of new variables, not their basic properties. > sql/log_event.cc | 10 ++ > sql/mysqld.cc | 8 -- > sql/sys_vars.cc | 13 +++ > 13 files changed, 576 insertions(+), 8 deletions(-) > > diff --git a/mysql-test/r/variables.result b/mysql-test/r/variables.result > index fef3e4a..e46e65f 100644 > --- a/mysql-test/r/variables.result > +++ b/mysql-test/r/variables.result > @@ -1797,3 +1797,13 @@ select * from information_schema.session_variables where variable_name='sql_mode > VARIABLE_NAME VARIABLE_VALUE > SQL_MODE ANSI_QUOTES > End of 5.5 tests > +show variables like 'log_slow%statements'; > +Variable_name Value > +log_slow_admin_statements OFF > +log_slow_slave_statements OFF > +select * from information_schema.global_variables where variable_name like 'log_slow_admin_statements'; > +VARIABLE_NAME VARIABLE_VALUE > +LOG_SLOW_ADMIN_STATEMENTS OFF > +select * from information_schema.global_variables where variable_name like 'log_slow_slave_statements'; > +VARIABLE_NAME VARIABLE_VALUE > +LOG_SLOW_SLAVE_STATEMENTS OFF You can remove this test. > diff --git a/mysql-test/suite/rpl/t/rpl_slow_query_log.test b/mysql-test/suite/rpl/t/rpl_slow_query_log.test > index faf037a..de336fc 100644 > --- a/mysql-test/suite/rpl/t/rpl_slow_query_log.test > +++ b/mysql-test/suite/rpl/t/rpl_slow_query_log.test > @@ -299,6 +299,49 @@ if ($master_slow_query == $slave_slow_query) > -- echo ### same number of queries in slow log: $master_slow_query > } > > +-- echo ******************************************************************** > +-- echo **** TRUNCATE the slow log then check whether runtime changes of > +-- echo **** log_slow_slave_statements work without slave restart. > +-- echo ******************************************************************** > + > +SET @old_log_slow_slave_statements= @@global.log_slow_slave_statements; > +SET @@global.log_slow_slave_statements = off; > +TRUNCATE mysql.slow_log; > + > +-- connection master > + > +--disable_warnings > +-- eval $slow_query; > +--enable_warnings > +sync_slave_with_master; > + > +-- connection slave > + > +let $slave_slow_query= `SELECT count(*) = 1 FROM mysql.slow_log WHERE sql_text like '$slow_query'`; > +if ($slave_slow_query) > +{ > + SELECT * FROM mysql.slow_log; > + die "Assertion failed! Slow query FOUND in slow query log. Bailing out!"; > +} please, NO! If you want to test whether a certain row is present in a certain table you do SELECT columns FROM table; may be with a WHERE clause. But never if (`SELECT`) { die; } > + > +SET @@global.log_slow_slave_statements = on; > + > +-- connection master > + > +--disable_warnings > +-- eval $slow_query; > +--enable_warnings > +sync_slave_with_master; > + > +-- connection slave > + > +let $slave_slow_query= `SELECT count(*) = 1 FROM mysql.slow_log WHERE sql_text like '$slow_query'`; > +if (!$slave_slow_query) > +{ > + SELECT * FROM mysql.slow_log; > + die "Assertion failed! Slow query NOT FOUND in slow query log. Bailing out!"; > +} Same. > + > -- connection master > SET @@global.log_output= @old_log_output; > SET @@global.long_query_time= @old_long_query_time; > diff --git a/mysql-test/suite/sys_vars/r/log_slow_admin_statements_basic.result b/mysql-test/suite/sys_vars/r/log_slow_admin_statements_basic.result > new file mode 100644 > index 0000000..1cd9135 > --- /dev/null > +++ b/mysql-test/suite/sys_vars/r/log_slow_admin_statements_basic.result Remove this useless test, please > diff --git a/mysql-test/suite/sys_vars/r/log_slow_slave_statements_basic.result b/mysql-test/suite/sys_vars/r/log_slow_slave_statements_basic.result > new file mode 100644 > index 0000000..b9e88ce > --- /dev/null > +++ b/mysql-test/suite/sys_vars/r/log_slow_slave_statements_basic.result > @@ -0,0 +1,76 @@ And this one, too. > diff --git a/mysql-test/suite/sys_vars/t/log_slow_admin_statements_func.test b/mysql-test/suite/sys_vars/t/log_slow_admin_statements_func.test > new file mode 100644 > index 0000000..78e4d35 > --- /dev/null > +++ b/mysql-test/suite/sys_vars/t/log_slow_admin_statements_func.test > + > +# ALTER, OPTIMIZE, CHECK and DROP operations should be logged in slow query log. > +SELECT sql_text FROM mysql.slow_log WHERE sql_text LIKE '%TABLE log_slow_admin_statements%'; this is how one checks whether a query was logged. > diff --git a/sql/log_event.cc b/sql/log_event.cc > index e99ef16..9793f7d 100644 > --- a/sql/log_event.cc > +++ b/sql/log_event.cc > @@ -4428,6 +4428,16 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi, > if (thd->m_digest != NULL) > thd->m_digest->reset(thd->m_token_array, max_digest_length); > > + if (thd->slave_thread && > + (thd->variables.sql_log_slow != opt_log_slow_slave_statements)) > + { > + /* > + The log_slow_slave_statements variable can be changed dynamically, > + so we have to set the sql_log_slow respectively. > + */ > + thd->variables.sql_log_slow= opt_log_slow_slave_statements; > + } Really? Why not if (thd->slave_thread) thd->variables.sql_log_slow= opt_log_slow_slave_statements; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2