Skip to content

Commit

Permalink
Revert MDEV-16592 "Change Item::with_sum_func to a virtual method"
Browse files Browse the repository at this point in the history 
Added back variable 'with_sum_func' to Item class as a bit field.

This made the code shorter, faster (removed some virtual methods,
less code to create an initialized item etc) and made many Item's 7 bytes
smaller.

The code is also easier to understand as 'with_sum_func' is threated as any
other Item variable when creating or copying items.
  • Loading branch information
@montywi @vuvova
montywi authored and vuvova committed May 19, 2021
1 parent 963e5e4 commit ae39f4f
Show file tree
Hide file tree
Showing 14 changed files with 81 additions and 144 deletions.
19 changes: 10 additions & 9 deletions sql/item.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,7 @@ Item::Item(THD *thd):
{
DBUG_ASSERT(thd);
maybe_null= with_window_func= with_field= in_rollup= with_param= 0;
is_in_with_cycle= 0;
is_in_with_cycle= with_sum_func= 0;
fixed= 1; // Simple Item's doesn't have to be fixed
is_autogenerated_name= 1;
null_value= 0;
Expand Down Expand Up @@ -486,6 +486,7 @@ Item::Item(THD *thd, Item *item):
with_field(item->with_field),
is_autogenerated_name(item->is_autogenerated_name),
is_in_with_cycle(item->is_in_with_cycle),
with_sum_func(item->with_sum_func),
marker(item->marker),
null_value(item->null_value),
is_expensive_cache(-1),
Expand Down Expand Up @@ -2248,15 +2249,15 @@ void Item::split_sum_func2(THD *thd, Ref_ptr_array ref_pointer_array,
else
{
/* Not a SUM() function */
if (unlikely((!with_sum_func() && !(split_flags & SPLIT_SUM_SELECT))))
if (unlikely((!with_sum_func && !(split_flags & SPLIT_SUM_SELECT))))
{
/*
This is not a SUM function and there are no SUM functions inside.
Nothing more to do.
*/
return;
}
if (likely(with_sum_func() ||
if (likely(with_sum_func ||
(type() == FUNC_ITEM &&
(((Item_func *) this)->functype() ==
Item_func::ISNOTNULLTEST_FUNC ||
Expand Down Expand Up @@ -5361,7 +5362,7 @@ resolve_ref_in_select_and_group(THD *thd, Item_ident *ref, SELECT_LEX *select)
ref->alias_name_used= TRUE;

/* If this is a non-aggregated field inside HAVING, search in GROUP BY. */
if (select->having_fix_field && !ref->with_sum_func() && group_list)
if (select->having_fix_field && !ref->with_sum_func && group_list)
{
group_by_ref= find_field_in_group_list(ref, group_list);

Expand Down Expand Up @@ -8052,13 +8053,13 @@ bool Item_ref::fix_fields(THD *thd, Item **reference)
*/
if (!((*ref)->type() == REF_ITEM &&
((Item_ref *)(*ref))->ref_type() == OUTER_REF) &&
(((*ref)->with_sum_func() && name.str &&
(((*ref)->with_sum_func && name.str &&
!(current_sel->get_linkage() != GLOBAL_OPTIONS_TYPE &&
current_sel->having_fix_field)) ||
!(*ref)->is_fixed()))
{
my_error(ER_ILLEGAL_REFERENCE, MYF(0),
name.str, ((*ref)->with_sum_func() ?
name.str, ((*ref)->with_sum_func ?
"reference to group function":
"forward reference in item list"));
goto error;
Expand All @@ -8084,7 +8085,7 @@ void Item_ref::set_properties()
We have to remember if we refer to a sum function, to ensure that
split_sum_func() doesn't try to change the reference.
*/
copy_with_sum_func(*ref);
with_sum_func= (*ref)->with_sum_func;
with_param= (*ref)->with_param;
with_window_func= (*ref)->with_window_func;
with_field= (*ref)->with_field;
Expand Down Expand Up @@ -8566,7 +8567,7 @@ Item_cache_wrapper::Item_cache_wrapper(THD *thd, Item *item_arg):
DBUG_ASSERT(orig_item->is_fixed());
Type_std_attributes::set(orig_item);
maybe_null= orig_item->maybe_null;
copy_with_sum_func(orig_item);
with_sum_func= orig_item->with_sum_func;
with_param= orig_item->with_param;
with_field= orig_item->with_field;
name= item_arg->name;
Expand Down Expand Up @@ -8978,7 +8979,7 @@ int Item_cache_wrapper::save_in_field(Field *to, bool no_conversions)

Item* Item_cache_wrapper::get_tmp_table_item(THD *thd)
{
if (!orig_item->with_sum_func() && !orig_item->const_item())
if (!orig_item->with_sum_func && !orig_item->const_item())
return new (thd->mem_root) Item_temptable_field(thd, result_field);
return copy_or_same(thd);
}
Expand Down
67 changes: 8 additions & 59 deletions sql/item.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,6 @@ struct KEY_FIELD;
struct SARGABLE_PARAM;
class RANGE_OPT_PARAM;
class SEL_TREE;
class With_sum_func_cache;

enum precedence {
LOWEST_PRECEDENCE,
Expand Down Expand Up @@ -928,7 +927,8 @@ class Item :public Value_source,
/* Indicates that name of this Item autogenerated or set by user */
is_autogenerated_name:1,
/* Indicates that this item is in CYCLE clause of WITH */
is_in_with_cycle:1;
is_in_with_cycle:1,
with_sum_func:1; /* True if item contains a sum func */

int16 marker;

Expand Down Expand Up @@ -2413,9 +2413,6 @@ class Item :public Value_source,
*/
virtual bool with_subquery() const { DBUG_ASSERT(is_fixed()); return false; }

virtual bool with_sum_func() const { return false; }
virtual With_sum_func_cache* get_with_sum_func_cache() { return NULL; }

Item* set_expr_cache(THD *thd);

virtual Item_equal *get_item_equal() { return NULL; }
Expand Down Expand Up @@ -2542,47 +2539,6 @@ class DbugStringItemTypeValue: public StringBuffer<128>
};
#endif

class With_sum_func_cache
{
protected:
bool m_with_sum_func; // True if the owner item contains a sum func
public:
With_sum_func_cache()
:m_with_sum_func(false)
{ }
With_sum_func_cache(const Item *a)
:m_with_sum_func(a->with_sum_func())
{ }
With_sum_func_cache(const Item *a, const Item *b)
:m_with_sum_func(a->with_sum_func() || b->with_sum_func())
{ }
With_sum_func_cache(const Item *a, const Item *b, const Item *c)
:m_with_sum_func(a->with_sum_func() || b->with_sum_func() ||
c->with_sum_func())
{ }
With_sum_func_cache(const Item *a, const Item *b, const Item *c,
const Item *d)
:m_with_sum_func(a->with_sum_func() || b->with_sum_func() ||
c->with_sum_func() || d->with_sum_func())
{ }
With_sum_func_cache(const Item *a, const Item *b, const Item *c,
const Item *d, const Item *e)
:m_with_sum_func(a->with_sum_func() || b->with_sum_func() ||
c->with_sum_func() || d->with_sum_func() ||
e->with_sum_func())
{ }
void set_with_sum_func() { m_with_sum_func= true; }
void reset_with_sum_func() { m_with_sum_func= false; }
void copy_with_sum_func(const Item *item)
{
m_with_sum_func= item->with_sum_func();
}
void join_with_sum_func(const Item *item)
{
m_with_sum_func|= item->with_sum_func();
}
};


/*
This class is a replacement for the former member Item::with_subselect.
Expand Down Expand Up @@ -5385,8 +5341,7 @@ class Item_sp
}
};

class Item_ref :public Item_ident,
protected With_sum_func_cache
class Item_ref :public Item_ident
{
protected:
void set_properties();
Expand Down Expand Up @@ -5426,12 +5381,11 @@ class Item_ref :public Item_ident,

/* Constructor need to process subselect with temporary tables (see Item) */
Item_ref(THD *thd, Item_ref *item)
:Item_ident(thd, item), With_sum_func_cache(*item),
set_properties_only(0), ref(item->ref) {}
Type type() const override { return REF_ITEM; }
Type real_type() const override
:Item_ident(thd, item), set_properties_only(0), ref(item->ref) {}
enum Type type() const override { return REF_ITEM; }
enum Type real_type() const override
{ return ref ? (*ref)->type() : REF_ITEM; }
bool eq(const Item *item, bool binary_cmp) const override
bool eq(const Item *item, bool binary_cmp) const
{
Item *it= ((Item *) item)->real_item();
return ref && (*ref)->eq(it, binary_cmp);
Expand Down Expand Up @@ -5623,8 +5577,6 @@ class Item_ref :public Item_ident,
return 0;
return cleanup_processor(arg);
}
bool with_sum_func() const override { return m_with_sum_func; }
With_sum_func_cache* get_with_sum_func_cache() override { return this; }
Item *field_transformer_for_having_pushdown(THD *thd, uchar *arg) override
{ return (*ref)->field_transformer_for_having_pushdown(thd, arg); }
Item *remove_item_direct_ref() override
Expand Down Expand Up @@ -5725,8 +5677,7 @@ class Expression_cache_tracker;
*/

class Item_cache_wrapper :public Item_result_field,
public With_subquery_cache,
protected With_sum_func_cache
public With_subquery_cache
{
private:
/* Pointer on the cached expression */
Expand Down Expand Up @@ -5755,8 +5706,6 @@ class Item_cache_wrapper :public Item_result_field,
Type real_type() const override { return orig_item->type(); }
bool with_subquery() const override
{ DBUG_ASSERT(fixed); return m_with_subquery; }
bool with_sum_func() const override { return m_with_sum_func; }
With_sum_func_cache* get_with_sum_func_cache() override { return this; }

bool set_cache(THD *thd);
Expression_cache_tracker* init_tracker(MEM_ROOT *mem_root);
Expand Down
12 changes: 6 additions & 6 deletions sql/item_cmpfunc.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ bool Item_in_optimizer::fix_left(THD *thd)
used_tables_cache= args[0]->used_tables();
}
eval_not_null_tables(NULL);
copy_with_sum_func(args[0]);
with_sum_func= args[0]->with_sum_func;
with_param= args[0]->with_param || args[1]->with_param;
with_field= args[0]->with_field;
if ((const_item_cache= args[0]->const_item()))
Expand All @@ -1354,7 +1354,7 @@ bool Item_in_optimizer::fix_left(THD *thd)
{
/* to avoid overriding is called to update left expression */
used_tables_and_const_cache_join(args[1]);
join_with_sum_func(args[1]);
with_sum_func= with_sum_func || args[1]->with_sum_func;
}
DBUG_RETURN(0);
}
Expand Down Expand Up @@ -1390,7 +1390,7 @@ bool Item_in_optimizer::fix_fields(THD *thd, Item **ref)
if (args[1]->maybe_null)
maybe_null=1;
m_with_subquery= true;
join_with_sum_func(args[1]);
with_sum_func= with_sum_func || args[1]->with_sum_func;
with_field= with_field || args[1]->with_field;
with_param= args[0]->with_param || args[1]->with_param;
used_tables_and_const_cache_join(args[1]);
Expand Down Expand Up @@ -1939,7 +1939,7 @@ bool Item_func_interval::fix_length_and_dec()
max_length= 2;
used_tables_and_const_cache_join(row);
not_null_tables_cache= row->not_null_tables();
join_with_sum_func(row);
with_sum_func= with_sum_func || row->with_sum_func;
with_param= with_param || row->with_param;
with_field= with_field || row->with_field;
return FALSE;
Expand Down Expand Up @@ -4939,7 +4939,7 @@ Item_cond::fix_fields(THD *thd, Item **ref)
const_item_cache= FALSE;
}

join_with_sum_func(item);
with_sum_func|= item->with_sum_func;
with_param|= item->with_param;
with_field|= item->with_field;
m_with_subquery|= item->with_subquery();
Expand Down Expand Up @@ -7058,7 +7058,7 @@ bool Item_equal::fix_fields(THD *thd, Item **ref)
used_tables_cache|= item->used_tables();
tmp_table_map= item->not_null_tables();
not_null_tables_cache|= tmp_table_map;
DBUG_ASSERT(!item->with_sum_func() && !item->with_subquery());
DBUG_ASSERT(!item->with_sum_func && !item->with_subquery());
if (item->maybe_null)
maybe_null= 1;
if (!item->get_item_equal())
Expand Down
30 changes: 12 additions & 18 deletions sql/item_func.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ void Item_func::sync_with_sum_func_and_with_field(List<Item> &list)
Item *item;
while ((item= li++))
{
join_with_sum_func(item);
with_sum_func|= item->with_sum_func;
with_window_func|= item->with_window_func;
with_field|= item->with_field;
with_param|= item->with_param;
Expand Down Expand Up @@ -353,13 +353,11 @@ Item_func::fix_fields(THD *thd, Item **ref)
return TRUE; /* purecov: inspected */
item= *arg;

if (item->maybe_null)
maybe_null=1;

join_with_sum_func(item);
with_param= with_param || item->with_param;
with_window_func= with_window_func || item->with_window_func;
with_field= with_field || item->with_field;
maybe_null |= item->maybe_null;
with_sum_func |= item->with_sum_func;
with_param |= item->with_param;
with_window_func |= item->with_window_func;
with_field |= item->with_field;
used_tables_and_const_cache_join(item);
not_null_tables_cache|= item->not_null_tables();
m_with_subquery|= item->with_subquery();
Expand Down Expand Up @@ -741,7 +739,7 @@ void Item_func::signal_divide_by_null()

Item *Item_func::get_tmp_table_item(THD *thd)
{
if (!Item_func::with_sum_func() && !const_item())
if (!with_sum_func && !const_item())
return new (thd->mem_root) Item_temptable_field(thd, result_field);
return copy_or_same(thd);
}
Expand Down Expand Up @@ -3497,7 +3495,6 @@ udf_handler::fix_fields(THD *thd, Item_func_or_sum *func,
}
uint i;
Item **arg,**arg_end;
With_sum_func_cache *with_sum_func_cache= func->get_with_sum_func_cache();
for (i=0, arg=arguments, arg_end=arguments+arg_count;
arg != arg_end ;
arg++,i++)
Expand All @@ -3519,14 +3516,11 @@ udf_handler::fix_fields(THD *thd, Item_func_or_sum *func,
*/
if (item->collation.collation->state & MY_CS_BINSORT)
func->collation.set(&my_charset_bin);
if (item->maybe_null)
func->maybe_null=1;
if (with_sum_func_cache)
with_sum_func_cache->join_with_sum_func(item);
func->with_window_func= func->with_window_func ||
item->with_window_func;
func->with_field= func->with_field || item->with_field;
func->with_param= func->with_param || item->with_param;
func->maybe_null |= item->maybe_null;
func->with_sum_func |= item->with_sum_func;
func->with_window_func |= item->with_window_func;
func->with_field |= item->with_field;
func->with_param |= item->with_param;
func->With_subquery_cache::join(item);
func->used_tables_and_const_cache_join(item);
f_args.arg_type[i]=item->result_type();
Expand Down
Loading

0 comments on commit ae39f4f

Please sign in to comment.