3370 Roy Lyseng 2011-05-10
Bug#12316645: Wrong cost calculation with optimizer_join_cache_level settings
Stage 0 - change some variable declarations to const, etc.
sql/sql_select.cc:
best_access_path() - variables are constified.
check_join_cache_usage - header converted to Doxygen format.
make_join_readinfo() - variables are constified, header converted to Doxygen.
modified:
sql/sql_select.cc
3369 Olav Sandstaa 2011-05-05
Fix for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
This crash is caused by a too strict assert that was added in the
patch for Bug#58463. The purpose of this assert was to detect
situations where the optimizer pushes an index condition (ICP) on the
primary index and then later during execution "changes its mind" and
uses a different index. This assert failed to take into account that
the handler might be scanned multiple times during execution of query
and that if MRR is used only on the first initialization of the MRR
scan it will be initialized to use an index. On the second
initialization of the MRR scan it will not have an active index open on
the handler.
Detailed explanation of how this situation occurs:
The optimizer has decided to scan the second table as a range scan and
pushed an index condition to the handler on the primary key. The range
scan will be done using an DS-MRR scan to read the second table
multiple times (due to join buffering is disabled). This scan is
initialized from QUICK_RANGE_SELECT::reset().
The first time QUICK_RANGE_SELECT::reset() is called it will
initialize the handler to use the primary index. Then it will call
handler::multi_range_read_init() which will set up DS-MRR for this
scan. This is done in DsMrr_impl::dsmrr_init(). The problematic assert
will be evaluated and at this point it passes (since the pushed index
condition is on the same active index as QUICK_RANGE_SELECT::reset()
initialized). The main thing the initialization of the DS-MRR scan
does is to create a "clone" of the handler object that will be used
for some part of the DS-MRR scan. In this cloning process it will
"copy" the pushed index condition from the main handler object to the
clone handler object and set up the clone to do the index scan on the
same index as the main handler was set up to use. Then it will "close"
the active index on the main handler object since this will be used
for reading the records from the base table.
Then a second scan of the table needs to be done and this causes a
second call to QUICK_RANGE_SELECT::reset(). QUICK_RANGE_SELECT::reset()
checks if the handler has been initialized, and since it has it does
not initialize the index. Then it calls handler::multi_range_read_init()
which calls DsMrr_impl::dsmrr_init(). In DsMrr_impl::dsmrr_init() the
problematic assert will be evaluated. And this time it fails due to the
handler has no active index (since this was "transferred" to the clone
handler object during the first initialization). Thus the assert fails
even though we have a valid state of the handler object.
The fix for this problem is to make the assert less strict by adding
the case where the handler does not have an active index as a legal
state.
This patch also adds some asserts that validate that the main handler
object and the clone handler object are in a consistent state with
regards to information about pushed index conditions.
@ mysql-test/include/mrr_tests.inc
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr_all.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr_cost.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr_cost_all.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr_cost_icp.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr_icp.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/innodb_mrr_none.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr_all.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr_cost.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr_cost_all.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr_cost_icp.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr_icp.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ mysql-test/r/myisam_mrr_none.result
Test case for Bug#12321461 CRASH IN DSMRR_IMPL::DSMRR_INIT ON SELECT STRAIGHT_JOIN
@ sql/handler.cc
Extend an assert in DsMrr_impl::dsmrr_init() to also pass when the
handler object does not have an active index.
modified:
mysql-test/include/mrr_tests.inc
mysql-test/r/innodb_mrr.result
mysql-test/r/innodb_mrr_all.result
mysql-test/r/innodb_mrr_cost.result
mysql-test/r/innodb_mrr_cost_all.result
mysql-test/r/innodb_mrr_cost_icp.result
mysql-test/r/innodb_mrr_icp.result
mysql-test/r/innodb_mrr_none.result
mysql-test/r/myisam_mrr.result
mysql-test/r/myisam_mrr_all.result
mysql-test/r/myisam_mrr_cost.result
mysql-test/r/myisam_mrr_cost_all.result
mysql-test/r/myisam_mrr_cost_icp.result
mysql-test/r/myisam_mrr_icp.result
mysql-test/r/myisam_mrr_none.result
sql/handler.cc
=== modified file 'sql/sql_select.cc'
--- a/sql/sql_select.cc 2011-05-02 13:22:25 +0000
+++ b/sql/sql_select.cc 2011-05-10 06:35:59 +0000
@@ -7066,10 +7066,10 @@ best_access_path(JOIN *join,
POSITION *pos,
POSITION *loose_scan_pos)
{
- THD *thd= join->thd;
+ THD *const thd= join->thd;
Key_use *best_key= NULL;
uint best_max_key_part= 0;
- my_bool found_constraint= 0;
+ bool found_constraint= false;
double best= DBL_MAX;
double best_time= DBL_MAX;
double records= DBL_MAX;
@@ -7082,7 +7082,7 @@ best_access_path(JOIN *join,
double best_quick_records= DBL_MAX;
table_map best_ref_depends_map= 0;
double tmp;
- bool best_uses_jbuf= FALSE;
+ bool best_uses_jbuf= false;
Loose_scan_opt loose_scan_opt;
DBUG_ENTER("best_access_path");
@@ -7095,28 +7095,27 @@ best_access_path(JOIN *join,
*/
if (unlikely(s->keyuse != NULL))
{ /* Use key if possible */
- TABLE *table= s->table;
- Key_use *keyuse;
+ TABLE *const table= s->table;
double best_records= DBL_MAX;
- uint max_key_part=0;
/* Test how we can use keys */
ha_rows rec=
s->records/MATCHING_ROWS_IN_OTHER_TABLE; // Assumed records/key
- for (keyuse=s->keyuse ; keyuse->table == table ;)
+ for (Key_use *keyuse=s->keyuse; keyuse->table == table; )
{
key_part_map found_part= 0;
table_map found_ref= 0;
- uint key= keyuse->key;
- KEY *keyinfo= table->key_info+key;
- bool ft_key= (keyuse->keypart == FT_KEYPART);
+ const uint key= keyuse->key;
+ uint max_key_part= 0;
+ KEY *const keyinfo= table->key_info+key;
+ const bool ft_key= (keyuse->keypart == FT_KEYPART);
/* Bitmap of keyparts where the ref access is over 'keypart=const': */
key_part_map const_part= 0;
/* The or-null keypart in ref-or-null access: */
key_part_map ref_or_null_part= 0;
/* Calculate how many key segments of the current key we can use */
- Key_use *start_key= keyuse;
+ Key_use *const start_key= keyuse;
loose_scan_opt.next_ref_key();
DBUG_PRINT("info", ("Considering ref access on key %s",
@@ -7130,7 +7129,7 @@ best_access_path(JOIN *join,
do /* For each keypart */
{
- uint keypart= keyuse->keypart;
+ const uint keypart= keyuse->keypart;
table_map best_part_found_ref= 0;
double best_prev_record_reads= DBL_MAX;
@@ -10667,19 +10666,17 @@ void revise_cache_usage(JOIN_TAB *join_t
}
-/*
+/**
Check whether a join buffer can be used to join the specified table
- SYNOPSIS
- check_join_cache_usage()
- tab joined table to check join buffer usage for
- join join for which the check is performed
- options options of the join
- no_jbuf_after don't use join buffering after table with this number
- icp_other_tables_ok OUT TRUE if condition pushdown supports
- other tables presence
+ @param tab joined table to check join buffer usage for
+ @param join join for which the check is performed
+ @param options options of the join
+ @param no_jbuf_after don't use join buffering after table with this number
+ @param icp_other_tables_ok[out] TRUE if condition pushdown supports
+ other tables presence
- DESCRIPTION
+ @details
The function finds out whether the table 'tab' can be joined using a join
buffer. This check is performed after the best execution plan for 'join'
has been chosen. If the function decides that a join buffer can be employed
@@ -10715,7 +10712,7 @@ void revise_cache_usage(JOIN_TAB *join_t
failure to do this results in an invocation of the function that destructs
the created object.
- NOTES
+ @note
An inner table of a nested outer join or a nested semi-join can be currently
joined only when a linked cache object is employed. In these cases setting
join cache level to an odd number results in denial of usage of any join
@@ -10727,7 +10724,7 @@ void revise_cache_usage(JOIN_TAB *join_t
an index. For these engines setting the value of join_cache_level to 5 or 6
results in that no join buffer is used to join the table.
- TODO
+ @todo
Support BKA inside SJ-Materialization nests. When doing this, we'll need
to only store sj-inner tables in the join buffer.
#if 0
@@ -10751,7 +10748,7 @@ void revise_cache_usage(JOIN_TAB *join_t
}
#endif
- RETURN
+ @return
Bitmap describing the chosen cache's properties:
1) the algorithm (JOIN_CACHE::ALG_NONE, JOIN_CACHE::ALG_BNL,
JOIN_CACHE::ALG_BKA, JOIN_CACHE::ALG_BKA_UNIQUE)
@@ -11247,33 +11244,28 @@ bool setup_sj_materialization(JOIN_TAB *
}
-/*
+/**
Plan refinement stage: do various setup things for the executor
- SYNOPSIS
- make_join_readinfo()
- join Join being processed
- options Join's options (checking for SELECT_DESCRIBE,
- SELECT_NO_JOIN_CACHE)
- no_jbuf_after Don't use join buffering after table with this number.
+ @param join Join being processed
+ @param options Join's options (checking for SELECT_DESCRIBE,
+ SELECT_NO_JOIN_CACHE)
+ @param no_jbuf_after Don't use join buffering after table with this number.
- DESCRIPTION
+ @return false if successful, true if error (Out of memory)
+
+ @details
Plan refinement stage: do various set ups for the executioner
- setup join buffering use
- push index conditions
- increment relevant counters
- etc
-
- RETURN
- FALSE - OK
- TRUE - Out of memory
*/
static bool
make_join_readinfo(JOIN *join, ulonglong options, uint no_jbuf_after)
{
- uint i, jcl;
- bool statistics= test(!(join->select_options & SELECT_DESCRIBE));
+ const bool statistics= test(!(join->select_options & SELECT_DESCRIBE));
uint first_sjm_table= MAX_TABLES;
uint last_sjm_table= MAX_TABLES;
@@ -11285,11 +11277,12 @@ make_join_readinfo(JOIN *join, ulonglong
if (setup_semijoin_dups_elimination(join, options, no_jbuf_after))
DBUG_RETURN(TRUE); /* purecov: inspected */
- for (i=join->const_tables ; i < join->tables ; i++)
+ for (uint i= join->const_tables; i < join->tables; i++)
{
- JOIN_TAB *tab=join->join_tab+i;
- TABLE *table=tab->table;
+ JOIN_TAB *const tab= join->join_tab+i;
+ TABLE *const table= tab->table;
bool icp_other_tables_ok;
+ uint jcl;
tab->read_record.table= table;
tab->read_record.file=table->file;
tab->read_record.unlock_row= rr_unlock_row;
@@ -11440,12 +11433,12 @@ make_join_readinfo(JOIN *join, ulonglong
If a join buffer is used to join a table the ordering by an index
for the first non-constant table cannot be employed anymore.
*/
- for (i=join->const_tables ; i < join->tables ; i++)
+ for (uint i= join->const_tables; i < join->tables; i++)
{
- JOIN_TAB *tab=join->join_tab+i;
+ JOIN_TAB *const tab=join->join_tab + i;
if (tab->use_join_cache)
{
- JOIN_TAB *sort_by_tab= join->get_sort_by_join_tab();
+ JOIN_TAB *const sort_by_tab= join->get_sort_by_join_tab();
if (sort_by_tab)
{
join->need_tmp= 1;
No bundle (reason: useless for push emails).
| Thread |
|---|
| • bzr push into mysql-trunk branch (roy.lyseng:3369 to 3370) Bug#12316645 | Roy Lyseng | 10 May |