- developers - lists.mariadb.org

[Maria-developers] Process suggestion for minor issues
by Rasmus Johansson 14 Jun '13

14 Jun '13

Hi MariaDB Developers, This is a suggestion on how to deal with minor or lower priority bugs and tasks in JIRA, http://mariadb.org/jira . Currently, the process we use makes a clear differentiation between major or higher prioirty issues (called majors altogether after this) and minor or lower priority issues (called minors altogether after this). The majors are the ones that should be fixed for a release and the minors are rolling, meaning that they are done when the corresponding developer has finished his/her majors. This of course sounds logical, but it also leads to a growing number of minors for every consecutive release, many of them which never will be fixed but in the current process will hang around. Here is a suggested process to handle the ever growing amount of minors. If a minor issue isn't fixed after two consecutive releases it's re-reviewed and either priority changed to major (or higher), or the minor issue is closed with resolution Won't Fix and a comment. Example of comment can be that "this is an edge case where effort and benefits doesn't correlate". It should also be noted that after closing an issue in JIRA, it doesn't mean that it disappears. It will still show up in free text searches and among the issues for the version where it was closed. Please provide your thoughts. Do you think it's an OK way to deal with the minors or do you prefer some other way? Best regards, Rasmus

6 8

Re: [Maria-developers] Review request: mdev-4635
by Alexander Barkov 14 Jun '13

14 Jun '13

Hi Sergei, On 06/14/2013 04:30 PM, Sergei Golubchik wrote: > Hi, Alexander! > > On Jun 13, Alexander Barkov wrote: >> On 06/13/2013 08:16 PM, Sergei Golubchik wrote: >>> >>>> === modified file 'mysql-test/r/strict.result' >>>> --- mysql-test/r/strict.result 2011-06-05 02:56:06 +0000 >>>> +++ mysql-test/r/strict.result 2013-06-13 12:15:43 +0000 >>>> @@ -213,11 +213,11 @@ ERROR 22007: Incorrect date value: '2004 >>>> INSERT INTO t1 (col1) VALUES(STR_TO_DATE('0.10.2004 15.30','%d.%m.%Y %H.%i')); >>>> ERROR 22007: Incorrect date value: '2004-10-00 15:30:00' for column 'col1' at row 1 >>>> INSERT INTO t1 (col1) VALUES(STR_TO_DATE('31.9.2004 15.30','%d.%m.%Y %H.%i')); >>>> -ERROR 22007: Incorrect date value: '2004-09-31 15:30:00' for column 'col1' at row 1 >>>> +ERROR HY000: Incorrect datetime value: '31.9.2004 15.30' for function str_to_date >>> >>> Hmm, I don't know. I liked the old error message more. >>> Indeed, datetime value '31.9.2004 15.30' is just fine for a function str_to_date(). >>> But '2004-09-31 15:30:00' is not fine for column 'col1' in the strict mode. >>> >>> Why did that change? >> >> Before the change the warning was generated in >> Field_timestamp::store_TIME_with_warning. >> >> Now this warning is generated much earlier, >> in Item_func_str_to_date::get_date(), >> after check_date() is called. > > Which, I think, it wrong. The date is perfectly ok, as far as > STR_TO_DATE is concerted, it's wrong for a field. > >> The problem was that Item_func_str_to_date: >> a. checked TIME_NO_ZERO_DATE >> b. did not check TIME_NO_ZERO_IN_DATE (the bug) >> c. did not check invalid dates >> >> After the patch it: >> >> a. checks TIME_NO_ZERO_DATE >> b. checks TIME_NO_ZERO_IN_DATE >> c. checks invalid dates >> >> I thought it would be good to check everything, >> for consistency. But a side effect is a different warning. > > I agree, it should check everything. But, perhaps, the field should not > pass TIME_NO_ZERO_DATE|TIME_NO_ZERO_IN_DATE down to the arg->get_date() ? It does not. The above warnings are not about zeros. They are about wrong dates: for example, there is no September 31, like in '31.9.2004 15.30'. The last version of the patch does not change the warnings. It works as follows: - str_to_date checks TIME_NO_ZERO_DATE and TIME_NO_ZERO_IN_DATE with help of check_date(), but only if the caller wants it (CONVERT_TZ wants, field does not). - str_to_date suppresses checking of invalid dates inside check_date() and leaves this responsibility to the caller (like it was before the patch). If we want check invalid dates inside check_date() in str_to_date, then this warning "Incorrect date value: '2004-09-31 15:30:00' for column 'col1' at row 1" won't be possible, because str_to_date does not know the name of the column it's being inserted into. But we could try to produce something like this: "Incorrect datetime value: '2004-09-31 15:30:00'" I.e. a better formatted value, but without the column name. > > Regards, > Sergei >

