maria-developers team mailing list archive

[Alexander Barkov <bar@xxxxxxxxxxx>]

Hi Aleksey, Sergei,

On 04/11/2018 01:34 AM, Aleksey Midenkov wrote:
> Hi Alexander!
> 
> On Tue, Apr 10, 2018 at 12:15 PM, Alexander Barkov <bar@xxxxxxxxxxx> wrote:
>>   Hi Aleksey,
>>
>> You added Type_handler_hybrid_field_type::m_vers_trx_id.
>>
>> Is it really needed? It seems to be always "false".
>> So this code in aggregate_for_comparison() seems to be a dead code:
>>
>>   if (m_vers_trx_id && (a == STRING_RESULT || b == STRING_RESULT))
>>     m_type_handler= &type_handler_datetime;
>>
>>
>> Can I remove m_vers_trx_id and this dead code?
> 
> Sure, please remove. Thanks!

Please find a patch attached.

It does the following:


1. Makes Field_vers_trx_id::type_handler() return
  &type_handler_vers_trx_id rather than &type_handler_longlong.
  Fixes Item_func::convert_const_compared_to_int_field() to
  test field_item->type_handler() against &type_handler_vers_trx_id,
  instead of testing field_item->vers_trx_id().

2. Removes VERS_TRX_ID related code from
  Type_handler_hybrid_field_type::aggregate_for_comparison(),
  because "BIGINT UNSIGNED GENERATED ALWAYS AS ROW {START|END}"
  columns behave just like a BIGINT in a regular comparison,
  i.e. when not inside AS OF.

3. Removes
   - Type_handler_hybrid_field_type::m_vers_trx_id;
   - Type_handler_hybrid_field_type::m_flags;
  because a "BIGINT UNSIGNED GENERATED ALWAYS AS ROW {START|END}"
  behaves like a regular BIGINT column when in UNION.

4. Removes Field::vers_trx_id(), Item::vers_trx_id(), Item::field_flags()
  They are not needed anymore. See N1.

All tests pass.

Thanks!

> 
>>
>>
>> Thanks!
>>

diff --git a/sql/field.h b/sql/field.h
index 7bd7963..b92de4f 100644
--- a/sql/field.h
+++ b/sql/field.h
@@ -1452,11 +1452,6 @@ class Field: public Value_source
     return flags & VERS_UPDATE_UNVERSIONED_FLAG;
   }
 
-  virtual bool vers_trx_id() const
-  {
-    return false;
-  }
-
   /*
     Validate a non-null field value stored in the given record
     according to the current thread settings, e.g. sql_mode.
@@ -2199,8 +2194,7 @@ class Field_vers_trx_id :public Field_longlong {
                        unsigned_arg),
         cached(0)
   {}
-  enum_field_types real_type() const { return MYSQL_TYPE_LONGLONG; }
-  enum_field_types type() const { return MYSQL_TYPE_LONGLONG;}
+  const Type_handler *type_handler() const { return &type_handler_vers_trx_id; }
   uint size_of() const { return sizeof(*this); }
   bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate, ulonglong trx_id);
   bool get_date(MYSQL_TIME *ltime, ulonglong fuzzydate)
@@ -2227,10 +2221,6 @@ class Field_vers_trx_id :public Field_longlong {
   }
   /* cmp_type() cannot be TIME_RESULT, because we want to compare this field against
      integers. But in all other cases we treat it as TIME_RESULT! */
-  bool vers_trx_id() const
-  {
-    return true;
-  }
 };
 
 
diff --git a/sql/handler.cc b/sql/handler.cc
index 2c93ffe..b8450e9 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -6854,7 +6854,8 @@ static Create_field *vers_init_sys_field(THD *thd, const char *field_name, int f
   f->flags= flags | NOT_NULL_FLAG;
   if (integer)
   {
-    f->set_handler(&type_handler_longlong);
+    DBUG_ASSERT(0); // Not implemented yet
+    f->set_handler(&type_handler_vers_trx_id);
     f->length= MY_INT64_NUM_DECIMAL_DIGITS - 1;
     f->flags|= UNSIGNED_FLAG;
   }
diff --git a/sql/item.cc b/sql/item.cc
index 56af69b..34a2d02 100644
--- a/sql/item.cc
+++ b/sql/item.cc
@@ -10756,11 +10756,6 @@ Item_field::excl_dep_on_grouping_fields(st_select_lex *sel)
   return find_matching_grouping_field(this, sel) != NULL;
 }
 
