A cleanup for MDEV-15340 + fix MDEV-15363 Wrong result for CAST(LAST_DAY(TIME'00:00:00') AS TIME)

The change N7 in MDEV-15340 (see the commit message) introduced
a regression in how CAST(AS TIME), HOUR(), TIME_TO_SEC() treat datetimes
'0000-00-DD mm:hh:ss' (i.e. with zero YYYYMM part and a non-zero day).
These functions historically do not mix days to hours on datetime-to-time
conversion. Implementations of the underlying methods used get_arg0_time()
to fetch MYSQL_TIME. After MDEV-15340, get_arg0_time() went through the
Time() constructor, which always adds '0000-00-DD' to hours automatically
(as in all other places in the code we do mix days to hours).

Changes:
1. Extending Time() to make it possible to choose a desired way of treating
   '0000-00-DD' (ignore or mix to hours) on datetime-to-time conversion.
   Adding a helper class Time::Options for this, which now describes two aspects
   of Time() creation:
   1. Flags for get_date()
   2. Days/hours mixing behavior.

2. Removing Item_func::get_arg0_time(). Using Time() directly
   in all affected classes. Forcing Time() to ignore (rather than mix)
   '0000-00-DD' in these affected classes by passing a suitable Options value.

3. Adding Time::to_seconds(), to reuse the code between
   Item_func_time_to_sec::decimal_op() and Item_func_time_to_sec::int_op().

4. Item_func::get_arg0_date() now returns only a datetime value,
   with automatic time-to-datetime conversion if needed. An assert was
   added to catch attempts to pass TIME_TIME_ONLY to get_arg0_date().
   All callers were checked not to pass TIME_TIME_ONLY, this revealed
   a bug MDEV-15363.

5. Changing Item_func_last_day::get_date() to remove the TIME_TIME_ONLY flag
   before calling get_arg0_date(). This fixes MDEV-15363.

mysql-test/r/func_time.result

@@ -3382,4 +3382,23 @@ ADDTIME(TIME'10:20:30', DATE'2001-01-01') AS c3,
 ADDTIME(TIME'10:20:30', COALESCE(DATE'2001-01-01',TIMESTAMP'2001-01-01 00:00:00')) AS c4;
 c1	c2	c3	c4
 NULL	NULL	NULL	NULL
+SELECT
+HOUR(TIMESTAMP'0000-00-01 10:00:00') AS h0,
+TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00') AS tts0,
+TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00.1') AS tts1,
+CAST(TIMESTAMP'0000-00-01 10:00:00' AS TIME) AS c0,
+CAST(TIMESTAMP'0000-00-01 10:00:00.1' AS TIME(1)) AS c2;
+h0	tts0	tts1	c0	c2
+10	36000	36000.1	10:00:00	10:00:00.1
+SET TIMESTAMP=DEFAULT;
+#
+# MDEV-15363 Wrong result for CAST(LAST_DAY(TIME'00:00:00') AS TIME)
+#
+SET TIMESTAMP=UNIX_TIMESTAMP('2018-02-17 01:02:03');
+SELECT
+LAST_DAY(TIME'00:00:00') AS c1,
+CAST(CAST(LAST_DAY(TIME'00:00:00') AS DATE) AS TIME) AS c2,
+CAST(LAST_DAY(TIME'00:00:00') AS TIME) AS c3;
+c1	c2	c3
+2018-02-28	00:00:00	00:00:00
 SET TIMESTAMP=DEFAULT;

mysql-test/t/func_time.test

@@ -1974,4 +1974,27 @@ SELECT
   ADDTIME(TIME'10:20:30', DATE'2001-01-01') AS c3,
   ADDTIME(TIME'10:20:30', COALESCE(DATE'2001-01-01',TIMESTAMP'2001-01-01 00:00:00')) AS c4;
 
