maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hello Sergei,

Please review a patch for MDEV-13997.

It's needed for:
MDEV-13995 MAX(timestamp) returns a wrong result near DST change
and for:
MDEV-4912 Add a plugin to field types (column types)

Thanks!

commit 55eb354cbbeef818d739c69a700528decacd963d
Author: Alexander Barkov <bar@xxxxxxxxxxx>
Date:   Wed Oct 4 15:12:36 2017 +0400

    MDEV-13997 Change Item_bool_rowready_func2 to cache const items at fix time rather than evaluation time
    
    Side effect: the second debug Note in cache_temporal_4265.result disappeared.
    
    Before this change:
    - During JOIN::cache_const_exprs(),
      Item::get_cache() for Item_date_add_interval() was called.
      The data type for date_add('2001-01-01',interval 5 day) is VARCHAR,
      because the first argument is VARCHAR (not temporal).
      Item_get_cache() created Item_cache_str('2001-01-06').
    - During evaluate_join_record(), get_datetime_value() was called,
      which called Item::get_date() for Item_cache_str('2001-01-06').
      This gave the second Note. Then, get_datetime_value() created
      a new cache, now Item_cache_temporal for '2001-01-06', so not
      further str_to_datetime() happened.
    
    After this change:
    - During tem_bool_rowready_func2::fix_length_and_dec(),
      Arg_comparator::set_cmp_func_datetime() is called,
      which immediately creates an instance of Item_cache_date for
      the result of date_add('2001-01-01',interval 5 day).
      So later no str_to_datetime happens any more,
      neither during JOIN::cache_const_exprs(),
      nor during evaluate_join_record().

diff --git a/mysql-test/r/cache_temporal_4265.result b/mysql-test/r/cache_temporal_4265.result
index 980bb95..7f215de 100644
--- a/mysql-test/r/cache_temporal_4265.result
+++ b/mysql-test/r/cache_temporal_4265.result
@@ -7,7 +7,6 @@ a
 2002-03-04
 Warnings:
 Note	1003	2000-01-01
-Note	1003	2000-01-06
 set debug_dbug='';
 drop table t1;
 create table t1 (id int not null, ut timestamp(6) not null);
diff --git a/sql/item.cc b/sql/item.cc
index f609bfd..992215f 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -109,6 +109,19 @@ void Item::push_note_converted_to_positive_complement(THD *thd)
 }
 
 
+longlong Item::val_datetime_packed_result()
+{
+  MYSQL_TIME ltime, tmp;
+  if (get_date_result(&ltime, TIME_FUZZY_DATES | TIME_INVALID_DATES))
+    return 0;
+  if (ltime.time_type != MYSQL_TIMESTAMP_TIME)
+    return pack_time(&ltime);
+  if ((null_value= time_to_datetime_with_warn(current_thd, &ltime, &tmp, 0)))
+    return 0;
+  return pack_time(&tmp);
+}
+
+
 /**
   Get date/time/datetime.
   Optionally extend TIME result to DATETIME.
@@ -9678,14 +9691,19 @@ bool  Item_cache_temporal::cache_value()
 {
   if (!example)
     return false;
+  value_cached= true;
+  value= example->val_datetime_packed_result();
+  null_value= example->null_value;
+  return true;
+}
+
 
+bool Item_cache_time::cache_value()
+{
+  if (!example)
+    return false;
   value_cached= true;
- 
-  MYSQL_TIME ltime;
-  if (example->get_date_result(&ltime, 0))
-    value=0;
-  else
-    value= pack_time(&ltime);
+  value= example->val_time_packed_result();
   null_value= example->null_value;
   return true;
 }
@@ -9736,8 +9754,8 @@ void Item_cache_temporal::store_packed(longlong val_arg, Item *example_arg)
 
 Item *Item_cache_temporal::clone_item(THD *thd)
 {
-  Item_cache_temporal *item= new (thd->mem_root)
-    Item_cache_temporal(thd, Item_cache_temporal::type_handler());
+  Item_cache *tmp= type_handler()->Item_get_cache(thd, this);
+  Item_cache_temporal *item= static_cast<Item_cache_temporal*>(tmp);
   item->store_packed(value, example);
   return item;
 }
diff --git a/sql/item.h b/sql/item.h
index a926ee9..73dee2f 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -1397,6 +1397,14 @@ class Item: public Value_source,
     ulonglong fuzzydate= TIME_FUZZY_DATES | TIME_INVALID_DATES | TIME_TIME_ONLY;
     return get_date(&ltime, fuzzydate) ? 0 : pack_time(&ltime);
   }
+  longlong val_datetime_packed_result();
+  longlong val_time_packed_result()
+  {
+    MYSQL_TIME ltime;
+    ulonglong fuzzydate= TIME_FUZZY_DATES | TIME_INVALID_DATES | TIME_TIME_ONLY;
+    return get_date_result(&ltime, fuzzydate) ? 0 : pack_time(&ltime);
+  }
+
   // Get a temporal value in packed DATE/DATETIME or TIME format
   longlong val_temporal_packed(enum_field_types f_type)
   {
@@ -5782,8 +5790,9 @@ class Item_cache_int: public Item_cache
 
 class Item_cache_temporal: public Item_cache_int
 {
-public:
+protected:
   Item_cache_temporal(THD *thd, const Type_handler *handler);
+public:
   String* val_str(String *str);
   my_decimal *val_decimal(my_decimal *);
   longlong val_int();
@@ -5801,8 +5810,37 @@ class Item_cache_temporal: public Item_cache_int
   */
   Item *clone_item(THD *thd);
   Item *convert_to_basic_const_item(THD *thd);
+};
+
+
+class Item_cache_time: public Item_cache_temporal
+{
+public:
+  Item_cache_time(THD *thd)
+   :Item_cache_temporal(thd, &type_handler_time2) { }
+  bool cache_value();
+  Item *get_copy(THD *thd, MEM_ROOT *mem_root)
+  { return get_item_copy<Item_cache_time>(thd, mem_root, this); }
+};
+
+
+class Item_cache_datetime: public Item_cache_temporal
+{
+public:
+  Item_cache_datetime(THD *thd)
+   :Item_cache_temporal(thd, &type_handler_datetime2) { }
+  Item *get_copy(THD *thd, MEM_ROOT *mem_root)
+  { return get_item_copy<Item_cache_datetime>(thd, mem_root, this); }
+};
+
+
+class Item_cache_date: public Item_cache_temporal
+{
+public:
+  Item_cache_date(THD *thd)
+   :Item_cache_temporal(thd, &type_handler_newdate) { }
   Item *get_copy(THD *thd, MEM_ROOT *mem_root)
-  { return get_item_copy<Item_cache_temporal>(thd, mem_root, this); }
+  { return get_item_copy<Item_cache_date>(thd, mem_root, this); }
 };
 
 
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 9954c66..90b150a 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -569,18 +569,24 @@ bool Arg_comparator::set_cmp_func_string()
 
 bool Arg_comparator::set_cmp_func_time()
 {
+  THD *thd= current_thd;
   m_compare_collation= &my_charset_numeric;
   func= is_owner_equal_func() ? &Arg_comparator::compare_e_time :
                                 &Arg_comparator::compare_time;
+  a= cache_converted_constant(thd, a, &a_cache, compare_type_handler());
+  b= cache_converted_constant(thd, b, &b_cache, compare_type_handler());
   return false;
 }
 
 
 bool Arg_comparator::set_cmp_func_datetime()
 {
+  THD *thd= current_thd;
   m_compare_collation= &my_charset_numeric;
   func= is_owner_equal_func() ? &Arg_comparator::compare_e_datetime :
                                 &Arg_comparator::compare_datetime;
+  a= cache_converted_constant(thd, a, &a_cache, compare_type_handler());
+  b= cache_converted_constant(thd, b, &b_cache, compare_type_handler());
   return false;
 }
 
