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 <b...@mariadb.com> 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)
{
_______________________________________________
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