1 0

[Maria-developers] Review request: new SHUTDOWN server command
by James Briggs 14 Jun '13

14 Jun '13

Hi Monty. Here's the shutdown command (WL#232) I wrote for review. I think there's 2 steps to improve server shutdown: 1) apply this patch now 2) later on, fix the old shutdown thread race problems, clean up the old kill_mysql() design and code, then add a few options to the shutdown command like WAIT|IMMEDIATE|ABORT. This code is released under the Open Source BSD-new License, according to the MariaDB Contributor Agreement. Thanks, James Briggs. james.briggs(a)yahoo.com http://www.jebriggs.com/blog/2013/05/patch-to-add-shutdown-statement-to-mys… --- sql_parse.cc 2013-03-11 03:29:13.000000000 -0700 +++ /home/james/mariadb-5.5.30-new/sql/sql_parse.cc 2013-05-15 13:17:05.000000000 -0700 @@ -1305,7 +1305,6 @@ my_ok(thd); break; } -#ifndef EMBEDDED_LIBRARY case COM_SHUTDOWN: { status_var_increment(thd->status_var.com_other); @@ -1333,7 +1332,6 @@ error=TRUE; break; } -#endif case COM_STATISTICS: { STATUS_VAR *current_global_status_var; // Big; Don't allocate on stack @@ -3736,6 +3734,31 @@ lex->kill_signal); break; } + case SQLCOM_SHUTDOWN: + { + // jeb - This code block is copied from COM_SHUTDOWN above. Since kill_mysql(void) {} doesn't take a level argument, the level code is pointless. + // jeb - In fact, the level code should be removed and Oracle Database statements implemented: SHUTDOWN, SHUTDOWN IMMEDIATE and SHUTDOWN ABORT. See WL#232. + + status_var_increment(thd->status_var.com_other); + if (check_global_access(thd,SHUTDOWN_ACL)) + break; /* purecov: inspected */ + + enum mysql_enum_shutdown_level level; + level= SHUTDOWN_DEFAULT; + if (level == SHUTDOWN_DEFAULT) + level= SHUTDOWN_WAIT_ALL_BUFFERS; // soon default will be configurable + else if (level != SHUTDOWN_WAIT_ALL_BUFFERS) + { + my_error(ER_NOT_SUPPORTED_YET, MYF(0), "this shutdown level"); + break; + } + DBUG_PRINT("SQLCOM_SHUTDOWN",("Got shutdown command for level %u", level)); + my_eof(thd); + kill_mysql(); + res=TRUE; + break; + } + #ifndef NO_EMBEDDED_ACCESS_CHECKS case SQLCOM_SHOW_GRANTS: { --- sql_yacc.yy 2013-03-11 03:29:19.000000000 -0700 +++ /home/james/mariadb-5.5.30-new/sql/sql_yacc.yy 2013-05-15 11:12:03.000000000 -0700 @@ -791,7 +791,7 @@ Currently there are 174 shift/reduce conflicts. We should not introduce new conflicts any more. */ -%expect 174 +%expect 196 /* Comments for TOKENS. @@ -1645,6 +1645,7 @@ definer_opt no_definer definer parse_vcol_expr vcol_opt_specifier vcol_opt_attribute vcol_opt_attribute_list vcol_attribute + shutdown END_OF_INPUT %type call sp_proc_stmts sp_proc_stmts1 sp_proc_stmt @@ -1796,6 +1797,7 @@ | savepoint | select | set + | shutdown | signal_stmt | show | slave @@ -13715,6 +13717,17 @@ ; +shutdown: + SHUTDOWN + { + LEX *lex=Lex; + lex->value_list.empty(); + lex->users_list.empty(); + lex->sql_command= SQLCOM_SHUTDOWN; + } + ; + + set_expr_or_default: expr { $$=$1; } | DEFAULT { $$=0; } --- sql_prepare.cc 2013-03-11 03:29:11.000000000 -0700 +++ /home/james/mariadb-5.5.30-new/sql/sql_prepare.cc 2013-05-15 03:07:00.000000000 -0700 @@ -2173,6 +2173,7 @@ case SQLCOM_GRANT: case SQLCOM_REVOKE: case SQLCOM_KILL: + case SQLCOM_SHUTDOWN: break; case SQLCOM_PREPARE: --- mysqld.cc 2013-03-11 03:29:14.000000000 -0700 +++ /home/james/mariadb-5.5.30-new/sql/mysqld.cc 2013-05-15 01:20:11.000000000 -0700 @@ -3333,6 +3333,7 @@ {"savepoint", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SAVEPOINT]), SHOW_LONG_STATUS}, {"select", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SELECT]), SHOW_LONG_STATUS}, {"set_option", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SET_OPTION]), SHOW_LONG_STATUS}, + {"shutdown", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SHUTDOWN]), SHOW_LONG_STATUS}, {"signal", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SIGNAL]), SHOW_LONG_STATUS}, {"show_authors", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SHOW_AUTHORS]), SHOW_LONG_STATUS}, {"show_binlog_events", (char*) offsetof(STATUS_VAR, com_stat[(uint) SQLCOM_SHOW_BINLOG_EVENTS]), SHOW_LONG_STATUS}, --- sql_lex.h 2013-03-11 03:29:13.000000000 -0700 +++ /home/james/mariadb-5.5.30-new/sql/sql_lex.h 2013-05-15 01:19:17.000000000 -0700 @@ -193,6 +193,7 @@ SQLCOM_SHOW_RELAYLOG_EVENTS, SQLCOM_SHOW_USER_STATS, SQLCOM_SHOW_TABLE_STATS, SQLCOM_SHOW_INDEX_STATS, SQLCOM_SHOW_CLIENT_STATS, + SQLCOM_SHUTDOWN, /*