@@ -683,17 +689,12 @@ Item** Arg_comparator::cache_converted_constant(THD *thd_arg, Item **value,
                                                 const Type_handler *handler)
 {
   /*
-    get_datetime_value creates Item_cache internally when comparing
-    values for the first row.
-    Arg_comparator::cache_converted_constant() is never called for TIME_RESULT.
-  */
-  DBUG_ASSERT(handler->cmp_type() != TIME_RESULT);
-  /*
     Don't need cache if doing context analysis only.
   */
   if (!thd_arg->lex->is_ps_or_view_context_analysis() &&
       (*value)->const_item() &&
-      handler->cmp_type() != (*value)->cmp_type())
+      handler->type_handler_for_comparison() !=
+      (*value)->type_handler_for_comparison())
   {
     Item_cache *cache= handler->Item_get_cache(thd_arg, *value);
     cache->setup(thd_arg, *value);
@@ -750,7 +751,8 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg,
     if (!thd)
       thd= current_thd;
     const Type_handler *h= Type_handler::get_handler_by_field_type(f_type);
-    Item_cache_temporal *cache= new (thd->mem_root) Item_cache_temporal(thd, h);
+    Item_cache *tmp_cache= h->Item_get_cache(thd, item);
+    Item_cache_temporal *cache= static_cast<Item_cache_temporal*>(tmp_cache);
     cache->store_packed(value, item);
     *cache_arg= cache;
     *item_arg= cache_arg;
@@ -759,63 +761,57 @@ get_datetime_value(THD *thd, Item ***item_arg, Item **cache_arg,
 }
 
 
-/*
-  Compare items values as dates.
-
-  SYNOPSIS
-    Arg_comparator::compare_datetime()
-
-  DESCRIPTION
-    Compare items values as DATE/DATETIME for both EQUAL_FUNC and from other
-    comparison functions. The correct DATETIME values are obtained
-    with help of the get_datetime_value() function.
-
-  RETURN
-      -1   a < b or at least one item is null
-       0   a == b
-       1   a > b
-*/
-
-int Arg_comparator::compare_temporal(enum_field_types type)
+int Arg_comparator::compare_time()
 {
-  bool a_is_null, b_is_null;
-  longlong a_value, b_value;
-
+  longlong val1= (*a)->val_time_packed();
+  if (!(*a)->null_value)
+  {
+    longlong val2= (*b)->val_time_packed();
+    if (!(*b)->null_value)
+      return compare_not_null_values(val1, val2);
+  }
   if (set_null)
-    owner->null_value= 1;
+    owner->null_value= true;
+  return -1;
+}
 
-  /* Get DATE/DATETIME/TIME value of the 'a' item. */
-  a_value= get_datetime_value(0, &a, &a_cache, type, &a_is_null);
-  if (a_is_null)
-    return -1;
 
-  /* Get DATE/DATETIME/TIME value of the 'b' item. */
-  b_value= get_datetime_value(0, &b, &b_cache, type, &b_is_null);
-  if (b_is_null)
-    return -1;
+int Arg_comparator::compare_e_time()
+{
+  longlong val1= (*a)->val_time_packed();
+  longlong val2= (*b)->val_time_packed();
+  if ((*a)->null_value || (*b)->null_value)
+    return MY_TEST((*a)->null_value && (*b)->null_value);
+  return MY_TEST(val1 == val2);
+}
 
-  /* Here we have two not-NULL values. */
-  if (set_null)
-    owner->null_value= 0;
 
-  /* Compare values. */
-  return a_value < b_value ? -1 : a_value > b_value ? 1 : 0;
-}
 
-int Arg_comparator::compare_e_temporal(enum_field_types type)
+int Arg_comparator::compare_datetime()
 {
-  bool a_is_null, b_is_null;
-  longlong a_value, b_value;
+  longlong val1= (*a)->val_datetime_packed();
+  if (!(*a)->null_value)
+  {
+    longlong val2= (*b)->val_datetime_packed();
+    if (!(*b)->null_value)
+      return compare_not_null_values(val1, val2);
+  }
+  if (set_null)
+    owner->null_value= true;
+  return -1;
+}
 
-  /* Get DATE/DATETIME/TIME value of the 'a' item. */
-  a_value= get_datetime_value(0, &a, &a_cache, type, &a_is_null);
 
-  /* Get DATE/DATETIME/TIME value of the 'b' item. */
-  b_value= get_datetime_value(0, &b, &b_cache, type, &b_is_null);
-  return a_is_null || b_is_null ? a_is_null == b_is_null
-                                : a_value == b_value;
+int Arg_comparator::compare_e_datetime()
+{
+  longlong val1= (*a)->val_datetime_packed();
+  longlong val2= (*b)->val_datetime_packed();
+  if ((*a)->null_value || (*b)->null_value)
+    return MY_TEST((*a)->null_value && (*b)->null_value);
+  return MY_TEST(val1 == val2);
 }
 
+
 int Arg_comparator::compare_string()
 {
   String *res1,*res2;
@@ -962,13 +958,7 @@ int Arg_comparator::compare_int_signed()
   {
     longlong val2= (*b)->val_int();
     if (!(*b)->null_value)
-    {
-      if (set_null)
-        owner->null_value= 0;
-      if (val1 < val2)	return -1;
-      if (val1 == val2)   return 0;
-      return 1;
-    }
+      return compare_not_null_values(val1, val2);
   }
   if (set_null)
     owner->null_value= 1;
@@ -2112,38 +2102,40 @@ bool Item_func_between::fix_length_and_dec_numeric(THD *thd)
 }
 
 
