MDEV-11485 Split Item_func_between::val_int() into virtual methods in Type_handler

- Removes "Item_result Item_func_opt_neg::m_compare_type" and introduces
  "Type_handler_hybrid_field_type Item_func_opt_neg::m_comparator" instead.

- Removes Item_func_between::compare_as_dates, because
  the new member m_comparator now contains the precise information
  about the data type that is used for comparison, which is important
  for TIME vs DATETIME.

- Adds a new method:
  Type_handler_hybrid_field_type::aggregate_for_comparison(const Type_handler*),
  as a better replacement for item_cmp_type(), which additionally can handle
  TIME vs DATE/DATETIME/TIMESTAMP correctly. Additionally, it correctly
  handles TIMESTAMP which fixes the problem reported in MDEV-11482.
  The old compare_as_dates/find_date_time_item() based code didn't handle
  comparison between TIME and TIMESTAMP correctly and erroneously used TIME
  comparison instead of DATETIME comparison.

- Adds a new method:
  Type_handler_hybrid_field_type::aggregate_for_comparison(Item **, uint nitems),
  as a better replacement for agg_cmp_type(), which can handle TIME.
- Splits Item_func_between::val_int() into pieces val_int_cmp_xxx(),
  one new method per XXX_RESULT.
- Adds a new virtual method Type_handler::Item_func_between_val_int()
  whose implementations use Item_func_between::val_int_cmp_xxx().
- Makes type_handler_longlong and type_handler_newdecimal public,
  as they are now needed in item_cmpfunc.cc.

Note:
This patch does not change Item_func_in to use the new aggregation methods,
so it still uses collect_cmp_type()/item_cmp_type() based aggregation.
Item_func_in will be changed in a separate patch and item_cmp_type() will be
removed.

mysql-test/r/type_timestamp.result

@@ -980,5 +980,17 @@ Warnings:
 Warning	1441	Datetime function: datetime field overflow
 DROP TABLE t1;
 #
+# MDEV-11482 Incorrect result for (time_expr BETWEEN timestamp_exp1 AND timestamp_expr2)
+#
+SET @@sql_mode=DEFAULT;
+SET @@timestamp=UNIX_TIMESTAMP('2001-01-01 10:20:30');
+CREATE TABLE t1 (a TIMESTAMP,b TIMESTAMP);
+INSERT INTO t1 VALUES ('2001-01-01 00:00:00','2001-01-01 23:59:59');
+SELECT * FROM t1 WHERE TIME'10:20:30' BETWEEN a and b;
+a	b
+2001-01-01 00:00:00	2001-01-01 23:59:59
+DROP TABLE t1;
+SET @@timestamp=DEFAULT;
+#
 # End of 10.3 tests
 #

mysql-test/t/type_timestamp.test

