- developers - lists.mariadb.org

[Maria-developers] Jira new field
by Roberto Spadim 17 Jun '13

17 Jun '13

hi guys, could we add a Reporter Label inside jira issue? instead of only Label (used by mariadb team) it's easier to filter issues, thinking about the reporter filter not for mariadb team filters -- Roberto Spadim

1 0

[Maria-developers] Is it possible to export last seen GTID from IO thread?
by Pavel Ivanov 15 Jun '13

15 Jun '13

Kristian, We had an idea that can make master failovers much easier, please could you tell us how feasible to do that in MariaDB. Currently when master dies and failover is necessary we have to wait while SQL thread catches up with IO thread on all replicas before we'll be able to decide which replica should be new master. Otherwise we can loose some transactions. But if IO thread could export some variable indicating which GTID it's seen last then we'll be able to understand which replica is furthest in replication stream faster, without waiting. Of course for multi-domain replication it will be more complicated and IO thread will have to synthesize gtid_slave_pos which server would have after applying all relay logs. But exporting this synthesized position will still help a lot. So what do you think? Is it possible to do this in MariaDB? Thank you, Pavel

1 0

Re: [Maria-developers] Process suggestion for minor issues
by Elena Stepanova 14 Jun '13

14 Jun '13

(warning: it's a long one) Hi Sergei, On 6/14/2013 6:26 PM, Sergei Golubchik wrote: > Hi, Elena! > > On Jun 14, Elena Stepanova wrote: >> On 6/14/2013 2:27 PM, Rasmus Johansson wrote: >>> 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". >> >> I don't think the algorithm is quite fair. It means that if a really >> minor bug happens to be found during a relatively slow period, it gets >> fixed, but another bug, even considerably more important (but still >> falling into "not major => minor" category), can be by this time closed >> as "not worth fixing" just because people had been busy with some other >> stuff and couldn't fix it over two previous releases; while logically, >> if somebody suddenly has time to fix a minor bug, the latter should be >> fixed first. > > This algorithm has a "re-reviewed and either priority changed to > major..." step, so it's not just the timing. Not like automatic closing > of old bugs. Yes, I got that part; but it doesn't change the potential faulty result. Lets say we have imaginary priorities 1-5 which are all "minor", 1 being the lowest. For a while you had had a bug of "priority 4". After 2 releases the reviewing force reviewed it, decided it was minor (rightly so), and closed it. A few days later you got a bug of "priority 1". A few more days later you suddenly have time to fix a bug -- you are stuck at your main task and need a distraction, or you've got a headache and can't work on anything serious, or you only have half an hour before you need to go pick up kids... whatever. Naturally, you go to the list of your open bugs, take a quick look and pick whatever fits. If you still had the bug of priority 4, you'd fix it now, but instead you fix the bug of priority 1. I don't know about others, but I do keep a bunch of open low-priority tasks, and while rarely, sometimes it happens that I can do something about a minor one. And then I look at them and pick by importance and "suitability for the moment", not by the creation time. > >> I'm fine with closing bugs as "Won't Fix", but imho it should be based >> on the merits of the bug itself, not on the bad timing of its >> creation. Since all our developers are very senior and can totally >> make such a decision on their own, I suppose anyone who a bug is >> assigned to can take a look at it and either close it as "Won't fix" >> with a proper explanation, regardless the timing (even right after >> receiving it), or keep it open. > > The main issue I didn't like - what that algorithm is making an attempt > of solving - is a growing pile of minor bugs that we aren't going to > fix. I agree, there is no point at keeping those that we are really not going to fix. But I don't see anything critically bad at preserving those that we *want* to fix, just currently don't have time to -- as long as we don't schedule them for the closest release, thus lying to users and to ourselves (more on that later). I also think it's two essentially different categories. Later here you are saying that "a closed bug can always be reopened later and fixed". If we put the "don't have resources to fix" bugs into the same trash bin as we currently use for "really won't fix" bugs, re-opening will never happen. Who will go through the whole bunch of "Won't fix" bugs, review them *again* and see if they are really not worth fixing, or if we had to close them because we didn't have resources? Certainly you can't do it when you just have little time to fix one-two small bugs, you'll spend more time searching. > > A bug is real, so it's difficult to say "won't fix", it's much easier, > psychologically, to say, "okay, I'll fix it, *when I have time*". But > really, for minor bugs this might never happen. But sooner or later somebody who's reviewing them will dare close it anyway?.. Okay, I agree that psychologically it is easier to reject a bug *together with somebody* than make such a decision on your own, group mentality and all that, I get it and have it myself; so if it's all about having some kind of committee for closing such bugs, let it be. (I don't really know who could this committee consist of. Generally, I'm a big supporter of a triage team, it makes the process less personal. But in a company where everyone is pretty much unique in his area, it's a bit strange, because at the end only the expert can bring in an educated verdict -- and this would normally be the person the bug is already assigned to anyway; others will just be there to hold his hand.) > And de-facto this leads to wrong expectations (users believe the bug > will be fixed in a specific version, but it isn't) and us lying to our > users. True, but I think the new approach will mess up more than it will fix. Now we have Fix version evolution like that: 5.5.30 => 5.5.31 => 5.5.32 => 5.5.33 => .... It's bad. I still think that we shouldn't schedule minor bugs for any particular version. We've discussed it before, and yes, I know that we do it to keep them on somebody's radar, but I would think that having it assigned to somebody should be enough, since it's already on a radar of the person who it's assigned to. But maybe I'm wrong and nobody looks at lists of bugs assigned to them -- shouldn't we try to change it instead, then? But with the new system, we will have 5.5.30 => 5.5.31 => "Won't fix, it isn't worth it". If I were a user, now I'd be mad. If it isn't worth fixing, why didn't you say so right away? Why schedule it and even move it to the next release then? That's even more of lying to users than we currently do, and in a more irritating way. > My idea is to be honest about it, and if we don't have resources to fix > a specific bug, we won't drag it forever from one release to another, > but close it eventually. I agree. But I think we shouldn't even start dragging it then. If you are going to have a committee which would regularly review bugs (those that have waited for 2 releases) and decide if they're worth fixing or not, why not do it right away -- let the same committee decide it for *new* bugs instead, so that unimportant ones don't even get scheduled and go to the trash bin immediately. It will spare users wrong expectations. > And yes, I hate closing non-fixed bugs too. > > If you can suggest another solution, please do. But "keep it open" > doesn't work, as we've seen already. Well, I'd suggest to consider adjusting yours a little, in the following way: - define who will be reviewing the bugs; - those person(s) will screen new bugs instead of those that have waited for 2 releases; - , set "Won't fix" when we really mean it (example: PBXT bugs, etc.), and create a new resolution "Don't have time to fix" or "Wishlist" or whatever, and put corresponding bugs there, while keeping them assigned. The last point is to preserve at least hypothetical possibility for the bugs to be re-opened later. So, if somebody has time to fix a minor bug, first he looks at the list of his open bugs (we already know that the committee kept only most worthy ones there). If nothing fits the moment, or (a miracle!) the list is empty, the somebody goes to the "wishlist" bugs assigned to him, and picks from there. At the same time, those bugs aren't shown as open, aren't scheduled for any particular release, the users aren't misled, etc. A couple more notes: - I think external bugs should generally have higher priority than otherwise equal ones found internally; - a different approach might be needed for bugs in new feature trees, it should be decided separately (the basic question is -- when we consider the feature good enough to be pushed to the main tree, whether it requires fixing minor bugs or not). /E > > Besides, a closed bug can always be reopened later and fixed. > > Regards, > Sergei >

2 1

[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