From: Roy Lyseng
Date: May 26 2011 12:00pm
Subject: Re: bzr commit into mysql-trunk branch (epotemkin:3004) Bug#11825482
List-Archive: http://lists.mysql.com/commits/138197
Message-Id: <4DDE40F5.4050902@oracle.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Hi Evgeny,

bugfix is approved, and I agree to all Øystein's comments.

Regarding the "fixed" handling, I think it is about time that we push all 
testing of the flag inside of fix_fields(), to avoid all this excessive testing 
and prevent more asserts in the area. But not in this bugfix, of course.

There are some irrelevant details that can be removed from the new test, e.g 
autocincrement, display width, create or replace view (create view is better), 
but this is optional to consider.

Thanks,
Roy


On 25.05.11 13.05, Evgeny Potemkin wrote:
> #At file:///work/bzrroot/11825482-bug/ based on revid:epotemkin@stripped
>
>   3004 Evgeny Potemkin	2011-05-25
>        Bug#11825482: Broken key length calculation for btree index.
>        Materializable views/derived tables employs HEAP engine for result small
>        enough to fit into memory. For doing btree index lookups heap engine
>        converts key provided by mysql server into internal format. When a null value
>        were given for a variable length field converter wasn't taking into account
>        length part thus creating a wrong internal key. This led to a wrong result.
>        Also fixed a 'failed assertion' bug that manifests itself while explaining
>        complex queries.
>
>        Now hp_rb_pack_key function takes int account length part of key when skipping
>        null values for a variable length fields.
>        Item_sum_num::fix_fields now checks 'fixed' flag prior to calling fix_fields
>        function.
>       @ mysql-test/r/heap.result
>          Added a test case for the bug#11825482.
>       @ mysql-test/t/heap.test
>          Added a test case for the bug#11825482.
>          Minor improvement for Vim's syntax highligh.
>       @ sql/item_sum.cc
>          Bug#11825482: Broken key length calculation for btree index.
>          Item_sum_num::fix_fields now checks 'fixed' flag prior to calling fix_fields
>          function.
>       @ storage/heap/hp_hash.c
>          Bug#11825482: Broken key length calculation for btree index.
>          Now hp_rb_pack_key function takes int account length part of key when skipping
>          null values for a variable length fields.
>
>      modified:
>        mysql-test/r/heap.result
>        mysql-test/t/heap.test
>        sql/item_sum.cc
>        storage/heap/hp_hash.c
> === modified file 'mysql-test/r/heap.result'
> --- a/mysql-test/r/heap.result	2010-10-25 09:20:53 +0000
> +++ b/mysql-test/r/heap.result	2011-05-25 11:05:49 +0000
> @@ -701,6 +701,9 @@ insert into t1 values ("abcdefghijklmnop
>   insert into t1 values ("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz");
>   ERROR 23000: Duplicate entry 'abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijkl' for key 'PRIMARY'
>   drop table t1;
> +#
> +# Bug 12796: Record doesn't show when selecting through index
> +#
>   CREATE TABLE t1 (a int, key(a)) engine=heap;
>   insert into t1 values (0);
>   delete from t1;
> @@ -738,3 +741,45 @@ SELECT c2 FROM t1;
>   c2
>   0
>   DROP TABLE t1;
> +#
> +# BUG#11825482: Broken key length calculation for btree index
> +#
> +CREATE TABLE h1 (f1 VARCHAR(1), f2 INT NOT NULL,
> +UNIQUE KEY h1i (f1,f2) USING BTREE ) ENGINE=HEAP;
> +INSERT INTO h1 VALUES(NULL,0),(NULL,1);
> +SELECT 'wrong' FROM dual WHERE ('h', 0) NOT IN (SELECT * FROM h1);
> +wrong
> +CREATE TABLE t1 (
> +pk int(11) NOT NULL AUTO_INCREMENT,
> +col_int_nokey INT,
> +col_varchar_nokey VARCHAR(1),
> +PRIMARY KEY (pk)
> +) ENGINE=InnoDB;
> +INSERT INTO t1 VALUES (19,5,'h');
> +CREATE TABLE t2 (col_int_nokey INT) ENGINE=InnoDB;
> +INSERT INTO t2 VALUES (1);
> +CREATE OR REPLACE VIEW v1 AS
> +SELECT col_varchar_nokey, COUNT( col_varchar_nokey )
> +FROM t1
> +WHERE col_int_nokey<= 141 AND pk<= 4
> +;
> +SELECT col_int_nokey
> +FROM t2
> +WHERE ('h', 0) NOT IN (
> +SELECT *
> +FROM v1
> +);
> +col_int_nokey
> +# shouldn't crash
> +EXPLAIN SELECT col_int_nokey
> +FROM t2
> +WHERE ('h', 0) NOT IN (
> +SELECT *
> +FROM v1
> +);
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	PRIMARY	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE noticed after reading const tables
> +2	DEPENDENT SUBQUERY	<derived3>	index_subquery	auto_key0	auto_key0	12	const,const	2	Using index; Using where
> +3	DERIVED	t1	index	PRIMARY	PRIMARY	4	NULL	1	Using where; Using index
> +DROP TABLE t1,t2,h1;
> +DROP VIEW v1;
>
> === modified file 'mysql-test/t/heap.test'
> --- a/mysql-test/t/heap.test	2010-10-25 09:20:53 +0000
> +++ b/mysql-test/t/heap.test	2011-05-25 11:05:49 +0000
> @@ -436,9 +436,9 @@ insert into t1 values ("abcdefghijklmnop
>   insert into t1 values ("abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz");
>   drop table t1;
>
> -#
> -# Bug 12796: Record doesn't show when selecting through index
> -#
> +--echo #
> +--echo # Bug 12796: Record doesn't show when selecting through index
> +--echo #
>   CREATE TABLE t1 (a int, key(a)) engine=heap;
>   insert into t1 values (0);
>   delete from t1;
> @@ -480,3 +480,39 @@ INSERT INTO t1 VALUES('', 0);
>   ALTER TABLE t1 MODIFY c1 VARCHAR(101);
>   SELECT c2 FROM t1;
>   DROP TABLE t1;
> +
> +--echo #
> +--echo # BUG#11825482: Broken key length calculation for btree index
> +--echo #
> +CREATE TABLE h1 (f1 VARCHAR(1), f2 INT NOT NULL,
> +                 UNIQUE KEY h1i (f1,f2) USING BTREE ) ENGINE=HEAP;
> +INSERT INTO h1 VALUES(NULL,0),(NULL,1);
> +SELECT 'wrong' FROM dual WHERE ('h', 0) NOT IN (SELECT * FROM h1);
> +
> +CREATE TABLE t1 (
> +  pk int(11) NOT NULL AUTO_INCREMENT,
> +  col_int_nokey INT,
> +  col_varchar_nokey VARCHAR(1),
> +  PRIMARY KEY (pk)
> +);
> +
> +INSERT INTO t1 VALUES (19,5,'h');
> +
> +CREATE TABLE t2 (col_int_nokey INT);
> +
> +INSERT INTO t2 VALUES (1);
> +
> +CREATE OR REPLACE VIEW v1 AS
> +  SELECT col_varchar_nokey, COUNT( col_varchar_nokey )
> +  FROM t1
> +  WHERE col_int_nokey<= 141 AND pk<= 4
> +;
> +
> +SELECT col_int_nokey FROM t2
> +WHERE ('h', 0) NOT IN ( SELECT * FROM v1);
> +--echo # shouldn't crash
> +EXPLAIN SELECT col_int_nokey FROM t2
> +WHERE ('h', 0) NOT IN ( SELECT * FROM v1);
> +
> +DROP TABLE t1,t2,h1;
> +DROP VIEW v1;
>
> === modified file 'sql/item_sum.cc'
> --- a/sql/item_sum.cc	2011-02-02 09:21:41 +0000
> +++ b/sql/item_sum.cc	2011-05-25 11:05:49 +0000
> @@ -1108,7 +1108,8 @@ Item_sum_num::fix_fields(THD *thd, Item
>     maybe_null=0;
>     for (uint i=0 ; i<  arg_count ; i++)
>     {
> -    if (args[i]->fix_fields(thd, args + i) || args[i]->check_cols(1))
> +    if ((!args[i]->fixed&&  args[i]->fix_fields(thd, args + i)) ||
> +        args[i]->check_cols(1))
>         return TRUE;
>       set_if_bigger(decimals, args[i]->decimals);
>       maybe_null |= args[i]->maybe_null;
>
> === modified file 'storage/heap/hp_hash.c'
> --- a/storage/heap/hp_hash.c	2010-07-27 10:16:49 +0000
> +++ b/storage/heap/hp_hash.c	2011-05-25 11:05:49 +0000
> @@ -806,8 +806,17 @@ uint hp_rb_pack_key(HP_KEYDEF *keydef, u
>       if (seg->null_bit)
>       {
>         if (!(*key++= (char) 1 - *old++))
> +      {
> +        /*
> +          Skip length part of a variable length field.
> +          Length of key-part used with heap_rkey() always 2.
> +          See also hp_hashnr().
> +        */
> +        if (seg->flag&  (HA_VAR_LENGTH_PART | HA_BLOB_PART))
> +          old+= 2;
>           continue;
>         }
> +    }
>       if (seg->flag&  HA_SWAP_KEY)
>       {
>         uint length= seg->length;
>