1 0

Re: [Maria-developers] Review request: mdev-4635
by Alexander Barkov 13 Jun '13

13 Jun '13

Hi, Serg, On 06/13/2013 08:16 PM, Sergei Golubchik wrote: > Hi, Alexander! > > On Jun 13, Alexander Barkov wrote: >> >> please review a fix for MDEV-4635 > > Is it for 5.1 or 5.5? For 5.3. > >> === modified file 'mysql-test/r/func_time.result' >> --- mysql-test/r/func_time.result 2013-03-20 15:13:00 +0000 >> +++ mysql-test/r/func_time.result 2013-06-13 08:39:15 +0000 >> @@ -1910,3 +1910,13 @@ SELECT 1 FROM DUAL WHERE MINUTE(TIMEDIFF >> 1 >> SELECT 1 FROM DUAL WHERE SECOND(TIMEDIFF(NULL, '12:12:12')); >> 1 >> +# >> +# MDEV-4635 Crash in UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y')) >> +# >> +SET TIME_ZONE='+02:00'; >> +SELECT UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y')); >> +UNIX_TIMESTAMP(STR_TO_DATE('2020','%Y')) >> +NULL >> +Warnings: >> +Error 1411 Incorrect datetime value: '2020' for function str_to_date >> +SET TIME_ZONE=DEFAULT; >> >> === modified file 'mysql-test/r/strict.result' >> --- mysql-test/r/strict.result 2011-06-05 02:56:06 +0000 >> +++ mysql-test/r/strict.result 2013-06-13 12:15:43 +0000 >> @@ -213,11 +213,11 @@ ERROR 22007: Incorrect date value: '2004 >> INSERT INTO t1 (col1) VALUES(STR_TO_DATE('0.10.2004 15.30','%d.%m.%Y %H.%i')); >> ERROR 22007: Incorrect date value: '2004-10-00 15:30:00' for column 'col1' at row 1 >> INSERT INTO t1 (col1) VALUES(STR_TO_DATE('31.9.2004 15.30','%d.%m.%Y %H.%i')); >> -ERROR 22007: Incorrect date value: '2004-09-31 15:30:00' for column 'col1' at row 1 >> +ERROR HY000: Incorrect datetime value: '31.9.2004 15.30' for function str_to_date > > Hmm, I don't know. I liked the old error message more. > Indeed, datetime value '31.9.2004 15.30' is just fine for a function str_to_date(). > But '2004-09-31 15:30:00' is not fine for column 'col1' in the strict mode. > > Why did that change? Before the change the warning was generated in Field_timestamp::store_TIME_with_warning. Now this warning is generated much earlier, in Item_func_str_to_date::get_date(), after check_date() is called. The problem was that Item_func_str_to_date: a. checked TIME_NO_ZERO_DATE b. did not check TIME_NO_ZERO_IN_DATE (the bug) c. did not check invalid dates After the patch it: a. checks TIME_NO_ZERO_DATE b. checks TIME_NO_ZERO_IN_DATE c. checks invalid dates I thought it would be good to check everything, for consistency. But a side effect is a different warning. But we can suppress invalid dates detection, so it: a. checks TIME_NO_ZERO_DATE b. checks TIME_NO_ZERO_IN_DATE c. does not check invalid dates The test for invalid dates can be suppressed by passing "fuzzy_date | TIME_INVALID_DATES" to check_date(). See this version of the patch attached. I agree that the old warning is nicer. But I'm not sure what's better. Suppressing the test rather slightly looks like hiding more potential bugs... > >> === modified file 'sql/item_timefunc.cc' >> --- sql/item_timefunc.cc 2013-03-20 15:13:00 +0000 >> +++ sql/item_timefunc.cc 2013-06-13 08:20:13 +0000 >> @@ -1126,7 +1125,7 @@ bool Item_func_unix_timestamp::get_times >> } >> >> MYSQL_TIME ltime; >> - if (get_arg0_date(&ltime, 0)) >> + if (get_arg0_date(&ltime, TIME_NO_ZERO_IN_DATE)) >> return 1; > > This is correct, of course. > > Regards, > Sergei >

1 0

[Maria-developers] Review request: mdev-4651
by Alexander Barkov 13 Jun '13

13 Jun '13

Hi Serg, Please review a patch for "mdev-4651 Crash in my_decimal2decimal in a ORDER BY query". Thanks.

1 0

[Maria-developers] Review request: mdev-4635
by Alexander Barkov 13 Jun '13

13 Jun '13

Hi Serg, please review a fix for MDEV-4635 Thanks.

1 0

Re: [Maria-developers] Review request: mdev-4634
by Alexander Barkov 13 Jun '13

13 Jun '13

Hi Sergei, Thanks for review! On 06/11/2013 09:33 PM, Sergei Golubchik wrote: > Hi, Alexander! > > This looks like a lot of changes for what you explained the reason of > the bug was. There will be lots of "whys" below... The fix itself was quite small. However, an attempt to write a reasonable test case revealed a few other bugs. I just tried to fixing everything in a single patch. Okey. Let's do everything separately. These are separate reports for the problems found during writing a test case for 4634: https://mariadb.atlassian.net/browse/MDEV-4652 https://mariadb.atlassian.net/browse/MDEV-4653 https://mariadb.atlassian.net/browse/MDEV-4654 Here's a new one, found today: https://mariadb.atlassian.net/browse/MDEV-4651 > > On Jun 11, Alexander Barkov wrote: >> === modified file 'sql/item.cc' >> --- sql/item.cc 2013-03-17 06:41:22 +0000 >> +++ sql/item.cc 2013-06-11 11:52:43 +0000 >> @@ -252,7 +252,8 @@ String *Item::val_string_from_decimal(St >> String *Item::val_string_from_date(String *str) >> { >> MYSQL_TIME ltime; >> - if (get_date(&ltime, TIME_FUZZY_DATE) || >> + uint time_flag= field_type() == MYSQL_TYPE_TIME ? TIME_TIME_ONLY : 0; >> + if (get_date(&ltime, TIME_FUZZY_DATE | time_flag) || > > Why? > >> str->alloc(MAX_DATE_STRING_REP_LENGTH)) >> { >> null_value= 1; >> >> === modified file 'sql/item_func.cc' >> --- sql/item_func.cc 2013-04-06 13:14:46 +0000 >> +++ sql/item_func.cc 2013-06-11 10:54:50 +0000 >> @@ -2456,7 +2456,8 @@ bool Item_func_min_max::get_date(MYSQL_T >> { >> Item **arg= args + i; >> bool is_null; >> - longlong res= get_datetime_value(thd, &arg, 0, compare_as_dates, &is_null); >> + longlong res= get_datetime_value(thd, &arg, 0, compare_as_dates, >> + &is_null, fuzzy_date); > > why is that? to handle CONVERT_TZ(GREATEST(...)) ? > That is to pass NO_ZERO_DATE down? > > I'm not sure it's a good idea. Strictly speaking, only the greatest > value should be NO_ZERO_DATE, not every single one. > > I'd suggest to add check_date in Item_func_min_max::get_date() instead. > >> /* Check if we need to stop (because of error or KILL) and stop the loop */ >> if (thd->is_error() || args[i]->null_value) >> @@ -2469,6 +2470,11 @@ bool Item_func_min_max::get_date(MYSQL_T >> min_max= res; >> } >> unpack_time(min_max, ltime); >> + >> + if (!(fuzzy_date & TIME_TIME_ONLY) && >> + ((null_value= check_date_with_warn(ltime, fuzzy_date)))) >> + return true; > > Yes, that's what I mean. If you check the return value anyway, there's > no need to check it inside get_datetime_value(). > >> if (compare_as_dates->field_type() == MYSQL_TYPE_DATE) >> { >> ltime->time_type= MYSQL_TIMESTAMP_DATE; >> >> === modified file 'sql/item_timefunc.cc' >> --- sql/item_timefunc.cc 2013-03-20 15:13:00 +0000 >> +++ sql/item_timefunc.cc 2013-06-11 10:57:09 +0000 >> @@ -1923,8 +1921,10 @@ void Item_date_add_interval::fix_length_ >> bool Item_date_add_interval::get_date(MYSQL_TIME *ltime, uint fuzzy_date) >> { >> INTERVAL interval; >> - >> - if (args[0]->get_date(ltime, TIME_NO_ZERO_DATE | TIME_FUZZY_DATE | TIME_NO_ZERO_IN_DATE) || >> + if (args[0]->get_date(ltime, >> + TIME_NO_ZERO_DATE | TIME_FUZZY_DATE | TIME_NO_ZERO_IN_DATE | >> + ((args[0]->field_type() == MYSQL_TYPE_TIME) ? >> + TIME_TIME_ONLY : 0)) || > > why is that? > >> get_interval_value(args[1], int_type, &value, &interval)) >> return (null_value=1); >> >> @@ -2290,6 +2290,12 @@ bool Item_time_typecast::get_date(MYSQL_ >> return 1; >> if (decimals < TIME_SECOND_PART_DIGITS) >> ltime->second_part= sec_part_truncate(ltime->second_part, decimals); >> + if (!(fuzzy_date & TIME_TIME_ONLY)) >> + { >> + if (ltime->time_type == MYSQL_TIMESTAMP_TIME) >> + ltime->time_type= MYSQL_TIMESTAMP_DATETIME; > > why? > >> + return ((null_value= check_date_with_warn(ltime, fuzzy_date))); >> + } >> /* >> MYSQL_TIMESTAMP_TIME value can have non-zero day part, >> which we should not lose. >> === modified file 'sql/item_timefunc.h' >> --- sql/item_timefunc.h 2012-12-28 12:41:46 +0000 >> +++ sql/item_timefunc.h 2013-06-11 11:19:03 +0000 >> @@ -492,6 +492,8 @@ class Item_timefunc :public Item_tempora >> Item_timefunc(Item *a,Item *b) :Item_temporal_func(a,b) {} >> Item_timefunc(Item *a, Item *b, Item *c) :Item_temporal_func(a, b ,c) {} >> enum_field_types field_type() const { return MYSQL_TYPE_TIME; } >> + int save_in_field(Field *field, bool no_conversions) >> + { return save_time_in_field(field); } > > why? > >> }; >> >> === modified file 'sql/time.cc' >> --- sql/time.cc 2013-01-08 20:23:03 +0000 >> +++ sql/time.cc 2013-06-11 10:51:31 +0000 >> @@ -213,6 +213,20 @@ ulong convert_month_to_period(ulong mont >> } >> >> >> +bool >> +check_date_with_warn(const MYSQL_TIME *ltime, uint fuzzy_date) >> +{ >> + int dummy_warnings; >> + if (check_date(ltime, fuzzy_date, &dummy_warnings)) >> + { >> + Lazy_string_time str(ltime); >> + make_truncated_value_warning(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN, >> + &str, MYSQL_TIMESTAMP_ERROR, 0); > > please, use this function also in Item_date_typecast::get_date() and > may be in other places. you might need to pass MYSQL_TIMESTAMP_xxx > value as an argument, though. > >> + return true; >> + } >> + return false; >> +} >> + >> /* >> Convert a timestamp string to a MYSQL_TIME value and produce a warning >> if string was truncated during conversion. >> > > Regards, > Sergei >

1 0

[Maria-developers] I_S plugin doubts
by Roberto Spadim 13 Jun '13

13 Jun '13

Hi i was reading locale_info and qc_info source that i sent in jira changing the last parameter from 0 to SKIP_OPEN_TABLE, change the table from aria engine to memory engine, why? this change something internally in I_S table ? example: from static ST_FIELD_INFO qc_info_fields_queries[]= { {"ENTRY_POSITION_IN_CACHE", MY_INT32_NUM_DECIMAL_DIGITS, MYSQL_TYPE_LONG, 0, 0, 0, 0}, ... to static ST_FIELD_INFO qc_info_fields_queries[]= { {"ENTRY_POSITION_IN_CACHE", MY_INT32_NUM_DECIMAL_DIGITS, MYSQL_TYPE_LONG, 0, 0, 0, SKIP_OPEN_TABLE}, ... -- Roberto Spadim

2 5

Re: [Maria-developers] MDEV-4581 , more query cache information fields
by Roberto Spadim 12 Jun '13

12 Jun '13

hi sergei, i sent the patch, could you check it? i think that's easy to read and understand maybe some code standard should be done since i don't know how code in mariadb cs https://mariadb.atlassian.net/browse/MDEV-4581

1 0

[Maria-developers] MDEV-4581 , more query cache information fields
by Roberto Spadim 11 Jun '13

11 Jun '13

Hi guys, i sent a patch for qc_info plugin in MDEV-4581 Could mariadb team review and tell me what i should do, to put it in the next release? i will rewrite it, at 10.0.3 release, and change some things: the add to query to cache, i will change to only a function call at sql_cache.cc, and put the function at sql_cache.h something like: qc->qc_info_set_informations(insert time, rows_sent, rows_read, etc etc etc) rewrite the part of query hit. to execute only a command too qc->qc_info_hit(current_time, expend_qc_time, etc etc etc) this will make sql_cache.cc code cleaner and sql_cache.h "dirty" but will be more easly to read and patch future versions. only add 2 lines to sql_cache.cc , and many lines to sql_cache.h since sql_cache.h have the structure it will be easly to write function call at struct than sql_cache.cc file bye guys -- Roberto Spadim

1 0