-bool Item_field::vers_trx_id() const
-{
-  DBUG_ASSERT(field);
-  return field->vers_trx_id();
-}
 
 void Item::register_in(THD *thd)
 {
diff --git a/sql/item.h b/sql/item.h
index 9574bdc..7bed5a1 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -832,10 +832,6 @@ class Item: public Value_source,
     return type_handler()->field_type();
   }
   virtual const Type_handler *type_handler() const= 0;
-  virtual uint field_flags() const
-  {
-    return 0;
-  }
   const Type_handler *type_handler_for_comparison() const
   {
     return type_handler()->type_handler_for_comparison();
@@ -1814,8 +1810,6 @@ class Item: public Value_source,
 
   virtual Item_field *field_for_view_update() { return 0; }
 
-  virtual bool vers_trx_id() const
-  { return false; }
   virtual Item *neg_transformer(THD *thd) { return NULL; }
   virtual Item *update_value_transformer(THD *thd, uchar *select_arg)
   { return this; }
@@ -2941,10 +2935,6 @@ class Item_field :public Item_ident,
     return field->type_handler();
   }
   TYPELIB *get_typelib() const { return field->get_typelib(); }
-  uint32 field_flags() const
-  {
-    return field->flags;
-  }
   enum_monotonicity_info get_monotonicity_info() const
   {
     return MONOTONIC_STRICT_INCREASING;
@@ -3042,7 +3032,6 @@ class Item_field :public Item_ident,
   uint32 max_display_length() const { return field->max_display_length(); }
   Item_field *field_for_view_update() { return this; }
   int fix_outer_field(THD *thd, Field **field, Item **reference);
-  virtual bool vers_trx_id() const;
   virtual Item *update_value_transformer(THD *thd, uchar *select_arg);
   Item *derived_field_transformer_for_having(THD *thd, uchar *arg);
   Item *derived_field_transformer_for_where(THD *thd, uchar *arg);
@@ -4890,12 +4879,6 @@ class Item_ref :public Item_ident
       return 0;
     return cleanup_processor(arg);
   }
-  virtual bool vers_trx_id() const
-  {
-    DBUG_ASSERT(ref);
-    DBUG_ASSERT(*ref);
-    return (*ref)->vers_trx_id();
-  }
 };
 
 
