Roy,
I looked a bit further into why it seems we need to set function
pointers etc. for each scan. It appears that init_read_record resets
these variables. That made me look into what part of init_read_record
is actually needed for each scan, and it seems the only thing is the
call to ha_rnd_init(). Hence, an alternative is to keep the current
initialization as is, and instead add a call to ha_rnd_init() in the
scan block. I have tested this and it seems to work. What do you think?
Test result changes looks OK. See inline for comments to the existing
solution.
--
Øystein
On 10/26/10 04:15 PM, Roy Lyseng wrote:
> #At file:///home/rl136806/mysql/repo/mysql-work5/ based on
> revid:roy.lyseng@stripped
>
> 3270 Roy Lyseng 2010-10-26
> Bug#52068: Optimizer generates invalid semijoin materialization plan
>
> When MaterializeScan semijoin strategy was used and there were one
> or more outer dependent tables before the semijoin tables, the scan
> over the materialized table was not properly reset for each row of
> the prefix outer tables.
>
> Example: suppose we have a join order:
>
> ot1 SJ-Mat-Scan(it2 it3) ot4
>
> Notice that this is called a MaterializeScan, even though there is an
> outer table ahead of the materialized tables. Usually a MaterializeScan
> has the outer tables after the materialized table, but this is
> a special (but legal) case with outer dependent tables both before and
> after the materialized table.
>
> For each qualifying row from ot1, a new scan over the materialized
> table must be set up. The code failed to do that, so all scans after
> the first one returned zero rows from the materialized table.
>
> mysql-test/include/subquery_sj.inc
> Bug#52068: Optimizer generates invalid semijoin materialization plan
> Added new test.
>
> mysql-test/r/subquery_sj_all.result
> mysql-test/r/subquery_sj_all_jcl6.result
> mysql-test/r/subquery_sj_all_jcl7.result
> mysql-test/r/subquery_sj_dupsweed.result
> mysql-test/r/subquery_sj_dupsweed_jcl6.result
> mysql-test/r/subquery_sj_dupsweed_jcl7.result
> mysql-test/r/subquery_sj_firstmatch.result
> mysql-test/r/subquery_sj_firstmatch_jcl6.result
> mysql-test/r/subquery_sj_firstmatch_jcl7.result
> mysql-test/r/subquery_sj_loosescan.result
> mysql-test/r/subquery_sj_loosescan_jcl6.result
> mysql-test/r/subquery_sj_loosescan_jcl7.result
> mysql-test/r/subquery_sj_mat.result
> mysql-test/r/subquery_sj_mat_jcl6.result
> mysql-test/r/subquery_sj_mat_jcl7.result
> mysql-test/r/subquery_sj_mat_nosj.result
> mysql-test/r/subquery_sj_none.result
> mysql-test/r/subquery_sj_none_jcl6.result
> mysql-test/r/subquery_sj_none_jcl7.result
> Bug#52068: Optimizer generates invalid semijoin materialization plan
> Earlier wrong semijoin materialization test results are corrected.
>
> sql/sql_select.cc
> Bug#52068: Optimizer generates invalid semijoin materialization plan
>
> In sub_select_sjm(), moved code that initializes the scan over the
> materialized table so that it is now performed for each scan of
> table, instead of only for the first scan.
>
...
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-10-26 10:43:50 +0000
> +++ b/sql/sql_select.cc 2010-10-26 14:15:32 +0000
> @@ -16954,28 +16954,27 @@ sub_select_sjm(JOIN *join, JOIN_TAB *joi
> Ok, materialization finished. Initialize the access to the temptable
> */
> sjm->materialized= TRUE;
> +
> join_tab->read_record.read_record= join_no_more_records;
> - if (sjm->is_scan)
> - {
> - /* Initialize full scan */
> - JOIN_TAB *last_tab= join_tab + (sjm->table_count - 1);
> - init_read_record(&last_tab->read_record, join->thd,
> - sjm->table, NULL, TRUE, TRUE, FALSE);
> -
> - DBUG_ASSERT(last_tab->read_record.read_record == rr_sequential);
Based on that init_read_record resets read_record.read_record maybe this
assert should also be moved. If rr_sequential is not chosen, it
probably does not make sense to attempt a sequential scan.
> - last_tab->read_first_record= join_read_record_no_init;
> - last_tab->read_record.copy_field= sjm->copy_field;
> - last_tab->read_record.copy_field_end= sjm->copy_field +
> - sjm->table_cols.elements;
> - last_tab->read_record.read_record= rr_sequential_and_unpack;
> - }
> }
>
> if (sjm->is_scan)
> {
> - /* Do full scan of the materialized table */
> + /*
> + For each prefix partial record (if any), initialize and perform
> + a full scan of the materialized table.
> + */
> JOIN_TAB *last_tab= join_tab + (sjm->table_count - 1);
>
> + init_read_record(&last_tab->read_record, join->thd,
> + sjm->table, NULL, TRUE, TRUE, FALSE);
> +
> + last_tab->read_first_record= join_read_record_no_init;
> + last_tab->read_record.read_record= rr_sequential_and_unpack;
> + last_tab->read_record.copy_field= sjm->copy_field;
> + last_tab->read_record.copy_field_end= sjm->copy_field +
> + sjm->table_cols.elements;
Minor nitpick: If it were up to me, I would put the whole RHS of the
last assignment on a separate line.
> +
> Item *save_cond= last_tab->select_cond;
> last_tab->set_select_cond(sjm->join_cond, __LINE__);
> rc= sub_select(join, last_tab, end_of_records);
>
>
>
>
>
--
Øystein
