← Back to team overview

maria-developers team mailing list archive

Re: Sprint 10.0: MDEV-8205 timediff returns null when comparing decimal time to time string value

 

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").

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

--- 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


Follow ups

References