maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Sergei,


On 06/12/2015 01:00 PM, Sergei Golubchik wrote:
> Hi, Alexander!
>
> On Jun 11, Alexander Barkov wrote:
> > The problem is in this piece of the code:
> >
> > >    if (args[0]->get_time(&l_time1) ||
> > >        args[1]->get_time(&l_time2) ||
> > >        l_time1.time_type != l_time2.time_type)
> > >      return (null_value= 1);
> >
> > get_time() forces decimal-to-time conversion to truncate the date part
> > and return MYSQL_TIMESTAMP_TIME.
> >
> > get_time() does not force string-to-time conversion to truncate the date
> > part. It still returns the full MYSQL_TIMESTAMP_DATETIME.
> >
> > I'm afraid a lot of the code rely in this behavior.
> > If we fix string-to-time conversion, mtr will start to fail in other cases.
>
> First, this is clearly a bug that get_time behaves differently for
> numbers and strings. One could get different results for
>
>    WHERE time_column = 'string'
>
> and
>
>    WHERE time_column = number
>
> Something needs to be fixed. We cannot fix string-to-time conversion to
> return MYSQL_TIMESTAMP_TIME in all cases. When a string literal is
> parsed we don't always know in advance whether it's a datetime or a
> time, so we can only indicate *preference* based on the context
> (preference matters, as it tells how to parse ambiguous strings like
> "10:10:10").

I'd say '10:10:10' should be unambiguously treated as time.
Colon is never used to delimit date parts. Is it?

Date parts are usually delimited by as follows:
'01-01-01'
'01.01.01'
'01/01/01'

But this is a kind of separate issue. Would you like me to create a task 
for this?
In other cases it could still use the *preference* flag:
'01#01#01' - could be either date or time, depending on the flag.


> So, I've fixed number_to_time to keep the date part:

For some reasons I even didn't consider fixing number_to_time() as an 
option.

Thanks. This fixes the bug. I pushed this change with a test added.


> --- a/sql-common/my_time.c
> +++ b/sql-common/my_time.c
> @@ -1319,9 +1319,6 @@ int number_to_time(my_bool neg, ulonglong nr, ulong sec_pa
>                              TIME_INVALID_DATES, was_cut) < 0)
>         return -1;
>
> -    ltime->year= ltime->month= ltime->day= 0;
> -    ltime->time_type= MYSQL_TIMESTAMP_TIME;
> -    *was_cut= MYSQL_TIME_NOTE_TRUNCATED;
>       return 0;
>     }
>
> The only test that changed results was type_time_hires, and I think the
> change was ok.
>
> Regards,
> Sergei