List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:June 21 2011 11:44am
Subject:Re: bzr commit into mysql-trunk branch (jorgen.loland:3385) Bug#12616131
View as plain text  
Hi,

Thanks for the patch.  I have verified that it fixes the problem.  Patch 
is approved, but I think it would be good if you could add a summary of 
the fix to the general commit comments, and add more details to the 
file-specific part.  For example, why can the "link stuff" be removed 
from JOIN_CACHE_BKA_UNIQUE::put_record()?  And a comment that the NULL 
handling is no longer needed in get_next_key()

There is also a typo wrt the bug number in join_cache.inc:

 > +--echo # Bug312616131 - JCL: NULL VS DATE + TWICE AS MANY ROWS

--
Øystein


On 06/20/11 04:13 PM, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-next-mr-opt-backporting/ based on
> revid:jorgen.loland@stripped
>
>   3385 Jorgen Loland	2011-06-20
>        Bug#12616131 - JCL: NULL VS DATE + TWICE AS MANY ROWS RETURNED WHEN JCL>=7
>
>        Consider a query of the form
>
>        SELECT * FROM t1 LEFT JOIN t2 on t1_col = t2_col
>
>        If the BKA UNIQUE algorithm is used on table t2, rows from t1 are read
>        into the join buffer and each row's "key" (i.e., the columns used in
>        the join) are then inserted into a hash table. When the buffer is
>        either full or there are no more rows in t1, join matches are found in
>        t2 using the key values in the hash table.
>
>        The key value that is inserted into the hash table is created by
>        calling cp_buffer_from_ref(). This effectively formats the key
>        as a t2_col and copies the necessary info from JOIN_REF.
>
>        The problem in this bug was that in the case where t1_col is nullable
>        but t2_col is not, the information about NULL-values in t1_col was
>        lost in cp_buffer_from_ref(). Thus, if t1_col had a value NULL, key
>        would not be NULL but 0 (not necessarily reliably). The result was
>        that t1 rows with a t1_col value of NULL would be joined with rows in
>        t2 with t2_col=0.
>
>        Note 1: The information about NULL values in t1_col is not lost as
>        long as t2_col is also nullable.
>
>        Note 2: If the join condition is<=>, the join correctness is handled
>        by a triggered condition added in pushdown_on_conditions()
>       @ mysql-test/include/join_cache.inc
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl0.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl1.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl2.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl3.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl4.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl5.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl6.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl7.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ mysql-test/r/join_cache_jcl8.result
>          Test case for BUG#52636 no longer functioned as a regression test due to
> changed execution plan. Fixed
>          Added test for BUG#12616131
>       @ sql/sql_join_cache.cc
>          For BKA_UNIQUE: Do not add join keys with NULL values into the
>          hash table if join condition is '='. In this case, it is
>          impossible to find a join match to the key.
>
>      modified:
>        mysql-test/include/join_cache.inc
>        mysql-test/r/join_cache_jcl0.result
>        mysql-test/r/join_cache_jcl1.result
>        mysql-test/r/join_cache_jcl2.result
>        mysql-test/r/join_cache_jcl3.result
>        mysql-test/r/join_cache_jcl4.result
>        mysql-test/r/join_cache_jcl5.result
>        mysql-test/r/join_cache_jcl6.result
>        mysql-test/r/join_cache_jcl7.result
>        mysql-test/r/join_cache_jcl8.result
>        sql/sql_join_cache.cc

...
Thread
bzr commit into mysql-trunk branch (jorgen.loland:3385) Bug#12616131Jorgen Loland20 Jun
  • Re: bzr commit into mysql-trunk branch (jorgen.loland:3385) Bug#12616131Øystein Grøvlen21 Jun