List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:November 24 2010 1:58pm
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3287) Bug#54641
View as plain text  
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;
>
>
>
>
>

Thread
bzr commit into mysql-trunk branch (jorgen.loland:3287) Bug#54641Jorgen Loland19 Nov
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3287) Bug#54641Guilhem Bichot22 Nov
Re: bzr commit into mysql-trunk branch (jorgen.loland:3287) Bug#54641Øystein Grøvlen24 Nov