MDEV-16190 Server crashes in Item_null_result::field_type on SELECT with time...

MDEV-16190 Server crashes in Item_null_result::field_type on SELECT with time field, ROLLUP and HAVING

virtual Item_null_result::get_date() was not overridden.
It used the inherited Item::get_date(), which tests field_type(),
which in case of Item_null_result calls result_field->field_type(),
and result_field is not really always set (e.g. it's not set in the
test case from the bug report).

Overriding Item_null::get_date() like it's done for other val_xxx() methods.
This make the code more symmetric across data types.

In the new reduction, get_date() immediately returns NULL without entering
into any data type specific code.

mysql-test/r/olap.result

@@ -766,3 +766,25 @@ b
 NULL
 DROP TABLE t1, t2;
 End of 5.0 tests
+#
+# Start of 10.1 tests
+#
+#
+# MDEV-16190 Server crashes in Item_null_result::field_type on SELECT with time field, ROLLUP and HAVING
+#
+CREATE TABLE t1 (t TIME) ENGINE=MyISAM;
+INSERT INTO t1 VALUES ('12:12:12');
+SELECT t, COUNT(*) FROM t1 GROUP BY t WITH ROLLUP HAVING t > '00:00:00';
+t	COUNT(*)
+12:12:12	1
+DROP TABLE t1;
+CREATE TABLE t1 (t TIME) ENGINE=MyISAM;
+INSERT INTO t1 VALUES ('12:12:12'),('12:12:13');
+SELECT t, COUNT(*) FROM t1 GROUP BY t WITH ROLLUP HAVING t > '00:00:00';
+t	COUNT(*)
+12:12:12	1
+12:12:13	1
+DROP TABLE t1;
+#
+# End of 10.1 tests
+#

mysql-test/t/olap.test

@@ -404,3 +404,26 @@ SELECT DISTINCT b FROM t1, t2 GROUP BY a, b WITH ROLLUP;
 DROP TABLE t1, t2;
 
 --echo End of 5.0 tests
+
+
+--echo #
+--echo # Start of 10.1 tests
+--echo #
+
+--echo #
+--echo # MDEV-16190 Server crashes in Item_null_result::field_type on SELECT with time field, ROLLUP and HAVING
+--echo #
+CREATE TABLE t1 (t TIME) ENGINE=MyISAM;
+INSERT INTO t1 VALUES ('12:12:12');
+SELECT t, COUNT(*) FROM t1 GROUP BY t WITH ROLLUP HAVING t > '00:00:00';
+DROP TABLE t1;
+
+CREATE TABLE t1 (t TIME) ENGINE=MyISAM;
+INSERT INTO t1 VALUES ('12:12:12'),('12:12:13');
+SELECT t, COUNT(*) FROM t1 GROUP BY t WITH ROLLUP HAVING t > '00:00:00';
+DROP TABLE t1;
+
+
+--echo #
+--echo # End of 10.1 tests
+--echo #

sql/item.cc

@@ -1287,7 +1287,7 @@ bool Item::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate)
                                                 ltime, fuzzydate,
                                                 field_name_or_null()))
       goto err;
-    break;
+    return null_value= false;
   }
   case REAL_RESULT:
   {
@@ -1295,7 +1295,7 @@ bool Item::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate)
     if (null_value || double_to_datetime_with_warn(value, ltime, fuzzydate,
                                                    field_name_or_null()))
       goto err;
-    break;
+    return null_value= false;
   }
   case DECIMAL_RESULT:
   {
@@ -1304,7 +1304,7 @@ bool Item::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate)
         decimal_to_datetime_with_warn(res, ltime, fuzzydate,
                                       field_name_or_null()))
       goto err;
-    break;
+    return null_value= false;
   }
   case STRING_RESULT:
   {
@@ -1314,15 +1314,20 @@ bool Item::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate)
         str_to_datetime_with_warn(res->charset(), res->ptr(), res->length(),
                                   ltime, fuzzydate))
       goto err;
-    break;
+    return null_value= false;
   }
   default:
+    null_value= true;
     DBUG_ASSERT(0);
   }
 
-  return null_value= 0;
-
 err:
+  return null_value|= make_zero_date(ltime, fuzzydate);
+}
+
+
+bool Item::make_zero_date(MYSQL_TIME *ltime, ulonglong fuzzydate)
+{
   /*
     if the item was not null and convertion failed, we return a zero date
     if allowed, otherwise - null.
@@ -1344,7 +1349,7 @@ bool Item::get_date(MYSQL_TIME *ltime,ulonglong fuzzydate)
     */
     ltime->time_type= MYSQL_TIMESTAMP_TIME;
   }
-  return null_value|= !(fuzzydate & TIME_FUZZY_DATES);
+  return !(fuzzydate & TIME_FUZZY_DATES);
 }
 
 bool Item::get_seconds(ulonglong *sec, ulong *sec_part)
@@ -3132,6 +3137,15 @@ my_decimal *Item_null::val_decimal(my_decimal *decimal_value)
 }
 
 
+bool Item_null::get_date(MYSQL_TIME *ltime, ulonglong fuzzydate)
+{
+  // following assert is redundant, because fixed=1 assigned in constructor
+  DBUG_ASSERT(fixed == 1);
+  make_zero_date(ltime, fuzzydate);
+  return (null_value= true);
+}
+
+
 Item *Item_null::safe_charset_converter(THD *thd, CHARSET_INFO *tocs)
 {
   return this;

sql/item.h

@@ -663,6 +663,12 @@ class Item: public Value_source,
   SEL_TREE *get_mm_tree_for_const(RANGE_OPT_PARAM *param);
 
   Field *create_tmp_field(bool group, TABLE *table, uint convert_int_length);
+  /*
+    This method is used if the item was not null but convertion to
+    TIME/DATE/DATETIME failed. We return a zero date if allowed,
+    otherwise - null.
+  */
+  bool make_zero_date(MYSQL_TIME *ltime, ulonglong fuzzydate);
 
 public:
   /*
@@ -2565,6 +2571,7 @@ class Item_null :public Item_basic_constant
   longlong val_int();
   String *val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
+  bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate);
   int save_in_field(Field *field, bool no_conversions);
   int save_safe_in_field(Field *field);
   bool send(Protocol *protocol, String *str);