MDEV-23525 Wrong result of MIN(time_expr) and MAX(time_expr) with GROUP BY

Problem:

When calculatung MIN() and MAX() in a query with GROUP BY, like this:

  SELECT MIN(time_expr), MAX(time_expr) FROM t1 GROUP BY i;

the code in Item_sum_min_max::update_field() erroneosly used
string format comparison, therefore '100:20:30' was considered as
smaller than '10:20:30'.

Fix:

1. Implementing low level "native" related methods in class Time:
     Time::Time(const Native &native)           - convert native to Time
     Time::to_native(Native *to, uint decimals) - convert Time to native

   The "native" binary representation for TIME is equal to
   the binary data format of Field_timef, which is used to
   store TIME when mysql56_temporal_format is ON (default).

2. Implementing Type_handler_time_common "native" related methods:

  Type_handler_time_common::cmp_native()
  Type_handler_time_common::Item_val_native_with_conversion()
  Type_handler_time_common::Item_val_native_with_conversion_result()
  Type_handler_time_common::Item_param_val_native()

3. Implementing missing "native representation" related methods
   in Field_time and Field_timef:

  Field_time::store_native()
  Field_time::val_native()
  Field_timef::store_native()
  Field_timef::val_native()

4. Implementing missing "native" related methods in all Items
   that can have the TIME data type:

  Item_timefunc::val_native()
  Item_name_const::val_native()
  Item_time_literal::val_native()
  Item_cache_time::val_native()
  Item_handled_func::val_native()

5. Marking Type_handler_time_common as "native ready".
   So now Item_sum_min_max::update_field() calculates
   values using min_max_update_native_field(),
   which uses native binary representation rather than string representation.

   Before this change, only the TIMESTAMP data type used native
   representation to calculate MIN() and MAX().

Benchmarks (see more details in MDEV):

  This change not only fixes the wrong result, but also
  makes a "SELECT .. MAX.. GROUP BY .." query faster:

  # TIME(0)
  CREATE TABLE t1 (id INT, time_col TIME) ENGINE=HEAP;
  INSERT INTO t1 VALUES (1,'10:10:10'); -- repeat this 1m times
  SELECT id, MAX(time_col) FROM t1 GROUP BY id;

  MySQL80: 0.159 sec
  10.3:    0.108 sec
  10.4:    0.094 sec (fixed)

  # TIME(6):
  CREATE TABLE t1 (id INT, time_col TIME(6)) ENGINE=HEAP;
  INSERT INTO t1 VALUES (1,'10:10:10.999999'); -- repeat this 1m times
  SELECT id, MAX(time_col) FROM t1 GROUP BY id;

  My80: 0.154
  10.3: 0.135
  10.4: 0.093 (fixed)

1 files changed, 100 insertions, 0 deletions

diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index 6763de17085..85052a1c1bc 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -628,6 +628,23 @@ uint Interval_DDhhmmssff::fsp(THD *thd, Item *item)
 }
 
 
+bool Time::to_native(Native *to, uint decimals) const
+{
+  if (!is_valid_time())
+  {
+    to->length(0);
+    return true;
+  }
+  uint len= my_time_binary_length(decimals);
+  if (to->reserve(len))
+    return true;
+  longlong tmp= TIME_to_longlong_time_packed(get_mysql_time());
+  my_time_packed_to_binary(tmp, (uchar*) to->ptr(), decimals);
+  to->length(len);
+  return false;
+}
+
+
 void Time::make_from_item(THD *thd, int *warn, Item *item, const Options opt)
 {
   *warn= 0;
@@ -812,6 +829,28 @@ void Time::make_from_time(int *warn, const MYSQL_TIME *from)
 }
 
 
+uint Time::binary_length_to_precision(uint length)
+{
+  switch (length) {
+  case 3: return 0;
+  case 4: return 2;
+  case 5: return 4;
+  case 6: return 6;
+  }
+  DBUG_ASSERT(0);
+  return 0;
+}
+
+
+Time::Time(const Native &native)
+{
+  uint dec= binary_length_to_precision(native.length());
+  longlong tmp= my_time_packed_from_binary((const uchar *) native.ptr(), dec);
+  TIME_from_longlong_time_packed(this, tmp);
+  DBUG_ASSERT(is_valid_time());
+}
+
+
 Time::Time(int *warn, const MYSQL_TIME *from, long curdays)
 {
   switch (from->time_type) {
@@ -1456,6 +1495,13 @@ Type_handler_timestamp_common::type_handler_for_native_format() const
 }
 
 
+const Type_handler *
+Type_handler_time_common::type_handler_for_native_format() const
+{
+  return &type_handler_time2;
+}
+
+
 /***************************************************************************/
 
 const Type_handler *Type_handler_typelib::type_handler_for_item_field() const
@@ -8391,6 +8437,51 @@ Type_handler_time_common::create_literal_item(THD *thd,
 }
 
 
+bool
+Type_handler_time_common::Item_val_native_with_conversion(THD *thd,
+                                                          Item *item,
+                                                          Native *to) const
+{
+  if (item->type_handler()->type_handler_for_native_format() ==
+      &type_handler_time2)
+    return item->val_native(thd, to);
+  return Time(thd, item).to_native(to, item->time_precision(thd));
+}
+
+
+bool
+Type_handler_time_common::Item_val_native_with_conversion_result(THD *thd,
+                                                                 Item *item,
+                                                                 Native *to)
+                                                                 const
+{
+  if (item->type_handler()->type_handler_for_native_format() ==
+      &type_handler_time2)
+    return item->val_native_result(thd, to);
+  MYSQL_TIME ltime;
+  if (item->get_date_result(thd, &ltime, Time::Options(thd)))
+    return true;
+  int warn;
+  return Time(&warn, &ltime, 0).to_native(to, item->time_precision(thd));
+}
+
+
+int Type_handler_time_common::cmp_native(const Native &a,
+                                         const Native &b) const
+{
+  // Optimize a simple case: equal fractional precision:
+  if (a.length() == b.length())
+    return memcmp(a.ptr(), b.ptr(), a.length());
+  longlong lla= Time(a).to_packed();
+  longlong llb= Time(b).to_packed();
+  if (lla < llb)
+    return -1;
+  if (lla> llb)
+    return 1;
+  return 0;
+}
+
+
 bool Type_handler_timestamp_common::TIME_to_native(THD *thd,
                                                    const MYSQL_TIME *ltime,
                                                    Native *to,
@@ -8501,6 +8592,15 @@ Type_handler_timestamp_common::Item_param_val_native(THD *thd,
 }
 
 
+bool
+Type_handler_time_common::Item_param_val_native(THD *thd,
+                                                Item_param *item,
+                                                Native *to) const
+{
+  return Time(thd, item).to_native(to, item->decimals);
+}
+
+
 LEX_CSTRING Charset::collation_specific_name() const
 {
   /*