@@ -6392,29 +6375,14 @@ class Item_type_holder: public Item,
 {
 protected:
   TYPELIB *enum_set_typelib;
-private:
-  void init_flags(Item *item)
-  {
-    if (item->real_type() == Item::FIELD_ITEM)
-    {
-      Item_field *item_field= (Item_field *)item->real_item();
-      m_flags|= (item_field->field->flags &
-               (VERS_SYS_START_FLAG | VERS_SYS_END_FLAG));
-      // TODO: additional field flag?
-      m_vers_trx_id= item_field->field->vers_trx_id();
-    }
-  }
 public:
   Item_type_holder(THD *thd, Item *item)
    :Item(thd, item),
     Type_handler_hybrid_field_type(item->real_type_handler()),
-    enum_set_typelib(0),
-    m_flags(0),
-    m_vers_trx_id(false)
+    enum_set_typelib(0)
   {
     DBUG_ASSERT(item->fixed);
     maybe_null= item->maybe_null;
-    init_flags(item);
   }
   Item_type_holder(THD *thd,
                    Item *item,
@@ -6424,27 +6392,20 @@ class Item_type_holder: public Item,
    :Item(thd),
     Type_handler_hybrid_field_type(handler),
     Type_geometry_attributes(handler, attr),
-    enum_set_typelib(attr->get_typelib()),
-    m_flags(0),
-    m_vers_trx_id(false)
+    enum_set_typelib(attr->get_typelib())
   {
     name= item->name;
     Type_std_attributes::set(*attr);
     maybe_null= maybe_null_arg;
-    init_flags(item);
   }
 
   const Type_handler *type_handler() const
   {
-    const Type_handler *handler= m_vers_trx_id ?
-      &type_handler_vers_trx_id :
-      Type_handler_hybrid_field_type::type_handler();
-    return handler->type_handler_for_item_field();
+    return Type_handler_hybrid_field_type::type_handler()->
+             type_handler_for_item_field();
   }
   const Type_handler *real_type_handler() const
   {
-    if (m_vers_trx_id)
-      return &type_handler_vers_trx_id;
     return Type_handler_hybrid_field_type::type_handler();
   }
 
@@ -6471,18 +6432,6 @@ class Item_type_holder: public Item,
   }
   Item* get_copy(THD *thd) { return 0; }
 
-private:
-  uint m_flags;
-  bool m_vers_trx_id;
-public:
-  uint32 field_flags() const
-  {
-    return m_flags;
-  }
-  virtual bool vers_trx_id() const
-  {
-    return m_vers_trx_id;
-  }
 };
 
 
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index ba503f1..fc07235 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -126,12 +126,9 @@ Type_handler_hybrid_field_type::aggregate_for_comparison(const char *funcname,
     many cases.
   */
   set_handler(items[0]->type_handler()->type_handler_for_comparison());
-  m_vers_trx_id= items[0]->vers_trx_id();
   for (uint i= 1 ; i < nitems ; i++)
   {
     unsigned_count+= items[i]->unsigned_flag;
-    if (!m_vers_trx_id)
-      m_vers_trx_id= items[i]->vers_trx_id();
     if (aggregate_for_comparison(items[i]->type_handler()->
                                  type_handler_for_comparison()))
     {
@@ -423,7 +420,8 @@ void Item_func::convert_const_compared_to_int_field(THD *thd)
         args[field= 1]->real_item()->type() == FIELD_ITEM)
     {
       Item_field *field_item= (Item_field*) (args[field]->real_item());
-      if (((field_item->field_type() == MYSQL_TYPE_LONGLONG && !field_item->vers_trx_id()) ||
+      if (((field_item->field_type() == MYSQL_TYPE_LONGLONG &&
+            field_item->type_handler() != &type_handler_vers_trx_id) ||
            field_item->field_type() ==  MYSQL_TYPE_YEAR))
         convert_const_to_int(thd, field_item, &args[!field]);
     }
diff --git a/sql/sql_type.cc b/sql/sql_type.cc
index 421ff0e..5d943d4 100644
--- a/sql/sql_type.cc
+++ b/sql/sql_type.cc
@@ -694,9 +694,7 @@ Type_handler_hybrid_field_type::aggregate_for_comparison(const Type_handler *h)
 
   Item_result a= cmp_type();
   Item_result b= h->cmp_type();
-  if (m_vers_trx_id && (a == STRING_RESULT || b == STRING_RESULT))
-    m_type_handler= &type_handler_datetime;
-  else if (a == STRING_RESULT && b == STRING_RESULT)
+  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;
diff --git a/sql/sql_type.h b/sql/sql_type.h
index dd37e2b..ef233fb 100644
--- a/sql/sql_type.h
+++ b/sql/sql_type.h
@@ -3224,16 +3224,15 @@ class Type_handler_set: public Type_handler_typelib
 class Type_handler_hybrid_field_type
 {
   const Type_handler *m_type_handler;
-  bool m_vers_trx_id;
   bool aggregate_for_min_max(const Type_handler *other);
 
 public:
   Type_handler_hybrid_field_type();
   Type_handler_hybrid_field_type(const Type_handler *handler)
-   :m_type_handler(handler), m_vers_trx_id(false)
+   :m_type_handler(handler)
   { }
   Type_handler_hybrid_field_type(const Type_handler_hybrid_field_type *other)
-    :m_type_handler(other->m_type_handler), m_vers_trx_id(other->m_vers_trx_id)
+    :m_type_handler(other->m_type_handler)
   { }
   void swap(Type_handler_hybrid_field_type &other)
   {