Roy,
Thanks for fixing most of my previous comments. As mentioned, I have
two issues with the current patch:
1. The changes to explain output in subquery_sj_mat_nosj.result does
not seem desirable. Do you know why this happens.
2. Since this patch changes how constant tables are handled with
prepared statements, I think you should add some tests cases with that
scenario.
I also have some comments to the commit message, see below
On 07/ 9/10 03:36 PM, Roy Lyseng wrote:
> #At file:///home/rl136806/mysql/repo/mysql-work4/ based on
revid:guilhem@stripped
>
> 3211 Roy Lyseng 2010-07-09
> Bug#43768: Prepared query with nested subqueries core dump on
second execution
>
> The problem here is that pullout of semijoin tables is
attempted for
> each execution, but because those tables are not "pushed back"
again
> after each execution, the pullout fails on second attempt.
>
> The solution chosen here is to pullout only those semijoin
tables that are
> functionally dependent upon the outer tables. This pullout
operation
> need to be performed only once, and, unlike the current
procedure, is not
> dependent upon the row count of the involved tables.
> We still pullout tables that are classified as const tables
based on row count
> if this is a direct execution, though.
>
> The practical implication of this is as follows:
>
> - Only outer tables containing zero or one rows will now be
extracted
> as "const tables". Thus, such tables from a semijoin nest
are no
> longer accessed during optimization, and some (rare)
optimizations
> are no longer possible.
>
> - In the majority of cases, there is no performance impact.
Often,
> the new strategy chosen is Materialization, meaning that
the row
> of these table is accessed only once and saved in local memory.
>
> - Const table analysis now has to be done in two phases:
> 1) Pullout tables based on dependencies. Both outer and
inner tables
> may apply, and
> 2) Pullout tables based on row count. Outer tables are
always pulled out,
> inner tables only if inside a non-prepared statement.
>
> In order to implement the latter point above, and assure that
pullout
> of semijoin tables occurs only once, make_join_statistics()
has been
> restructured slightly. The conditional logic within the
function has also
> been enhanced for better readability.
>
> The logic of make_join_statistics() is now as follows:
>
> 1. Initialize JOIN data structures
> (major part of first loop in existing implementation).
>
> 2. Update dependencies based on join information
> (the Warshall algorithm).
>
> 3. Make key descriptions (update_ref_and_keys()).
>
> 4. Pull out semijoin tables, called only once.
>
> 5. Extract tables with zero or one rows as const tables
> (in prepared mode, consider outer tables only, no semijoin
tables).
>
> 6. Extract dependent tables as const tables.
> (in prepared mode, consider outer tables only, no semijoin
tables).
>
> 7. The remaining parts of the function.
>
> mysql-test/r/select_found.result
> Possible keys changed from "PRIMARY,kid" to "kid".
> Happens because analysis order is slightly changed, but
harmless as
> the table is identified as "const".
>
> mysql-test/r/subselect_sj.result
> Added test case for bug#43768
> A number of plan changes because of extensive testing of
semi-join
> tables with 0, 1 and 2 rows.
>
> mysql-test/r/subselect_sj_jcl6.result
> Ditto.
>
> mysql-test/t/subselect_sj.test
> Added test case for bug#43768
> bug#46744 now becomes a duplicate of bug#50489, and test
case is moved.
Please, update these comments to refer to the new test/result files.
>
> mysql-test/suite/optimizer_unfixed_bugs/t/bug46744.test
> Test for bug#46744 moved here.
>
> sql/sql_select.cc
> pull_out_semijoin_tables()
> join_tab->emb_sj_nest is no longer updated. Comments updated.
> make_join_statistics()
> Removed const table pullout from first loop.
> Simplified testing based on inner/outer/semi-join properties.
> Calls pull_out_semijoin_tables() just after dependency
analysis.
> Then, added loop that performs pullout based on row count
> but excludes semijoined tables in prepared statements.
> Second call of pull_out_semijoin_tables() is needed after
this.
> Simplified testing based on inner/outer/semi-join properties.
> Semijoin tables are no longer pulled out based on row count in
> prepared statements.
> Because pull_out_semijoin_tables() is no longer called for
each
> execution, emb_sj_nest is now set in this function.
Last sentence seems a bit strange. After a bit of thinking I now
realize that "this" refers to make_join_statistics() and not
pull_out_semijoin_tables(), but that was not clear to me when I first
read this.
--
Øystein
