List:Commits« Previous MessageNext Message »
From:Evgeny Potemkin Date:January 17 2011 11:48am
Subject:Re: bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#56690
View as plain text  
Hi Ole John,

Ok to push with extended test case. Since you completely change the 
implementation of

select_max_min_finder_subselect::cmp_xxx() it would be good to extend the test case to
something like following:

CREATE TABLE t1 (
  pk INT NOT NULL PRIMARY KEY,
  number INT,
  KEY key_number (number)
);
INSERT INTO t1 VALUES (8,8),(1,null);

CREATE TABLE t2 (
  pk INT NOT NULL PRIMARY KEY,
  number INT,
  KEY key_number (number)
);

SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2 GROUP BY number);
SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2);

SELECT * FROM t1 WHERE t1.number>  ANY(SELECT number FROM t2 GROUP BY number);
SELECT * FROM t1 WHERE t1.number>  ANY(SELECT number FROM t2);

INSERT INTO t2 VALUES (3,NULL);
INSERT INTO t2 VALUES (4,166);

SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2 GROUP BY number);
SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2);

SELECT * FROM t1 WHERE t1.number>  ANY(SELECT number FROM t2 GROUP BY number);
SELECT * FROM t1 WHERE t1.number>  ANY(SELECT number FROM t2);

DELETE FROM t2;
INSERT INTO t2 VALUES (1,2);
INSERT INTO t2 VALUES (3,NULL);
INSERT INTO t2 VALUES (2,8);

SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2 GROUP BY number);
SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2);

SELECT * FROM t1 WHERE t1.number>  ANY(SELECT number FROM t2 GROUP BY number);
SELECT * FROM t1 WHERE t1.number>  ANY(SELECT number FROM t2);

DROP TABLE t1,t2;

Regards, Evgen.