@@ -576,6 +576,17 @@ EXPLAIN SELECT * FROM t1 WHERE a >= DATE_ADD(TIMESTAMP'9999-01-01 00:00:00',INTE
 EXPLAIN SELECT * FROM t1 WHERE a >= COALESCE(DATE_ADD(TIMESTAMP'9999-01-01 00:00:00',INTERVAL 1000 YEAR));
 DROP TABLE t1;
 
+--echo #
+--echo # MDEV-11482 Incorrect result for (time_expr BETWEEN timestamp_exp1 AND timestamp_expr2)
+--echo #
+SET @@sql_mode=DEFAULT;
+SET @@timestamp=UNIX_TIMESTAMP('2001-01-01 10:20:30');
+CREATE TABLE t1 (a TIMESTAMP,b TIMESTAMP);
+INSERT INTO t1 VALUES ('2001-01-01 00:00:00','2001-01-01 23:59:59');
+SELECT * FROM t1 WHERE TIME'10:20:30' BETWEEN a and b;
+DROP TABLE t1;
+SET @@timestamp=DEFAULT;
+
 --echo #
 --echo # End of 10.3 tests
 --echo #

sql/item_cmpfunc.h

@@ -834,10 +834,10 @@ class Item_func_opt_neg :public Item_bool_func
 {
 protected:
   /*
-    The result type that will be used for comparison.
-    cmp_type() of all arguments are collected to here.
+    The data type handler that will be used for comparison.
+    Data type handlers of all arguments are mixed to here.
   */
-  Item_result m_compare_type;
+  Type_handler_hybrid_field_type m_comparator;
   /*
     The collation that will be used for comparison in case
     when m_compare_type is STRING_RESULT.
@@ -872,11 +872,13 @@ class Item_func_between :public Item_func_opt_neg
                              Field *field, Item *value);
 public:
   String value0,value1,value2;
-  /* TRUE <=> arguments will be compared as dates. */
-  Item *compare_as_dates;
   Item_func_between(THD *thd, Item *a, Item *b, Item *c):
-    Item_func_opt_neg(thd, a, b, c), compare_as_dates(FALSE) { }
-  longlong val_int();
+    Item_func_opt_neg(thd, a, b, c) { }
+  longlong val_int()
+  {
+    DBUG_ASSERT(fixed);
+    return m_comparator.type_handler()->Item_func_between_val_int(this);
+  }
   enum Functype functype() const   { return BETWEEN; }
   const char *func_name() const { return "between"; }
   enum precedence precedence() const { return BETWEEN_PRECEDENCE; }
@@ -893,13 +895,19 @@ class Item_func_between :public Item_func_opt_neg
   {
     Item_args::propagate_equal_fields(thd,
                                       Context(ANY_SUBST,
-                                              m_compare_type,
+                                              m_comparator.cmp_type(),
                                               compare_collation()),
                                       cond);
     return this;
   }
   Item *get_copy(THD *thd, MEM_ROOT *mem_root)
   { return get_item_copy<Item_func_between>(thd, mem_root, this); }
+
+  longlong val_int_cmp_string();
+  longlong val_int_cmp_temporal();
+  longlong val_int_cmp_int();
+  longlong val_int_cmp_real();
+  longlong val_int_cmp_decimal();
 };
 
 
@@ -1664,7 +1672,7 @@ class Item_func_in :public Item_func_opt_neg
       will be replaced to a zero-filled Item_string.
       Such a change would require rebuilding of cmp_items.
     */
-    Context cmpctx(ANY_SUBST, m_compare_type,
+    Context cmpctx(ANY_SUBST, m_comparator.cmp_type(),
                    Item_func_in::compare_collation());
     for (uint i= 0; i < arg_count; i++)
     {

sql/sql_type.cc

@@ -23,7 +23,6 @@
 static Type_handler_tiny        type_handler_tiny;
 static Type_handler_short       type_handler_short;
 static Type_handler_long        type_handler_long;
-static Type_handler_longlong    type_handler_longlong;
 static Type_handler_int24       type_handler_int24;
 static Type_handler_year        type_handler_year;
 static Type_handler_bit         type_handler_bit;
@@ -38,7 +37,6 @@ static Type_handler_datetime2   type_handler_datetime2;
 static Type_handler_timestamp   type_handler_timestamp;
 static Type_handler_timestamp2  type_handler_timestamp2;
 static Type_handler_olddecimal  type_handler_olddecimal;
-static Type_handler_newdecimal  type_handler_newdecimal;
 static Type_handler_string      type_handler_string;
 static Type_handler_tiny_blob   type_handler_tiny_blob;
 static Type_handler_medium_blob type_handler_medium_blob;
@@ -54,6 +52,8 @@ static Type_handler_set         type_handler_set;
 Type_handler_null        type_handler_null;
 Type_handler_row         type_handler_row;
 Type_handler_varchar     type_handler_varchar;
+Type_handler_longlong    type_handler_longlong;
+Type_handler_newdecimal  type_handler_newdecimal;
 
 
 /**
@@ -123,6 +123,55 @@ Type_handler_hybrid_field_type::Type_handler_hybrid_field_type()
 }
 
 
+/*
+  Collect built-in data type handlers for comparison.
+  This method is very similar to item_cmp_type() defined in item.cc.
+  Now they coexist. Later item_cmp_type() will be removed.
+  In addition to item_cmp_type(), this method correctly aggregates
+  TIME with DATETIME/TIMESTAMP/DATE, so no additional find_date_time_item()
+  is needed after this call.
+*/
+void
+Type_handler_hybrid_field_type::aggregate_for_comparison(const Type_handler *h)
+{
+  Item_result a= cmp_type();
+  Item_result b= h->cmp_type();
+  if (a == STRING_RESULT && b == STRING_RESULT)
+    m_type_handler= &type_handler_long_blob;
+  else if (a == INT_RESULT && b == INT_RESULT)
+    m_type_handler= &type_handler_longlong;
+  else if (a == ROW_RESULT || b == ROW_RESULT)
+    m_type_handler= &type_handler_row;
+  else if (a == TIME_RESULT || b == TIME_RESULT)
+  {
+    if ((a == TIME_RESULT) + (b == TIME_RESULT) == 1)
+    {
+      /*
+        We're here if there's only one temporal data type:
+        either m_type_handler or h.
+      */
+      if (b == TIME_RESULT)
+        m_type_handler= h; // Temporal types bit non-temporal types
+    }
+    else
+    {
+      /*
+        We're here if both m_type_handler and h are temporal data types.
+      */
+      if (field_type() != MYSQL_TYPE_TIME || h->field_type() != MYSQL_TYPE_TIME)
+        m_type_handler= &type_handler_datetime; // DATETIME bits TIME
+    }
+  }
+  else if ((a == INT_RESULT || a == DECIMAL_RESULT) &&
+           (b == INT_RESULT || b == DECIMAL_RESULT))
+  {
+    m_type_handler= &type_handler_newdecimal;
+  }
+  else
+    m_type_handler= &type_handler_double;
+}
+
+
 const Type_handler *
 Type_handler::get_handler_by_field_type(enum_field_types type)
 {
@@ -1141,3 +1190,43 @@ Type_handler_string_result::Item_func_hybrid_field_type_get_date(
 }
 
 /***************************************************************************/
+
+longlong Type_handler_row::
+           Item_func_between_val_int(Item_func_between *func) const
+{
+  DBUG_ASSERT(0);
+  func->null_value= true;
+  return 0;
+}
+
+longlong Type_handler_string_result::
+           Item_func_between_val_int(Item_func_between *func) const
+{
+  return func->val_int_cmp_string();
+}
+
+longlong Type_handler_temporal_result::
+           Item_func_between_val_int(Item_func_between *func) const
+{
+  return func->val_int_cmp_temporal();
+}
+
+longlong Type_handler_int_result::
+           Item_func_between_val_int(Item_func_between *func) const
+{
+  return func->val_int_cmp_int();
+}
+
+longlong Type_handler_real_result::
+           Item_func_between_val_int(Item_func_between *func) const
+{
+  return func->val_int_cmp_real();
+}
+
+longlong Type_handler_decimal_result::
+           Item_func_between_val_int(Item_func_between *func) const
+{
+  return func->val_int_cmp_decimal();
+}
+
+/***************************************************************************/

sql/sql_type.h

@@ -29,6 +29,7 @@ class Item_cache;
 class Item_sum_hybrid;
 class Item_func_hex;
 class Item_func_hybrid_field_type;
+class Item_func_between;
 class Type_std_attributes;
 class Sort_param;
 class Arg_comparator;
@@ -314,6 +315,8 @@ class Type_handler
                                             MYSQL_TIME *,
                                             ulonglong fuzzydate) const= 0;
 
+  virtual longlong
+  Item_func_between_val_int(Item_func_between *func) const= 0;
 };
 
 
@@ -410,6 +413,7 @@ class Type_handler_row: public Type_handler
     return true;
   }
 
+  longlong Item_func_between_val_int(Item_func_between *func) const;
 };
 
 
