Hi Jørgen,
Thanks for patch. Sorry it took some days to review this. The
patched raised some questions around how LooseScan works that I tried
to get answers to. (E.g., I wondered whether each inner table is
actually tagged with the correct strategy. The answer is no, but your
patch still works since JOIN_TAB::get_sj_strategy(), will look up the
strategy for the first table in the sj_nest. Did not realize that
until I looked at it in the debugger.)
I think the following needs to fixed:
- Since this patch is about ensuring that join buffering is not
used, I think you should add EXPLAIN variants for the failing
queries, so this can be verified.
- Comments in subquery_sj.inc about these queries giving wrong
results with loosescan should be removed.
I also wondered about this line:
> + if (!(i<= no_jbuf_after))
Any good reason not to simplify this to:
if (i > no_jbuf_after)
--
Øystein
On 11/19/10 01:19 PM, Jorgen Loland wrote:
> #At
file:///export/home/jl208045/mysql/mysql-next-mr-opt-backporting-54641-2/ based
on revid:tor.didriksen@stripped
>
> 3287 Jorgen Loland 2010-11-19
> Bug#54641: semijoin loosescan gives duplicate rows
>
> The loosescan semijoin strategy does not handle join buffering
> on any of the tables in the loosescan range. Prior to this bug
> fix, check_join_cache_usage() would not do join buffering on a
> join table if the loosescan_match_tab pointer was set for it.
>
> However, loosescan_match_tab is a pointer from the first to the
> last join table handled by the loosescan strategy. Join
> buffering could therefore be used on all join tables except
> the first in the loosescan range.
>
> The fix is to skip join buffering if the join table is in
> the loosescan range, i.e., if
> join_tab->get_sj_strategy() == SJ_OPT_LOOSE_SCAN,
> instead of checking loosescan_match_tab.
> @ mysql-test/r/subquery_sj_loosescan.result
> Updated test result after fixing BUG#54641
> @ mysql-test/r/subquery_sj_loosescan_jcl6.result
> Updated test result after fixing BUG#54641
> @ mysql-test/r/subquery_sj_loosescan_jcl7.result
> Updated test result after fixing BUG#54641
> @ sql/sql_select.cc
> In check_join_cache_usage: skip join buffering if the join
> table is in the loosescan range. This was previously done by
> checking if the join table's loosescan_match_tab was set, but
> is now done by checking if it's semijoin strategy is
> SJ_OPT_LOOSE_SCAN.
>
> modified:
> mysql-test/r/subquery_sj_loosescan.result
> mysql-test/r/subquery_sj_loosescan_jcl6.result
> mysql-test/r/subquery_sj_loosescan_jcl7.result
> sql/sql_select.cc
> === modified file 'mysql-test/r/subquery_sj_loosescan.result'
> --- a/mysql-test/r/subquery_sj_loosescan.result 2010-11-01 15:11:10 +0000
> +++ b/mysql-test/r/subquery_sj_loosescan.result 2010-11-19 12:19:28 +0000
> @@ -3495,8 +3495,6 @@ FROM t1 AS t1_1 JOIN t1 AS t1_2 ON t1_1
> );
> int_key
> 9
> -9
> -7
> 7
> SELECT t0.int_key
> FROM t0, t2
> @@ -3506,8 +3504,6 @@ FROM t1 AS t1_1 JOIN t1 AS t1_2 ON t1_1
> );
> int_key
> 9
> -9
> -7
> 7
> DROP TABLE t0, t1, t2;
> # End of bug#46550
>
> === modified file 'mysql-test/r/subquery_sj_loosescan_jcl6.result'
> --- a/mysql-test/r/subquery_sj_loosescan_jcl6.result 2010-11-01
15:11:10 +0000
> +++ b/mysql-test/r/subquery_sj_loosescan_jcl6.result 2010-11-19
12:19:28 +0000
> @@ -3499,8 +3499,6 @@ FROM t1 AS t1_1 JOIN t1 AS t1_2 ON t1_1
> );
> int_key
> 9
> -9
> -7
> 7
> SELECT t0.int_key
> FROM t0, t2
> @@ -3510,8 +3508,6 @@ FROM t1 AS t1_1 JOIN t1 AS t1_2 ON t1_1
> );
> int_key
> 9
> -9
> -7
> 7
> DROP TABLE t0, t1, t2;
> # End of bug#46550
>
> === modified file 'mysql-test/r/subquery_sj_loosescan_jcl7.result'
> --- a/mysql-test/r/subquery_sj_loosescan_jcl7.result 2010-11-01
15:11:10 +0000
> +++ b/mysql-test/r/subquery_sj_loosescan_jcl7.result 2010-11-19
12:19:28 +0000
> @@ -3499,8 +3499,6 @@ FROM t1 AS t1_1 JOIN t1 AS t1_2 ON t1_1
> );
> int_key
> 9
> -9
> -7
> 7
> SELECT t0.int_key
> FROM t0, t2
> @@ -3510,8 +3508,6 @@ FROM t1 AS t1_1 JOIN t1 AS t1_2 ON t1_1
> );
> int_key
> 9
> -9
> -7
> 7
> DROP TABLE t0, t1, t2;
> # End of bug#46550
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc 2010-11-16 16:17:25 +0000
> +++ b/sql/sql_select.cc 2010-11-19 12:19:28 +0000
> @@ -10509,9 +10509,13 @@ uint check_join_cache_usage(JOIN_TAB *ta
> goto no_join_cache;
>
> /* No join buffering if prevented by no_jbuf_after */
> - if (!(i<= no_jbuf_after) || tab->loosescan_match_tab)
> + if (!(i<= no_jbuf_after))
> goto no_join_cache;
>
> + /* No join buffering if this semijoin nest is handled by loosescan */
> + if (tab_sj_strategy == SJ_OPT_LOOSE_SCAN)
> + goto no_join_cache;
> +
> /* Neither if semijoin Materialization */
> if (sj_is_materialize_strategy(tab_sj_strategy))
> goto no_join_cache;
>
>
>
>
>