On 12/09/10 16:18, 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-09
>        Fix for bug#56690 Wrong results with subquery with GROUP BY inside<  ANY
> clause
>
>        A subquery of the form :
>
>        1) SELECT ... WHERE<column>  <  ANY(select<column>...)
>
>        Is transformed into the form by the optimizer:
>
>        2) SELECT ... WHERE<column>  <  (select MAX(<column>)...)
>
>        The Min/Max aggregation of subquery columns is implemented in
>        'class select_max_min_finder_subselect'.
>
>        The handling of NULL values in this implementation was incorrect as
>        they was interpreted as a NULL-value being '>' than any other value being
>        compared. This is incorrect wrt. SQL semantics which specifies that
>        NULL values are 'undefined' and should be removed as soon as a non-NULL
>        value is encountered.
>
>        This fix changes implementation of all
>        select_max_min_finder_subselect::cmp_<type>() methods to follow the
>        correct SQL semantics as described above. (Which also simplifies the
>        logic IMHO)
>
>        It also changes the methods to be 'private' within
>        class select_max_min_finder_subselect.
>
>      modified:
>        mysql-test/r/subselect.result
>        mysql-test/t/subselect.test
>        sql/sql_class.cc
>        sql/sql_class.h
> === modified file 'mysql-test/r/subselect.result'
> --- a/mysql-test/r/subselect.result	2010-09-09 12:46:13 +0000
> +++ b/mysql-test/r/subselect.result	2010-12-09 13:18:54 +0000
> @@ -4733,4 +4733,30 @@ ORDER BY (SELECT * FROM t1 WHERE MATCH(a
>   SELECT * FROM t2 UNION SELECT * FROM t2
>   ORDER BY (SELECT * FROM t1 WHERE MATCH(a) AGAINST ('+abc' IN BOOLEAN MODE));
>   DROP TABLE t1,t2;
> +#
> +# Bug #56690  Wrong results with subquery with
> +# GROUP BY inside<  ANY clause
> +#
> +CREATE TABLE t1 (
> +pk INT NOT NULL PRIMARY KEY,
> +number INT,
> +KEY key_number (number)
> +);
> +INSERT INTO t1 VALUES (8,8);
> +CREATE TABLE t2 (
> +pk INT NOT NULL PRIMARY KEY,
> +number INT,
> +KEY key_number (number)
> +);
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t2 VALUES (2,8);
> +INSERT INTO t2 VALUES (3,NULL);
> +INSERT INTO t2 VALUES (4,166);
> +SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2 GROUP BY number);
> +pk	number
> +8	8
> +SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2);
> +pk	number
> +8	8
> +DROP TABLE t1,t2;
>   End of 5.1 tests
>
> === modified file 'mysql-test/t/subselect.test'
> --- a/mysql-test/t/subselect.test	2010-04-15 14:04:24 +0000
> +++ b/mysql-test/t/subselect.test	2010-12-09 13:18:54 +0000
> @@ -3725,4 +3725,32 @@ SELECT * FROM t2 UNION SELECT * FROM t2
>   DROP TABLE t1,t2;
>   --enable_result_log
>
> +--echo #
> +--echo # Bug #56690  Wrong results with subquery with
> +--echo # GROUP BY inside<  ANY clause
> +--echo #
> +
> +CREATE TABLE t1 (
> + pk INT NOT NULL PRIMARY KEY,
> + number INT,
> + KEY key_number (number)
> +);
> +INSERT INTO t1 VALUES (8,8);
> +
> +CREATE TABLE t2 (
> + pk INT NOT NULL PRIMARY KEY,
> + number INT,
> + KEY key_number (number)
> +);
> +
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t2 VALUES (2,8);
> +INSERT INTO t2 VALUES (3,NULL);
> +INSERT INTO t2 VALUES (4,166);
> +
> +SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2 GROUP BY number);
> +SELECT * FROM t1 WHERE t1.number<  ANY(SELECT number FROM t2);
> +
> +DROP TABLE t1,t2;
> +
>   --echo End of 5.1 tests
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc	2010-10-23 12:55:44 +0000
> +++ b/sql/sql_class.cc	2010-12-09 13:18:54 +0000
> @@ -2355,26 +2355,24 @@ bool select_max_min_finder_subselect::cm
>   {
>     Item *maxmin= ((Item_singlerow_subselect *)item)->element_index(0);
>     double val1= cache->val_real(), val2= maxmin->val_real();
> -  if (fmax)
> -    return (cache->null_value&&  !maxmin->null_value) ||
> -      (!cache->null_value&&  !maxmin->null_value&&
> -       val1>  val2);
> -  return (maxmin->null_value&&  !cache->null_value) ||
> -    (!cache->null_value&&  !maxmin->null_value&&
> -     val1<  val2);
> +  if (cache->null_value)
> +    return false;
> +  else if (maxmin->null_value)
> +    return true;
> +  else
> +    return (fmax) ? (val1>  val2) : (val1<  val2);
>   }
>
>   bool select_max_min_finder_subselect::cmp_int()
>   {
>     Item *maxmin= ((Item_singlerow_subselect *)item)->element_index(0);
>     longlong val1= cache->val_int(), val2= maxmin->val_int();
> -  if (fmax)
> -    return (cache->null_value&&  !maxmin->null_value) ||
> -      (!cache->null_value&&  !maxmin->null_value&&
> -       val1>  val2);
> -  return (maxmin->null_value&&  !cache->null_value) ||
> -    (!cache->null_value&&  !maxmin->null_value&&
> -     val1<  val2);
> +  if (cache->null_value)
> +    return false;
> +  else if (maxmin->null_value)
> +    return true;
> +  else
> +    return (fmax) ? (val1>  val2) : (val1<  val2);
>   }
>
>   bool select_max_min_finder_subselect::cmp_decimal()
> @@ -2382,13 +2380,14 @@ bool select_max_min_finder_subselect::cm
>     Item *maxmin= ((Item_singlerow_subselect *)item)->element_index(0);
>     my_decimal cval, *cvalue= cache->val_decimal(&cval);
>     my_decimal mval, *mvalue= maxmin->val_decimal(&mval);
> -  if (fmax)
> -    return (cache->null_value&&  !maxmin->null_value) ||
> -      (!cache->null_value&&  !maxmin->null_value&&
> -       my_decimal_cmp(cvalue, mvalue)>  0) ;
> -  return (maxmin->null_value&&  !cache->null_value) ||
> -    (!cache->null_value&&  !maxmin->null_value&&
> -     my_decimal_cmp(cvalue,mvalue)<  0);
> +  if (cache->null_value)
> +    return false;
> +  else if (maxmin->null_value)
> +    return true;
> +  else
> +    return (fmax)
> +      ? (my_decimal_cmp(cvalue,mvalue)>  0)
> +      : (my_decimal_cmp(cvalue,mvalue)<  0);
>   }
>
>   bool select_max_min_finder_subselect::cmp_str()
> @@ -2401,13 +2400,14 @@ bool select_max_min_finder_subselect::cm
>     */
>     val1= cache->val_str(&buf1);
>     val2= maxmin->val_str(&buf1);
> -  if (fmax)
> -    return (cache->null_value&&  !maxmin->null_value) ||
> -      (!cache->null_value&&  !maxmin->null_value&&
> -       sortcmp(val1, val2, cache->collation.collation)>  0) ;
> -  return (maxmin->null_value&&  !cache->null_value) ||
> -    (!cache->null_value&&  !maxmin->null_value&&
> -     sortcmp(val1, val2, cache->collation.collation)<  0);
> +  if (cache->null_value)
> +    return false;
> +  else if (maxmin->null_value)
> +    return true;
> +  else
> +    return (fmax)
> +      ? (sortcmp(val1, val2, cache->collation.collation)>  0)
> +      : (sortcmp(val1, val2, cache->collation.collation)<  0);
>   }
>
>   bool select_exists_subselect::send_data(List<Item>  &items)
>
> === modified file 'sql/sql_class.h'
> --- a/sql/sql_class.h	2010-10-23 12:55:44 +0000
> +++ b/sql/sql_class.h	2010-12-09 13:18:54 +0000
> @@ -2827,6 +2827,7 @@ public:
>     {}
>     void cleanup();
>     bool send_data(List<Item>  &items);
> +private:
>     bool cmp_real();
>     bool cmp_int();
>     bool cmp_decimal();
>
>
>
>

Thread
bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#56690Ole John Aske9 Dec
  • Re: bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#56690Evgeny Potemkin17 Jan