MDEV-19961 MIN(timestamp_column) returns a wrong result in a GROUP BY query

mysql-test/main/timezone2.result

@@ -588,5 +588,27 @@ ts	cts	uts	ucts
 DROP TABLE t1,t2;
 SET time_zone=DEFAULT;
 #
+# MDEV-19961 MIN(timestamp_column) returns a wrong result in a GROUP BY query
+#
+SET time_zone='Europe/Moscow';
+CREATE OR REPLACE TABLE t1 (i INT, d TIMESTAMP);
+SET timestamp=1288477526  /* this is summer time */ ;
+INSERT INTO t1 VALUES (3,NULL);
+SET timestamp=1288477526+3599 /* this is winter time*/ ;
+INSERT INTO t1 VALUES (3,NULL);
+SELECT i, d, UNIX_TIMESTAMP(d) FROM t1 ORDER BY d;
+i	d	UNIX_TIMESTAMP(d)
+3	2010-10-31 02:25:26	1288477526
+3	2010-10-31 02:25:25	1288481125
+SELECT i, MIN(d) FROM t1 GROUP BY i;
+i	MIN(d)
+3	2010-10-31 02:25:26
+SELECT i, MAX(d) FROM t1 GROUP BY i;
+i	MAX(d)
+3	2010-10-31 02:25:25
+DROP TABLE t1;
+SET timestamp=DEFAULT;
+SET time_zone=DEFAULT;
+#
 # End of 10.4 tests
 #

mysql-test/main/timezone2.test

@@ -534,6 +534,24 @@ SELECT ts, cts, UNIX_TIMESTAMP(ts) AS uts, UNIX_TIMESTAMP(cts) AS ucts FROM t2;
 DROP TABLE t1,t2;
 SET time_zone=DEFAULT;
 
+--echo #
+--echo # MDEV-19961 MIN(timestamp_column) returns a wrong result in a GROUP BY query
+--echo #
+
+SET time_zone='Europe/Moscow';
+CREATE OR REPLACE TABLE t1 (i INT, d TIMESTAMP);
+SET timestamp=1288477526  /* this is summer time */ ;
+INSERT INTO t1 VALUES (3,NULL);
+SET timestamp=1288477526+3599 /* this is winter time*/ ;
+INSERT INTO t1 VALUES (3,NULL);
+SELECT i, d, UNIX_TIMESTAMP(d) FROM t1 ORDER BY d;
+SELECT i, MIN(d) FROM t1 GROUP BY i;
+SELECT i, MAX(d) FROM t1 GROUP BY i;
+DROP TABLE t1;
+SET timestamp=DEFAULT;
+SET time_zone=DEFAULT;
+
+
 --echo #
 --echo # End of 10.4 tests
 --echo #

sql/item_cmpfunc.h

@@ -132,6 +132,9 @@ class Arg_comparator: public Sql_alloc
   int compare_e_json_str();
   int compare_e_str_json();
 
+  void min_max_update_field_native(THD *thd, Field *field, Item *item,
+                                   int cmp_sign);
+
   Item** cache_converted_constant(THD *thd, Item **value, Item **cache,
                                   const Type_handler *type);
   inline bool is_owner_equal_func()

sql/item_sum.cc

@@ -3086,18 +3086,32 @@ void Item_sum_min_max::update_field()
     tmp_item= args[0];
     args[0]= direct_item;
   }
-  switch (result_type()) {
-  case STRING_RESULT:
-    min_max_update_str_field();
-    break;
-  case INT_RESULT:
-    min_max_update_int_field();
-    break;
-  case DECIMAL_RESULT:
-    min_max_update_decimal_field();
-    break;
-  default:
-    min_max_update_real_field();
+  if (Item_sum_min_max::type_handler()->is_val_native_ready())
+  {
+    /*
+      TODO-10.5: change Item_sum_min_max to use val_native() for all data types
+      - make all type handlers val_native() ready
+      - use min_max_update_native_field() for all data types
+      - remove Item_sum_min_max::min_max_update_{str|real|int|decimal}_field()
+    */
+    min_max_update_native_field();
+  }
+  else
+  {
+    switch (Item_sum_min_max::type_handler()->cmp_type()) {
+    case STRING_RESULT:
+    case TIME_RESULT:
+      min_max_update_str_field();
+      break;
+    case INT_RESULT:
+      min_max_update_int_field();
+      break;
+    case DECIMAL_RESULT:
+      min_max_update_decimal_field();
+      break;
+    default:
+      min_max_update_real_field();
+    }
   }
   if (unlikely(direct_added))
   {
@@ -3108,6 +3122,40 @@ void Item_sum_min_max::update_field()
 }
 
 
+void Arg_comparator::min_max_update_field_native(THD *thd,
+                                                 Field *field,
+                                                 Item *item,
+                                                 int cmp_sign)
+{
+  DBUG_ENTER("Arg_comparator::min_max_update_field_native");
+  if (!item->val_native(current_thd, &m_native2))
+  {
+    if (field->is_null())
+      field->store_native(m_native2); // The first non-null value
+    else
+    {
+      field->val_native(&m_native1);
+      if ((cmp_sign * m_compare_handler->cmp_native(m_native2, m_native1)) < 0)
+        field->store_native(m_native2);
+    }
+    field->set_notnull();
+  }
+  DBUG_VOID_RETURN;
+}
+
+
+void
+Item_sum_min_max::min_max_update_native_field()
+{
+  DBUG_ENTER("Item_sum_min_max::min_max_update_native_field");
+  DBUG_ASSERT(cmp);
+  DBUG_ASSERT(type_handler_for_comparison() == cmp->compare_type_handler());
+  THD *thd= current_thd;
+  cmp->min_max_update_field_native(thd, result_field, args[0], cmp_sign);
+  DBUG_VOID_RETURN;
+}
+
+
 void
 Item_sum_min_max::min_max_update_str_field()
 {

sql/item_sum.h

@@ -1117,6 +1117,7 @@ class Item_sum_min_max :public Item_sum_hybrid
   void min_max_update_real_field();
   void min_max_update_int_field();
   void min_max_update_decimal_field();
+  void min_max_update_native_field();
   void cleanup();
   bool any_value() { return was_values; }
   void no_rows_in_result();

sql/sql_type.h

@@ -3244,6 +3244,16 @@ class Type_handler
   {
     return MYSQL_TIMESTAMP_ERROR;
   }
+  /*
+    Return true if the native format is fully implemented for a data type:
+    - Field_xxx::val_native()
+    - Item_xxx::val_native() for all classes supporting this data type
+    - Type_handler_xxx::cmp_native()
+  */
+  virtual bool is_val_native_ready() const
+  {
+    return false;
+  }
   virtual bool is_timestamp_type() const
   {
     return false;
@@ -5582,6 +5592,10 @@ class Type_handler_timestamp_common: public Type_handler_temporal_with_date
   {
     return MYSQL_TIMESTAMP_DATETIME;
   }
+  bool is_val_native_ready() const
+  {
+    return true;
+  }
   bool is_timestamp_type() const
   {
     return true;