+#
+# Make sure that time functions that in 10.2 used get_arg0_time()
+# do not mix days to hours for dates with zero YYYYMM and non-zero days.
+#
+
+SELECT
+  HOUR(TIMESTAMP'0000-00-01 10:00:00') AS h0,
+  TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00') AS tts0,
+  TIME_TO_SEC(TIMESTAMP'0000-00-01 10:00:00.1') AS tts1,
+  CAST(TIMESTAMP'0000-00-01 10:00:00' AS TIME) AS c0,
+  CAST(TIMESTAMP'0000-00-01 10:00:00.1' AS TIME(1)) AS c2;
+
+SET TIMESTAMP=DEFAULT;
+
+--echo #
+--echo # MDEV-15363 Wrong result for CAST(LAST_DAY(TIME'00:00:00') AS TIME)
+--echo #
+
+SET TIMESTAMP=UNIX_TIMESTAMP('2018-02-17 01:02:03');
+SELECT
+  LAST_DAY(TIME'00:00:00') AS c1,
+  CAST(CAST(LAST_DAY(TIME'00:00:00') AS DATE) AS TIME) AS c2,
+  CAST(LAST_DAY(TIME'00:00:00') AS TIME) AS c3;
 SET TIMESTAMP=DEFAULT;

sql/item_func.h

@@ -160,18 +160,10 @@ public:
   void print_args(String *str, uint from, enum_query_type query_type);
   inline bool get_arg0_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
   {
-    return fuzzy_date & TIME_TIME_ONLY ? get_arg0_time(ltime) :
+    DBUG_ASSERT(!(fuzzy_date & TIME_TIME_ONLY));
-                                         get_arg0_datetime(ltime, fuzzy_date);
-  }
-  inline bool get_arg0_datetime(MYSQL_TIME *ltime, ulonglong fuzzy_date)
-  {
     Datetime dt(current_thd, args[0], fuzzy_date);
     return (null_value= dt.copy_to_mysql_time(ltime));
   }