@@ -457,6 +461,7 @@ class Type_handler_real_result: public Type_handler_numeric
   bool Item_func_hybrid_field_type_get_date(Item_func_hybrid_field_type *,
                                             MYSQL_TIME *,
                                             ulonglong fuzzydate) const;
+  longlong Item_func_between_val_int(Item_func_between *func) const;
 };
 
 
@@ -489,6 +494,7 @@ class Type_handler_decimal_result: public Type_handler_numeric
   bool Item_func_hybrid_field_type_get_date(Item_func_hybrid_field_type *,
                                             MYSQL_TIME *,
                                             ulonglong fuzzydate) const;
+  longlong Item_func_between_val_int(Item_func_between *func) const;
 };
 
 
@@ -521,6 +527,7 @@ class Type_handler_int_result: public Type_handler_numeric
   bool Item_func_hybrid_field_type_get_date(Item_func_hybrid_field_type *,
                                             MYSQL_TIME *,
                                             ulonglong fuzzydate) const;
+  longlong Item_func_between_val_int(Item_func_between *func) const;
 };
 
 
@@ -551,6 +558,7 @@ class Type_handler_temporal_result: public Type_handler
   bool Item_func_hybrid_field_type_get_date(Item_func_hybrid_field_type *,
                                             MYSQL_TIME *,
                                             ulonglong fuzzydate) const;
+  longlong Item_func_between_val_int(Item_func_between *func) const;
 };
 
 
@@ -585,6 +593,7 @@ class Type_handler_string_result: public Type_handler
   bool Item_func_hybrid_field_type_get_date(Item_func_hybrid_field_type *,
                                             MYSQL_TIME *,
                                             ulonglong fuzzydate) const;
+  longlong Item_func_between_val_int(Item_func_between *func) const;
 };
 
 
@@ -960,6 +969,10 @@ class Type_handler_hybrid_field_type
   {
     return (m_type_handler= Type_handler::get_handler_by_result_type(type));
   }
+  const Type_handler *set_handler_by_cmp_type(Item_result type)
+  {
+    return (m_type_handler= Type_handler::get_handler_by_cmp_type(type));
+  }
   const Type_handler *set_handler_by_result_type(Item_result type,
                                                  uint max_octet_length,
                                                  CHARSET_INFO *cs)
@@ -977,6 +990,8 @@ class Type_handler_hybrid_field_type
   {
     return (m_type_handler= Type_handler::get_handler_by_real_type(type));
   }
+  void aggregate_for_comparison(const Type_handler *other);
+  bool aggregate_for_comparison(Item **items, uint nitems);
 };
 
 
@@ -997,5 +1012,7 @@ class Type_handler_hybrid_real_field_type:
 extern Type_handler_row   type_handler_row;
 extern Type_handler_null  type_handler_null;
 extern Type_handler_varchar type_handler_varchar;
+extern Type_handler_longlong type_handler_longlong;
+extern Type_handler_newdecimal type_handler_newdecimal;
 
 #endif /* SQL_TYPE_H_INCLUDED */