-longlong Item_func_between::val_int_cmp_temporal()
+bool Item_func_between::fix_length_and_dec_temporal(THD *thd)
 {
-  THD *thd= current_thd;
-  longlong value, a, b;
-  Item *cache, **ptr;
-  bool value_is_null, a_is_null, b_is_null;
+  if (!thd->lex->is_ps_or_view_context_analysis())
+  {
+    for (uint i= 0; i < 3; i ++)
+    {
+      if (args[i]->const_item() &&
+          args[i]->type_handler_for_comparison() != m_comparator.type_handler())
+      {
+        Item_cache *cache= m_comparator.type_handler()->Item_get_cache(thd, args[i]);
+        if (!cache || cache->setup(thd, args[i]))
+          return true;
+        thd->change_item_tree(&args[i], cache);
+      }
+    }
+  }
+  return false;
+}
 
-  ptr= &args[0];
-  enum_field_types f_type= m_comparator.type_handler()->field_type();
-  value= get_datetime_value(thd, &ptr, &cache, f_type, &value_is_null);
-  if (ptr != &args[0])
-    thd->change_item_tree(&args[0], *ptr);
 
-  if ((null_value= value_is_null))
+longlong Item_func_between::val_int_cmp_temporal()
+{
+  enum_field_types f_type= m_comparator.type_handler()->field_type();
+  longlong value= args[0]->val_temporal_packed(f_type), a, b;
+  if ((null_value= args[0]->null_value))
     return 0;
-
-  ptr= &args[1];
-  a= get_datetime_value(thd, &ptr, &cache, f_type, &a_is_null);
-  if (ptr != &args[1])
-    thd->change_item_tree(&args[1], *ptr);
-
-  ptr= &args[2];
-  b= get_datetime_value(thd, &ptr, &cache, f_type, &b_is_null);
-  if (ptr != &args[2])
-    thd->change_item_tree(&args[2], *ptr);
-
-  if (!a_is_null && !b_is_null)
+  a= args[1]->val_temporal_packed(f_type);
+  b= args[2]->val_temporal_packed(f_type);
+  if (!args[1]->null_value && !args[2]->null_value)
     return (longlong) ((value >= a && value <= b) != negated);
-  if (a_is_null && b_is_null)
+  if (args[1]->null_value && args[2]->null_value)
     null_value= true;
-  else if (a_is_null)
-    null_value= value <= b;			// not null if false range.
+  else if (args[1]->null_value)
+    null_value= value <= b;                    // not null if false range.
   else
     null_value= value >= a;
   return (longlong) (!null_value && negated);
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index f086260..62066ef 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -60,9 +60,14 @@ class Arg_comparator: public Sql_alloc
 
   int set_cmp_func(Item_func_or_sum *owner_arg, Item **a1, Item **a2);
 
-  int compare_temporal(enum_field_types type);
-  int compare_e_temporal(enum_field_types type);
-
+  int compare_not_null_values(longlong val1, longlong val2)
+  {
+    if (set_null)
+      owner->null_value= false;
+    if (val1 < val2) return -1;
+    if (val1 == val2) return 0;
+    return 1;
+  }
 public:
   /* Allow owner function to use string buffers. */
   String value1, value2;
@@ -112,10 +117,10 @@ class Arg_comparator: public Sql_alloc
   int compare_e_row();           // compare args[0] & args[1]
   int compare_real_fixed();
   int compare_e_real_fixed();
-  int compare_datetime()   { return compare_temporal(MYSQL_TYPE_DATETIME); }
-  int compare_e_datetime() { return compare_e_temporal(MYSQL_TYPE_DATETIME); }
-  int compare_time()       { return compare_temporal(MYSQL_TYPE_TIME); }
-  int compare_e_time()     { return compare_e_temporal(MYSQL_TYPE_TIME); }
+  int compare_datetime();
+  int compare_e_datetime();
+  int compare_time();
+  int compare_e_time();
   int compare_json_str_basic(Item *j, Item *s);
   int compare_json_str();
   int compare_str_json();
@@ -899,6 +904,7 @@ class Item_func_between :public Item_func_opt_neg
   {
     return agg_arg_charsets_for_comparison(cmp_collation, args, 3);
   }
+  bool fix_length_and_dec_temporal(THD *);
   bool fix_length_and_dec_numeric(THD *);
   virtual void print(String *str, enum_query_type query_type);
   bool eval_not_null_tables(void *opt_arg);
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index 737d01f..66c6e14 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -2650,9 +2650,27 @@ Type_handler_string_result::Item_get_cache(THD *thd, const Item *item) const
 }
 
 Item_cache *