-  inline bool get_arg0_time(MYSQL_TIME *ltime)
-  {
-    return (null_value= Time(args[0]).copy_to_mysql_time(ltime));
-  }
   bool is_null() { 
     update_null_value();
     return null_value; 

sql/item_timefunc.cc

@@ -993,15 +993,15 @@ longlong Item_func_quarter::val_int()
 longlong Item_func_hour::val_int()
 {
   DBUG_ASSERT(fixed == 1);
-  MYSQL_TIME ltime;
+  Time tm(args[0], Time::Options_for_cast());
-  return get_arg0_time(&ltime) ? 0 : ltime.hour;
+  return (null_value= !tm.is_valid_time()) ? 0 : tm.get_mysql_time()->hour;
 }
 
 longlong Item_func_minute::val_int()
 {
   DBUG_ASSERT(fixed == 1);
-  MYSQL_TIME ltime;
+  Time tm(args[0], Time::Options_for_cast());
-  return get_arg0_time(&ltime) ? 0 : ltime.minute;
+  return (null_value= !tm.is_valid_time()) ? 0 : tm.get_mysql_time()->minute;
 }
 
 /**
@@ -1010,8 +1010,8 @@ longlong Item_func_minute::val_int()
 longlong Item_func_second::val_int()
 {
   DBUG_ASSERT(fixed == 1);
-  MYSQL_TIME ltime;
+  Time tm(args[0], Time::Options_for_cast());
-  return get_arg0_time(&ltime) ? 0 : ltime.second;
+  return (null_value= !tm.is_valid_time()) ? 0 : tm.get_mysql_time()->second;
 }
 
 
@@ -1267,24 +1267,20 @@ longlong Item_func_unix_timestamp::val_int_endpoint(bool left_endp, bool *incl_e
 longlong Item_func_time_to_sec::int_op()
 {
   DBUG_ASSERT(fixed == 1);
-  MYSQL_TIME ltime;
+  Time tm(args[0], Time::Options_for_cast());
-  if (get_arg0_time(&ltime))
+  return ((null_value= !tm.is_valid_time())) ? 0 : tm.to_seconds();
-    return 0;
-
-  longlong seconds=ltime.hour*3600L+ltime.minute*60+ltime.second;
-  return ltime.neg ? -seconds : seconds;
 }
 
 
 my_decimal *Item_func_time_to_sec::decimal_op(my_decimal* buf)
 {
   DBUG_ASSERT(fixed == 1);
-  MYSQL_TIME ltime;
+  Time tm(args[0], Time::Options_for_cast());
-  if (get_arg0_time(&ltime))
+  if ((null_value= !tm.is_valid_time()))
     return 0;
-
+  const MYSQL_TIME *ltime= tm.get_mysql_time();
-  longlong seconds= ltime.hour*3600L+ltime.minute*60+ltime.second;
+  longlong seconds= tm.to_seconds_abs();
-  return seconds2my_decimal(ltime.neg, seconds, ltime.second_part, buf);
+  return seconds2my_decimal(ltime->neg, seconds, ltime->second_part, buf);
 }
 
 
@@ -1991,7 +1987,7 @@ String *Item_func_date_format::val_str(String *str)
   const MY_LOCALE *lc= 0;
   DBUG_ASSERT(fixed == 1);
   
-  if (get_arg0_date(&l_time, is_time_format ? TIME_TIME_ONLY : 0))
+  if ((null_value= args[0]->get_date(&l_time, is_time_format ? TIME_TIME_ONLY : 0)))
     return 0;
   
   if (!(format = args[1]->val_str(str)) || !format->length())
@@ -2607,17 +2603,12 @@ void Item_char_typecast::fix_length_and_dec_internal(CHARSET_INFO *from_cs)
 
 bool Item_time_typecast::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
 {
-  if (get_arg0_time(ltime))
+  Time tm(args[0], Time::Options_for_cast());
-    return 1;
+  if ((null_value= !tm.is_valid_time()))
+    return true;
+  tm.copy_to_mysql_time(ltime);
   if (decimals < TIME_SECOND_PART_DIGITS)
     my_time_trunc(ltime, decimals);
-  /*
-    MYSQL_TIMESTAMP_TIME value can have non-zero day part,
-    which we should not lose.
-  */
-  if (ltime->time_type != MYSQL_TIMESTAMP_TIME)
-    ltime->year= ltime->month= ltime->day= 0;
-  ltime->time_type= MYSQL_TIMESTAMP_TIME;
   return (fuzzy_date & TIME_TIME_ONLY) ? 0 :
          (null_value= check_date_with_warn(ltime, fuzzy_date,
                                            MYSQL_TIMESTAMP_ERROR)); 
@@ -2936,10 +2927,9 @@ bool Item_func_maketime::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
 longlong Item_func_microsecond::val_int()
 {
   DBUG_ASSERT(fixed == 1);
-  MYSQL_TIME ltime;
+  Time tm(args[0], Time::Options_for_cast());
-  if (!get_arg0_date(&ltime, TIME_TIME_ONLY))
+  return ((null_value= !tm.is_valid_time())) ?
-    return ltime.second_part;
+         0 : tm.get_mysql_time()->second_part;
-  return 0;
 }
 
 
@@ -3322,7 +3312,7 @@ bool Item_func_str_to_date::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
 
 bool Item_func_last_day::get_date(MYSQL_TIME *ltime, ulonglong fuzzy_date)
 {
-  if (get_arg0_date(ltime, fuzzy_date) ||
+  if (get_arg0_date(ltime, fuzzy_date & ~TIME_TIME_ONLY) ||
       (ltime->month == 0))
     return (null_value=1);
   uint month_idx= ltime->month-1;

sql/sql_type.cc

@@ -121,12 +121,12 @@ bool Type_handler_data::init()
 Type_handler_data *type_handler_data= NULL;
 
 
-void Time::make_from_item(Item *item, sql_mode_t flags)
+void Time::make_from_item(Item *item, const Options opt)
 {
-  if (item->get_date(this, flags))
+  if (item->get_date(this, opt.get_date_flags()))
     time_type= MYSQL_TIMESTAMP_NONE;
   else
-    valid_MYSQL_TIME_to_valid_value();
+    valid_MYSQL_TIME_to_valid_value(opt);
 }
 
 

sql/sql_type.h

@@ -94,6 +94,47 @@ struct SORT_FIELD_ATTR;
 */
 class Time: private MYSQL_TIME
 {
+public:
+  enum datetime_to_time_mode_t
+  {
+    DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS,
+    DATETIME_TO_TIME_YYYYMMDD_TRUNCATE
+  };
+  class Options
+  {
+    sql_mode_t              m_get_date_flags;
+    datetime_to_time_mode_t m_datetime_to_time_mode;
+  public:
+    Options()
+     :m_get_date_flags(flags_for_get_date()),
+      m_datetime_to_time_mode(DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS)
+    { }
+    Options(sql_mode_t flags)
+     :m_get_date_flags(flags),
+      m_datetime_to_time_mode(DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS)
+    { }
+    Options(sql_mode_t flags, datetime_to_time_mode_t dtmode)
+     :m_get_date_flags(flags),
+      m_datetime_to_time_mode(dtmode)
+    { }
+    sql_mode_t get_date_flags() const
+    { return m_get_date_flags; }
+    datetime_to_time_mode_t datetime_to_time_mode() const
+    { return m_datetime_to_time_mode; }
+  };
+  /*
+    CAST(AS TIME) historically does not mix days to hours.
+    This is different comparing to how implicit conversion
+    in Field::store_time_dec() works (e.g. on INSERT).
+  */
+  class Options_for_cast: public Options
+  {
+  public:
+    Options_for_cast()
+     :Options(flags_for_get_date(), DATETIME_TO_TIME_YYYYMMDD_TRUNCATE)
+    { }
+  };
+private:
   bool is_valid_value_slow() const
   {
     return time_type == MYSQL_TIMESTAMP_NONE || is_valid_time_slow();
@@ -113,7 +154,7 @@ class Time: private MYSQL_TIME
     e.g. returned from Item::get_date().
     After this call, "this" is a valid TIME value.
   */
-  void valid_datetime_to_valid_time()
+  void valid_datetime_to_valid_time(const Options opt)
   {
     DBUG_ASSERT(time_type == MYSQL_TIMESTAMP_DATE ||
                 time_type == MYSQL_TIMESTAMP_DATETIME);
@@ -123,7 +164,9 @@ class Time: private MYSQL_TIME
     */
     DBUG_ASSERT(day < 32);
     DBUG_ASSERT(hour < 24);
-    if (year == 0 && month == 0)
+    if (year == 0 && month == 0 &&
+        opt.datetime_to_time_mode() ==
+        DATETIME_TO_TIME_YYYYMMDD_000000DD_MIX_TO_HOURS)
     {
       /*
         The maximum hour value after mixing days will be 31*24+23=767,
@@ -148,12 +191,12 @@ class Time: private MYSQL_TIME
     - either a valid TIME (within the supported TIME range),
     - or MYSQL_TIMESTAMP_NONE
   */
-  void valid_MYSQL_TIME_to_valid_value()
+  void valid_MYSQL_TIME_to_valid_value(const Options opt)
   {
     switch (time_type) {
     case MYSQL_TIMESTAMP_DATE:
     case MYSQL_TIMESTAMP_DATETIME:
-      valid_datetime_to_valid_time();
+      valid_datetime_to_valid_time(opt);
       break;
     case MYSQL_TIMESTAMP_NONE:
       break;
@@ -165,11 +208,11 @@ class Time: private MYSQL_TIME
       break;
     }
   }
-  void make_from_item(class Item *item, sql_mode_t flags);
+  void make_from_item(class Item *item, const Options opt);
 public:
   Time() { time_type= MYSQL_TIMESTAMP_NONE; }
-  Time(Item *item) { make_from_item(item, flags_for_get_date()); }
+  Time(Item *item) { make_from_item(item, Options()); }
-  Time(Item *item, sql_mode_t flags) { make_from_item(item, flags); }
+  Time(Item *item, const Options opt) { make_from_item(item, opt); }
   static sql_mode_t flags_for_get_date()
   { return TIME_TIME_ONLY | TIME_INVALID_DATES; }
   static sql_mode_t comparison_flags_for_get_date()
@@ -207,6 +250,15 @@ public:
       return 1;
     return 0;
   }
+  longlong to_seconds_abs() const
+  {
+    DBUG_ASSERT(is_valid_time_slow());
+    return hour * 3600L + minute * 60 + second;
+  }
+  longlong to_seconds() const
+  {
+    return neg ? -to_seconds_abs() : to_seconds_abs();
+  }
 };