List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:December 16 2010 4:42pm
Subject:Re: bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#57030
View as plain text  
Hi Ole,

This fix is incorrect. Anyway, IMO, the another your fix is better, with some 
additions.

Regards, Evgen.


On 12/03/10 17:23, Ole John Aske wrote:
> #At file:///net/fimafeng09/export/home/tmp/oleja/mysql/mysql-5.1/ based on
> revid:georgi.kodinov@stripped
>
>   3477 Ole John Aske	2010-12-03
>        Fix for bug#57030: 'BETWEEN' evaluation is incorrect
>
>        Root cause for this bug is that the optimizer try to detect&  optimize the
> special case:
>          '<field>  BETWEEN c1 AND c1' and handle this as the condition
> '<field>  = c1'
>
>        This was implemented inside add_key_field(.. *field, *value[]...) which
> assumed
>        field to refer key Field, and value[] to refer a [low...high] constant pair.
>        value[0] and value[1] was then compared for equality.
>
>        In a 'normal' BETWEEN condition of the form '<field>  BETWEEN val1 and
> val2' the
>        BETWEEN operation is represented with an argementlist containing the
>        values [<field>, val1, val2] - add_key_field() is then called with
>        parameters field=<field>, *value=val1.
>
>        However, if the BETWEEN predicate specified:
>
>        1)  '<const1>  BETWEEN<const2>  AND<field>
>
>        the 'field' and 'value' arguments to add_key_field() had to be swapped. This
> was implemented
>        by trying the cheat add_key_field() to handle it like:
>
>        2) '<const1>  GE<const2>  AND<const1>  LE<field>'
>
>        As we didn't really replace the BETWEEN operation with 'ge' and 'le',
>        add_key_field() still handled it as a 'BETWEEN' and compared the (swapped)
>        arguments<const1>  and<const2>  for equality. If they was equal,
> the
>        condition 1) was incorrectly 'optimized' to:
>
>        3) '<field>  EQ<const1>'
>
>        This fix uses the 'num_values' argument to detect the cases where the BETWEEN
>        should not be interpreted as a BETWEEN anymore. if 'num_values<  2' it has
> logicaly
>        been transformed to a GE / LE.
>
>      modified:
>        mysql-test/r/range.result
>        mysql-test/t/range.test
>        sql/sql_select.cc
> === modified file 'mysql-test/r/range.result'
> --- a/mysql-test/r/range.result	2010-08-24 15:51:32 +0000
> +++ b/mysql-test/r/range.result	2010-12-03 14:23:13 +0000
> @@ -1666,4 +1666,91 @@ c_key	c_notkey
>   1	1
>   3	3
>   DROP TABLE t1;
> +#
> +# Bug #57030: 'BETWEEN' evaluation is incorrect
> +#
> +CREATE TABLE t1(pk INT PRIMARY KEY, i4 INT);
> +CREATE UNIQUE INDEX i4_uq ON t1(i4);
> +INSERT INTO t1 VALUES (1,10), (2,20), (3,30);
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	const	i4_uq	i4_uq	5	const	1	
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10;
> +pk	i4
> +1	10
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	i4_uq	i4_uq	5	NULL	1	Using where
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4;
> +pk	i4
> +1	10
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	i4_uq	i4_uq	5	NULL	3	Using where
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4;
> +pk	i4
> +1	10
> +2	20
> +3	30
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	i4_uq	i4_uq	5	NULL	1	Using where
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10;
> +pk	i4
> +1	10
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10;
> +pk	i4
> +1	10
> +2	20
> +3	30
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE
> +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11;
> +pk	i4
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE
> +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0;
> +pk	i4
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE noticed after reading
> const tables
> +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0;
> +pk	i4
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	i4_uq	i4_uq	5	NULL	2	Using where
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999;
> +pk	i4
> +1	10
> +2	20
> +3	30
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	NULL	NULL	NULL	NULL	NULL	NULL	NULL	Impossible WHERE noticed after reading
> const tables
> +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30;
> +pk	i4
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20';
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	range	i4_uq	i4_uq	5	NULL	1	Using where
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20';
> +pk	i4
> +1	10
> +2	20
> +DROP TABLE t1;
>   End of 5.1 tests
>
> === modified file 'mysql-test/t/range.test'
> --- a/mysql-test/t/range.test	2010-08-24 15:51:32 +0000
> +++ b/mysql-test/t/range.test	2010-12-03 14:23:13 +0000
> @@ -1325,4 +1325,63 @@ SELECT * FROM t1 WHERE 2 NOT BETWEEN c_n
>
>   DROP TABLE t1;
>
> +--echo #
> +--echo # Bug #57030: 'BETWEEN' evaluation is incorrect
> +--echo #
> +
> +# Test some BETWEEN predicates which does *not* follow the
> +# 'normal' pattern of<field>  BETWEEN<low const>  AND<high const>
> +
> +CREATE TABLE t1(pk INT PRIMARY KEY, i4 INT);
> +CREATE UNIQUE INDEX i4_uq ON t1(i4);
> +
> +INSERT INTO t1 VALUES (1,10), (2,20), (3,30);
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10;
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 10;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4;
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND i4;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4;
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND i4;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10;
> +SELECT * FROM t1 WHERE 10 BETWEEN i4 AND 10;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10;
> +SELECT * FROM t1 WHERE 10 BETWEEN 10 AND 10;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11;
> +SELECT * FROM t1 WHERE 10 BETWEEN 11 AND 11;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0;
> +SELECT * FROM t1 WHERE 10 BETWEEN 100 AND 0;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0;
> +SELECT * FROM t1 WHERE i4 BETWEEN 100 AND 0;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999;
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND 99999999999999999;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30;
> +SELECT * FROM t1 WHERE i4 BETWEEN 999999999999999 AND 30;
> +
> +EXPLAIN
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20';
> +SELECT * FROM t1 WHERE i4 BETWEEN 10 AND '20';
> +
> +
> +DROP TABLE t1;
> +
>   --echo End of 5.1 tests
>
> === modified file 'sql/sql_select.cc'
> --- a/sql/sql_select.cc	2010-10-29 08:23:06 +0000
> +++ b/sql/sql_select.cc	2010-12-03 14:23:13 +0000
> @@ -3363,6 +3363,7 @@ add_key_field(KEY_FIELD **key_fields,uin
>           */
>           if ((cond->functype() != Item_func::BETWEEN) ||
>               ((Item_func_between*) cond)->negated ||
> +            num_values<  2 ||
>               !value[0]->eq(value[1], field->binary()))
>             return;
>           eq_func= TRUE;
The actual bug here is 2 typos, correct check should be

!value[1]->eq(value[2], field->binary())

The comment above 'if' confirms this and in your another fix you also compare value[1] and
value[2].







Thread
bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#57030Ole John Aske3 Dec
  • Re: bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#57030Evgeny Potemkin16 Dec