List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:April 11 2011 9:36am
Subject:Re: bzr commit into mysql-trunk branch (roy.lyseng:3359) Bug#11822517
View as plain text  
Hi Roy,

I think changing the meaning of remaining_tables is very good since it 
makes the interface more general.  This is also illustrated by the patch 
since you are able to remove semi-join specific code from 
best_extension_by_limited search().  AFAICT, you should also be able to 
simplify the code that compute the constant n_tables in greedy_search(), 
and I suggest you include that in this patch.

However, you are also adding more semi-join dependent code into 
choose_table_order(), and I do not think that is a good idea.  In my 
opinion, excluded_tables and join_tables are general concepts that 
should rather be computed by the caller, which knows the context, and 
passed as parameters.  The same also holds for jtab_sort_func.  This 
will, in my opinion, give a better separation of concerns.

See also inline for some comments to the commit comments.

On 04/ 6/11 03:27 PM, Roy Lyseng wrote:
> #At file:///home/rl136806/mysql/repo/mysql-work5/ based on
> revid:roy.lyseng@stripped
>
>   3359 Roy Lyseng	2011-04-06
>        Bug#11822517: Refactor call interface of choose_plan()
>
>        Part 2 - Change meaning of greedy_search() argument 'remaining_tables'.
>
>        The purpose of this patch is to make the interface of greedy_search()
>        and related functions simpler.
>        It is also a preparatory step for a simpler test for dependencies when
>        doing semi join materialization together with outer join (WL#5561).
>
>        The meaning of the argument remaining_tables to greedy_search() is changed:
>        Before, it meant all tables not added to the current join plan,
>        regardless of whether the tables were to be optimized or not.
>        Now, it means all tables remaining to be added.

I think the last sentence needs to be qualified with "by this.  The way 
it since now, I do not see what has changes since what is described in 
the previous sentence.

>
>        The difference is important when making a plan for a materialized
>        semi join nest. Before, remaining_tables included also the tables
>        belonging to the query, but not part of the semi join nest being
>        optimized. This makes for simpler implementation of
>        best_extension_by_limited_search() and easier maintenance.
>
>        However, best_access_path() requires that information about all tables
>        not yet added to the plan be available. We implement this by adding
>        a member variable excluded_tables in the Optimize_join_tables class,

Never heard of this class.  I guess you mean Optimize_table_order ...

>        which contains the set of all tables in the query block that are not
>        being optimized in the current operation.
>
>        sql/sql_select.cc
>          New member variable excluded_tables is added to Optimize_join_tables.
>
>          In choose_table_order(), join_tables is calculated as the set of
>          tables to be optimized.
>
>          In best_extension_by_limited_search(), use of allowed_tables is
>          eliminated, and logic has been slightly simplified.
>
>      modified:
>        sql/sql_select.cc

...

-- 
Øystein
Thread
bzr commit into mysql-trunk branch (roy.lyseng:3359) Bug#11822517Roy Lyseng6 Apr
  • Re: bzr commit into mysql-trunk branch (roy.lyseng:3359) Bug#11822517Guilhem Bichot8 Apr
  • Re: bzr commit into mysql-trunk branch (roy.lyseng:3359) Bug#11822517Øystein Grøvlen11 Apr
    • Re: bzr commit into mysql-trunk branch (roy.lyseng:3359) Bug#11822517Roy Lyseng11 Apr
      • Re: bzr commit into mysql-trunk branch (roy.lyseng:3359) Bug#11822517Øystein Grøvlen11 Apr