List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:October 28 2010 11:02am
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3270)
Bug#52068
View as plain text  
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
Thread
bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3270) Bug#52068Roy Lyseng26 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3270)Bug#52068Øystein Grøvlen28 Oct
    • Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3270)Bug#52068Roy Lyseng29 Oct
Re: bzr commit into mysql-next-mr-bugfixing branch (roy.lyseng:3270)Bug#52068Roy Lyseng1 Nov