List:Commits« Previous MessageNext Message »
From:Øystein Grøvlen Date:October 12 2010 12:01pm
Subject:Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3260)
Bug#52329
View as plain text  
Hi Jørgen,

Thanks for the patch.  Your fix looks good.

However, when the comment right above your fix, it occurred to me that I 
had seen a similar comment somewhere else. I found the same comment in 
setup_sj_materialization() and this code also has the exact same bug. (I 
think large parts of setup_sj_materialization has been copy-pasted from 
subselect_hash_sj_engine::init_permanent()). This bug can be triggered 
by a variant of your test case: Remove group by part and turn off 
firstmatch.

I think it would be good if you could fix both cases in same bug fix. 
If you put the modified test case in subquery_sj.inc, it should cover 
both code changes, since subquery_sj_mat_nosj.test will test subquery 
materialization, and subquery_sj_mat.test will run without FirstMatch.

And we should probably also create a worklog to get rid of this 
copy-pasted code.

--
Øystein

On 10/11/10 11:34 AM, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-next-mr-opt-backporting-52329/ based on
> revid:tor.didriksen@stripped
>
>   3260 Jorgen Loland	2010-10-11
>        Bug#52329: Wrong result: subquery materialization, IN, non-null
>                   field followed by nullable
>
>        Consider a query
>
>        SELECT * FROM t1
>        WHERE (a1, a2) IN (
>        SELECT b1, b2 FROM t2 ...)
>
>        When solved using materialized subselect, we check if a record
>        from t1 is part of the result set by constructing a key from the
>        a1 and a2 values and perform a lookup based on that key. If a1
>        and a2 are CHAR(3) with values foo and bar, the KEY is "foobar".
>        If a2 is NULLable, the null-bit should be the first bit for this
>        field's part of the KEY ("foo<nullbit_a2>bar").
>
>        Before, the null-bit pointer for each key field wrongly pointed
>        to the first bit of the KEY instead of the first bit of the key
>        part. Thus, when setting the null bit for a2 above, the first bit
>        of a1 was wrongly updated: "0oo bar" ('f' in foo replaced
>        with null-bit of a2, and the null-bit of a2 not set)
>
>        This patch sets the null-pointer for each key part to the first
>        bit of that field's part of KEY, not the first bit of KEY.
>
>        The bug was masked if the first field was NOT NULL.
>       @ mysql-test/include/subquery_mat.inc
>          Added test for BUG#52329
>       @ mysql-test/r/subquery_mat.result
>          Added test for BUG#52329
>       @ mysql-test/r/subquery_mat_all.result
>          Added test for BUG#52329
>       @ mysql-test/r/subquery_mat_none.result
>          Added test for BUG#52329
>       @ sql/item_subselect.cc
>          subselect_hash_sj_engine::init_permanent:
>          Make the null-pointer for each key part point to the first
>          bit of that field's part of KEY, not the first bit of KEY.
>
>      modified:
>        mysql-test/include/subquery_mat.inc
>        mysql-test/r/subquery_mat.result
>        mysql-test/r/subquery_mat_all.result
>        mysql-test/r/subquery_mat_none.result
>        sql/item_subselect.cc
> === modified file 'mysql-test/include/subquery_mat.inc'
> --- a/mysql-test/include/subquery_mat.inc	2010-09-08 14:39:38 +0000
> +++ b/mysql-test/include/subquery_mat.inc	2010-10-11 09:34:05 +0000
> @@ -815,3 +815,40 @@ DROP TABLE t1, t2;
>
>   --echo # End BUG#56367
>
> +--echo #
> +--echo # BUG#52329 - Wrong result: subquery materialization, IN,
> +--echo #             non-null field followed by nullable
> +--echo #
> +
> +CREATE TABLE t1 (a1 CHAR(8) NOT NULL, a2 char(8) NOT NULL);
> +
> +CREATE TABLE t2a (b1 char(8), b2 char(8));
> +CREATE TABLE t2b (b1 CHAR(8), b2 char(8) NOT NULL);
> +CREATE TABLE t2c (b1 CHAR(8) NOT NULL, b2 char(8));
> +
> +INSERT INTO t1 VALUES ('1 - 12', '2 - 22');
> +
> +INSERT INTO t2a VALUES ('1 - 11', '2 - 21'),
> +                       ('1 - 11', '2 - 21'),
> +                       ('1 - 12', '2 - 22'),
> +                       ('1 - 12', '2 - 22'),
> +                       ('1 - 13', '2 - 23');
> +
> +INSERT INTO t2b SELECT * FROM t2a;
> +INSERT INTO t2c SELECT * FROM t2a;
> +
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +      SELECT b1, b2 FROM t2a WHERE b1>  '0' GROUP BY b1, b2);
> +
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +      SELECT b1, b2 FROM t2b WHERE b1>  '0' GROUP BY b1, b2);
> +
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +      SELECT b1, b2 FROM t2c WHERE b1>  '0' GROUP BY b1, b2);
> +
> +DROP TABLE t1,t2a,t2b,t2c;
> +
> +--echo # End BUG#52329
>
> === modified file 'mysql-test/r/subquery_mat.result'
> --- a/mysql-test/r/subquery_mat.result	2010-09-08 14:39:38 +0000
> +++ b/mysql-test/r/subquery_mat.result	2010-10-11 09:34:05 +0000
> @@ -1097,4 +1097,37 @@ SELECT t1.* FROM t1 LEFT JOIN t2 ON t1.a
>   a
>   DROP TABLE t1, t2;
>   # End BUG#56367
> +#
> +# BUG#52329 - Wrong result: subquery materialization, IN,
> +#             non-null field followed by nullable
> +#
> +CREATE TABLE t1 (a1 CHAR(8) NOT NULL, a2 char(8) NOT NULL);
> +CREATE TABLE t2a (b1 char(8), b2 char(8));
> +CREATE TABLE t2b (b1 CHAR(8), b2 char(8) NOT NULL);
> +CREATE TABLE t2c (b1 CHAR(8) NOT NULL, b2 char(8));
> +INSERT INTO t1 VALUES ('1 - 12', '2 - 22');
> +INSERT INTO t2a VALUES ('1 - 11', '2 - 21'),
> +('1 - 11', '2 - 21'),
> +('1 - 12', '2 - 22'),
> +('1 - 12', '2 - 22'),
> +('1 - 13', '2 - 23');
> +INSERT INTO t2b SELECT * FROM t2a;
> +INSERT INTO t2c SELECT * FROM t2a;
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2a WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2b WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2c WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +DROP TABLE t1,t2a,t2b,t2c;
> +# End BUG#52329
>   set optimizer_switch=default;
>
> === modified file 'mysql-test/r/subquery_mat_all.result'
> --- a/mysql-test/r/subquery_mat_all.result	2010-09-08 14:39:38 +0000
> +++ b/mysql-test/r/subquery_mat_all.result	2010-10-11 09:34:05 +0000
> @@ -1096,4 +1096,37 @@ SELECT t1.* FROM t1 LEFT JOIN t2 ON t1.a
>   a
>   DROP TABLE t1, t2;
>   # End BUG#56367
> +#
> +# BUG#52329 - Wrong result: subquery materialization, IN,
> +#             non-null field followed by nullable
> +#
> +CREATE TABLE t1 (a1 CHAR(8) NOT NULL, a2 char(8) NOT NULL);
> +CREATE TABLE t2a (b1 char(8), b2 char(8));
> +CREATE TABLE t2b (b1 CHAR(8), b2 char(8) NOT NULL);
> +CREATE TABLE t2c (b1 CHAR(8) NOT NULL, b2 char(8));
> +INSERT INTO t1 VALUES ('1 - 12', '2 - 22');
> +INSERT INTO t2a VALUES ('1 - 11', '2 - 21'),
> +('1 - 11', '2 - 21'),
> +('1 - 12', '2 - 22'),
> +('1 - 12', '2 - 22'),
> +('1 - 13', '2 - 23');
> +INSERT INTO t2b SELECT * FROM t2a;
> +INSERT INTO t2c SELECT * FROM t2a;
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2a WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2b WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2c WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +DROP TABLE t1,t2a,t2b,t2c;
> +# End BUG#52329
>   set optimizer_switch=default;
>
> === modified file 'mysql-test/r/subquery_mat_none.result'
> --- a/mysql-test/r/subquery_mat_none.result	2010-09-08 14:39:38 +0000
> +++ b/mysql-test/r/subquery_mat_none.result	2010-10-11 09:34:05 +0000
> @@ -1096,4 +1096,37 @@ SELECT t1.* FROM t1 LEFT JOIN t2 ON t1.a
>   a
>   DROP TABLE t1, t2;
>   # End BUG#56367
> +#
> +# BUG#52329 - Wrong result: subquery materialization, IN,
> +#             non-null field followed by nullable
> +#
> +CREATE TABLE t1 (a1 CHAR(8) NOT NULL, a2 char(8) NOT NULL);
> +CREATE TABLE t2a (b1 char(8), b2 char(8));
> +CREATE TABLE t2b (b1 CHAR(8), b2 char(8) NOT NULL);
> +CREATE TABLE t2c (b1 CHAR(8) NOT NULL, b2 char(8));
> +INSERT INTO t1 VALUES ('1 - 12', '2 - 22');
> +INSERT INTO t2a VALUES ('1 - 11', '2 - 21'),
> +('1 - 11', '2 - 21'),
> +('1 - 12', '2 - 22'),
> +('1 - 12', '2 - 22'),
> +('1 - 13', '2 - 23');
> +INSERT INTO t2b SELECT * FROM t2a;
> +INSERT INTO t2c SELECT * FROM t2a;
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2a WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2b WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +SELECT * FROM t1
> +WHERE (a1, a2) IN (
> +SELECT b1, b2 FROM t2c WHERE b1>  '0' GROUP BY b1, b2);
> +a1	a2
> +1 - 12	2 - 22
> +DROP TABLE t1,t2a,t2b,t2c;
> +# End BUG#52329
>   set optimizer_switch=default;
>
> === modified file 'sql/item_subselect.cc'
> --- a/sql/item_subselect.cc	2010-09-28 15:17:29 +0000
> +++ b/sql/item_subselect.cc	2010-10-11 09:34:05 +0000
> @@ -3214,7 +3214,7 @@ bool subselect_hash_sj_engine::init_perm
>                                       use that information instead.
>                                    */
>                                    cur_ref_buff + null_count,
> -                                 null_count ? tab->ref.key_buff : 0,
> +                                 null_count ? cur_ref_buff : 0,
>                                    cur_key_part->length, tab->ref.items[i]);
>       cur_ref_buff+= cur_key_part->store_length;
>     }
>
>
>
>
>


-- 
Øystein Grøvlen, Senior Staff Engineer
Sun Microsystems, Database Group
Trondheim, Norway
Thread
bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3260) Bug#52329Jorgen Loland11 Oct
  • Re: bzr commit into mysql-next-mr-bugfixing branch (jorgen.loland:3260)Bug#52329Øystein Grøvlen12 Oct