- Part 4: Removing calls to sql_alloc() and sql_calloc()
Other things:
- Added current_thd in some places to make it clear that it's called (easier to remove later)
- Move memory allocation from Item_func_case::fix_length_and_dec() to Item_func_case::fix_fields()
- Added mem_root to some new calls
- Fixed some wrong UNINIT_VAR() calls
- Fixed a bug in generate_partition_syntax() in case of errors
- Added mem_root to argument to new thread_info
- Simplified my_parse_error() call in sql_yacc.yy
- Added mem_root to all calls to new Item
- Added private method operator new(size_t size) to Item to ensure that
we always use a mem_root when creating an item.
This saves use once call to current_thd per Item creation
Added mandatory thd parameter to Item (and all derivative classes) constructor.
Added thd parameter to all routines that may create items.
Also removed "current_thd" from Item::Item. This reduced number of
pthread_getspecific() calls from 290 to 177 per OLTP RO transaction.
Alternative fix that doesn't cause view.test crash in --ps:
Remember when Item_ref was fixed right in the constructor
and did not have a full Item_ref::fix_fields() call. Later
in PS/SP, after Item_ref::cleanup, we use this knowledge
to avoid doing full fix_fields() for items that were never
supposed to be fix_field'ed.
Simplify the test case.
- Changed ER(ER_...) to ER_THD(thd, ER_...) when thd was known or if there was many calls to current_thd in the same function.
- Changed ER(ER_..) to ER_THD_OR_DEFAULT(current_thd, ER...) in some places where current_thd is not necessary defined.
- Removing calls to current_thd when we have access to thd
Part of this is optimization (not calling current_thd when not needed),
but part is bug fixing for error condition when current_thd is not defined
(For example on startup and end of mysqld)
Notable renames done as otherwise a lot of functions would have to be changed:
- In JOIN structure renamed:
examined_rows -> join_examined_rows
record_count -> join_record_count
- In Field, renamed new_field() to make_new_field()
Other things:
- Added DBUG_ASSERT(thd == tmp_thd) in Item_singlerow_subselect() just to be safe.
- Removed old 'tab' prefix in JOIN_TAB::save_explain_data() and use members directly
- Added 'thd' as argument to a few functions to avoid calling current_thd.
Fixed several optimizer issues relatied to GROUP BY:
a) Refering to a SELECT column in HAVING sometimes calculated it twice, which caused problems with non determinstic functions
b) Removing duplicate fields and constants from GROUP BY was done too late for "using index for group by" optimization to work
c) EXPLAIN SELECT ... GROUP BY did wrongly show 'Using filesort' in some cases involving "Using index for group-by"
a) was fixed by:
- Changed last argument to Item::split_sum_func2() from bool to int to allow more flags
- Added flag argument to Item::split_sum_func() to allow on to specify if the item was in the SELECT part
- Mark all split_sum_func() calls from SELECT with SPLIT_SUM_SELECT
- Changed split_sum_func2() to do nothing if called with an argument that is not a sum function and doesn't include sum functions, if we are not an argument to SELECT.
This ensures that in a case like
select a*sum(b) as f1 from t1 where a=1 group by c having f1 <= 10;
That 'a' in the SELECT part is stored as a reference in the temporary table togeher with sum(b) while the 'a' in having isn't (not needed as 'a' is already a reference to a column in the result)
b) was fixed by:
- Added an extra remove_const() pass for GROUP BY arguments before make_join_statistics() in case of one table SELECT.
This allowes get_best_group_min_max() to optimize things better.
c) was fixed by:
- Added test for group by optimization in JOIN::exec_inner for
select->quick->get_type() == QUICK_SELECT_I::QS_TYPE_GROUP_MIN_MAX
item.cc:
- Simplifed Item::split_sum_func2()
- Split test to make them faster and easier to read
- Changed last argument to Item::split_sum_func2() from bool to int to allow more flags
- Added flag argument to Item::split_sum_func() to allow on to specify if the item was in the SELECT part
- Changed split_sum_func2() to do nothing if called with an argument that is not a sum function and doesn't include sum functions, if we are not an argument to SELECT.
opt_range.cc:
- Simplified get_best_group_min_max() by calcuating first how many group_by elements.
- Use join->group instead of join->group_list to test if group by, as join->group_list may be NULL if everything was optimized away.
sql_select.cc:
- Added an extra remove_const() pass for GROUP BY arguments before make_join_statistics() in case of one table SELECT.
- Use group instead of group_list to test if group by, as group_list may be NULL if everything was optimized away.
- Moved printing of "Error in remove_const" to remove_const() instead of having it in caller.
- Simplified some if tests by re-ordering code.
- update_depend_map_for_order() and remove_const() fixed to handle the case where make_join_statistics() has not yet been called (join->join_tab is 0 in this case)
Fixes over the original patch:
- Fix variable/class/other names
- Fix the JSON output to be in line with the output of other JSON
constructs we produce
This is MDEV-7601, including it's sub tasks MDEV-7594, MDEV-7555, MDEV-7590, MDEV-7581, MDEV-7589
The problem was that select_lex->non_agg_fields was not properly reset for re-execution and this caused an overwrite of a random memory position.
The fix was move non_agg_fields from select_lext to JOIN, which is properly reset.
Removing Item_cache::used_table_map, Item_cache::used_tables() and
Item_cache::set_used_tables(). Using the same inherited from
Item_basic_constant implementations instead.
Step #7 (mostly preparatory for the next step #8):
Splitting the function get_mm_parts() into a virtual method in Item.
This changes a virtual call for item->type() into a virtual call for item->get_mm_tree(),
but also *removes* one virtual call Item_cond::functype(), which used to distinguish
between COND_AND_FUNC vs COND_OR_FUNC.
Step#5: changing the function remove_eq_conds() into a virtual method in Item.
It removes 6 virtual calls for Item_func::type(), and adds only 2
virtual calls for Item***::remove_eq_conds().
Step #3: Splitting the function check_equality() into a method in Item.
Implementing Item::check_equality() and Item_func_eq::check_equality().
Implement Item_func_eq::build_equal_items() in addition to
Item_func::build_equal_items() and moving the call for check_equality()
from Item_func::build_equal_items() to Item_func_eq::build_equal_items().
Step 2c:
After discussion with Igor, it appeared that Item_field and Item_ref
could not appear in this context in the old function build_equal_item_for_cond:
else if (cond->type() == Item::FUNC_ITEM ||
cond->real_item()->type() == Item::FIELD_ITEM)
The part of the condition checking for Item_field::FIELD_ITEM was a dead code.
- Moving implementation of Item_ident_or_func_or_sum::build_equal_items()
to Item_func::build_equal_items()
- Restoring deriving of Item_ident and Item_sum_or_func from Item_result_field.
Removing Item_ident_or_func_or_sum.
MDEV-7950 Item_func::type() takes 0.26% in OLTP RO (Step#2)
- Item_ref was doing unnecessary extra job after the "MDEV-7950 Step#2" patch.
Fallback to Item::build_equal_items() if real_type() is not FIELD_ITEM.
Note, Item_ref::build_equal_items() will most likely be further simplified.
Waiting for Sanja and Igor to check a possibly dead code.
- Safety: Adding Item_sum::build_equal_items() with ASSERT, as Item_sum
should never appear in build_equal_items() context.
Step#2:
1. Removes the function build_equal_items_for_cond() and
introduces a new method Item::build_equal_items() instead,
with specific implementations in the following Items:
Item (the default implementation)
Item_ident_or_func_or_sum
Item_cond
Item_cond_and
2. Adds a new abstract class Item_ident_or_func_or_sum,
a common parent for Item_ident and Item_func_or_sum,
as they have exactly the same build_equal_items().
3. Renames Item_cond_and::cond_equal to Item_cond_and::m_cond_equal,
to avoid confusion between the member and local variables named
"cond_equal".
Taking into account implicit dependence of constant view field from nullable table of left join added.
Fixed finding real table to check if it turned to NULL (materialized view & derived taken into account)
Removed incorrect uninitialization.
- Adding a new class Item_args, represending regular function or
aggregate function arguments array.
- Adding a new class Item_func_or_sum,
a parent class for Item_func and Item_sum
- Moving Item_result_field::name() to Item_func_or_sum(),
as name() is not needed on Item_result_field level.
Backport from mysql-5.5 to mysql-5.1
Bug# 19699237: UNINITIALIZED VARIABLE IN
ITEM_FIELD::STR_RESULT LEADS TO INCORRECT
BEHAVIOR
ISSUE:
------
When the following conditions are satisfied in a query, a
server crash occurs:
a) Two rows are compared using a NULL-safe equal-to operator.
b) Each of these rows belong to different charsets.
SOLUTION:
---------
When one charset is converted to another for comparision,
the constructor of "Item_func_conv_charset" is called.
This will attempt to use the Item_cache if the string is a
constant. This check succeeds because the "used_table_map"
of the Item_cache class is never set to the correct value.
Since it is mistakenly assumed to be a constant, it tries
to fetch the relevant null value related fields which are
yet to be initialized. This results in valgrind issues
and wrong results.
The fix is to update the "used_table_map" of "Item_cache".
This will allow "Item_func_conv_charset" to realise that
this is not a constant.
- Renaming Item::is_bool_func() to is_bool_type(), to avoid assumption
that the item is an Item_func derivant.
- Deriving Item_func_spatial_rel from Item_bool_func rather than Item_int_func
LEADS TO INCORRECT BEHAVIOR
ISSUE:
------
When the following conditions are satisfied in a query, a
server crash occurs:
a) Two rows are compared using a NULL-safe equal-to operator.
b) Each of these rows belong to different charsets.
SOLUTION:
---------
When one charset is converted to another for comparision,
the constructor of "Item_func_conv_charset" is called.
This will attempt to use the Item_cache if the string is a
constant. This check succeeds because the "used_table_map"
of the Item_cache class is never set to the correct value.
Since it is mistakenly assumed to be a constant, it tries
to fetch the relevant null value related fields which are
yet to be initialized. This results in valgrind issues
and wrong results.
The fix is to update the "used_table_map" of "Item_cache".
This will allow "Item_func_conv_charset" to realise that
this is not a constant.