-Type_handler_temporal_result::Item_get_cache(THD *thd, const Item *item) const
+Type_handler_timestamp_common::Item_get_cache(THD *thd, const Item *item) const
 {
-  return new (thd->mem_root) Item_cache_temporal(thd, item->type_handler());
+  return new (thd->mem_root) Item_cache_datetime(thd);
+}
+
+Item_cache *
+Type_handler_datetime_common::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_datetime(thd);
+}
+
+Item_cache *
+Type_handler_time_common::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_time(thd);
+}
+
+Item_cache *
+Type_handler_date_common::Item_get_cache(THD *thd, const Item *item) const
+{
+  return new (thd->mem_root) Item_cache_date(thd);
 }
 
 /*************************************************************************/
@@ -3494,7 +3512,7 @@ bool Type_handler_numeric::
 bool Type_handler_temporal_result::
        Item_func_between_fix_length_and_dec(Item_func_between *func) const
 {
-  return func->fix_length_and_dec_numeric(current_thd);
+  return func->fix_length_and_dec_temporal(current_thd);
 }
 
 bool Type_handler_string_result::
@@ -5222,7 +5240,7 @@ Item *Type_handler_time_common::
   longlong value= item->val_time_packed();
   if (item->null_value)
     return new (thd->mem_root) Item_null(thd, item->name.str);
-  cache= new (thd->mem_root) Item_cache_temporal(thd, this);
+  cache= new (thd->mem_root) Item_cache_time(thd);
   if (cache)
     cache->store_packed(value, item);
   return cache;
@@ -5236,7 +5254,7 @@ Item *Type_handler_temporal_with_date::
   longlong value= item->val_datetime_packed();
   if (item->null_value)
     return new (thd->mem_root) Item_null(thd, item->name.str);
-  cache= new (thd->mem_root) Item_cache_temporal(thd, this);
+  cache= new (thd->mem_root) Item_cache_datetime(thd);
   if (cache)
     cache->store_packed(value, item);
   return cache;
diff --git a/sql/sql_type.h b/sql/sql_type.h
index 9101721..029b52e 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -1584,7 +1584,6 @@ class Type_handler_temporal_result: public Type_handler
                                    Item *source_expr, Item *source_const) const;
   bool subquery_type_allows_materialization(const Item *inner,
                                             const Item *outer) const;
-  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool Item_sum_hybrid_fix_length_and_dec(Item_sum_hybrid *func) const;
   bool Item_sum_sum_fix_length_and_dec(Item_sum_sum *) const;
   bool Item_sum_avg_fix_length_and_dec(Item_sum_avg *) const;
@@ -2066,6 +2065,7 @@ class Type_handler_time_common: public Type_handler_temporal_result
   }
   int Item_save_in_field(Item *item, Field *field, bool no_conversions) const;
   String *print_item_value(THD *thd, Item *item, String *str) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool Item_hybrid_func_fix_attributes(THD *thd,
                                        const char *name,
                                        Type_handler_hybrid_field_type *,
@@ -2152,6 +2152,7 @@ class Type_handler_date_common: public Type_handler_temporal_with_date
   bool Column_definition_fix_attributes(Column_definition *c) const;
   uint Item_decimal_precision(const Item *item) const;
   String *print_item_value(THD *thd, Item *item, String *str) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool Item_hybrid_func_fix_attributes(THD *thd,
                                        const char *name,
                                        Type_handler_hybrid_field_type *,
@@ -2225,6 +2226,7 @@ class Type_handler_datetime_common: public Type_handler_temporal_with_date
     return Item_send_datetime(item, protocol, buf);
   }
   String *print_item_value(THD *thd, Item *item, String *str) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool Item_hybrid_func_fix_attributes(THD *thd,
                                        const char *name,
                                        Type_handler_hybrid_field_type *,
@@ -2304,6 +2306,7 @@ class Type_handler_timestamp_common: public Type_handler_temporal_with_date
     return Item_send_datetime(item, protocol, buf);
   }
   String *print_item_value(THD *thd, Item *item, String *str) const;
+  Item_cache *Item_get_cache(THD *thd, const Item *item) const;
   bool Item_hybrid_func_fix_attributes(THD *thd,
                                        const char *name,
                                        Type_handler_hybrid_field_type *,