Re: [Maria-developers] Dead code Type_handler_hybrid_field_type::m_vers_trx_id ?

2018-04-16 Thread Alexander Barkov
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  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= _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
  _handler_vers_trx_id rather than _handler_longlong.
  Fixes Item_func::convert_const_compared_to_int_field() to
  test field_item->type_handler() against _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 _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(_handler_longlong);
+DBUG_ASSERT(0); // Not implemented yet
+f->set_handler(_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 

Re: [Maria-developers] Dead code Type_handler_hybrid_field_type::m_vers_trx_id ?

2018-04-10 Thread Aleksey Midenkov
Hi Alexander!

On Tue, Apr 10, 2018 at 12:15 PM, Alexander Barkov  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= _handler_datetime;
>
>
> Can I remove m_vers_trx_id and this dead code?

Sure, please remove. Thanks!

>
>
> Thanks!
>

___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp


[Maria-developers] Dead code Type_handler_hybrid_field_type::m_vers_trx_id ?

2018-04-10 Thread Alexander Barkov
  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= _handler_datetime;


Can I remove m_vers_trx_id and this dead code?


Thanks!


___
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp