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
