List:Commits« Previous MessageNext Message »
From:Roy Lyseng Date:December 15 2010 8:39am
Subject:Re: bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#58818
View as plain text  
Hi Ole John,

thank you for fixing this bug, the bugfix is approved.

You may also add something like: "According to the SQL standard, a HAVING 
predicate has an implicit IS TRUE added to filter out UNKNOWN values resulting 
from HAVING condition evaluation. We must ensure that the complete HAVING 
condition is evaluated before the HAVING is augmented with a triggered condition".

Please also see one question to the code below.

On 12/8/10 4:34 PM, 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-08
>        Fix for bug#58818 Incorrect result for IN/ANY subquery with HAVING condition
>
>        The fix is to ensure that the HAVING condition 'IS TRUE' when doing a
>        Item_in_subselect::single_value_transformer
>
>        If the ::single_value_transformer() find an existing HAVING condition it used
>        to do the transformation:
>
>         1) HAVING cond ->  (HAVING Cond) AND (cond_guard
> (Item_ref_null_helper(...))
>
>        As the AND condition above is Mc'Carty evaluated, the idea was that the
>        right side of the AND cond would be executed only if the HAVING evaluated to
> true.
>
>        This logic failed to considder the 3-value logic required by NULL/UNKNOWN
> evaluation.
>        Evaluating 'UNKNOWN AND<right cond>' requires the right AND-argument to
> be evaluated and
>        'false' returned if<right cond>  evaluated to false, else unknown.
>
>        To get the required behaviour this fix change the transformation in 1) to be:
>
>        2) HAVING cond ->  ((HAVING Cond) IS TRUE) AND (cond_guard
> (Item_ref_null_helper(...))
>
>      modified:
>        mysql-test/r/subselect.result
>        mysql-test/t/subselect.test
>        sql/item_subselect.cc
> === 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-08 15:34:17 +0000
> @@ -4733,4 +4733,56 @@ 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 #58818: Incorrect result for IN/ANY subquery
> +# with HAVING condition
> +#
> +CREATE TABLE t1(i INT);
> +INSERT INTO t1 VALUES (1), (2), (3);
> +CREATE TABLE t1s(i INT);
> +INSERT INTO t1s VALUES (10), (20), (30);
> +CREATE TABLE t2s(i INT);
> +INSERT INTO t2s VALUES (100), (200), (300);
> +SELECT * FROM t1
> +WHERE t1.i NOT IN
> +(
> +SELECT STRAIGHT_JOIN t2s.i
> +FROM
> +t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +HAVING t2s.i = 999
> +);
> +i
> +1
> +2
> +3
> +SELECT * FROM t1
> +WHERE t1.I IN
> +(
> +SELECT STRAIGHT_JOIN t2s.i
> +FROM
> +t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +HAVING t2s.i = 999
> +) IS UNKNOWN;
> +i
> +SELECT * FROM t1
> +WHERE NOT t1.I = ANY
> +(
> +SELECT STRAIGHT_JOIN t2s.i
> +FROM
> +t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +HAVING t2s.i = 999
> +);
> +i
> +1
> +2
> +3
> +SELECT * FROM t1
> +WHERE t1.i = ANY (
> +SELECT STRAIGHT_JOIN t2s.i
> +FROM
> +t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +HAVING t2s.i = 999
> +) IS UNKNOWN;
> +i
> +DROP TABLE t1,t1s,t2s;
>   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-08 15:34:17 +0000
> @@ -3725,4 +3725,53 @@ SELECT * FROM t2 UNION SELECT * FROM t2
>   DROP TABLE t1,t2;
>   --enable_result_log
>
> +--echo #
> +--echo # Bug #58818: Incorrect result for IN/ANY subquery
> +--echo # with HAVING condition
> +--echo #
> +
> +CREATE TABLE t1(i INT);
> +INSERT INTO t1 VALUES (1), (2), (3);
> +CREATE TABLE t1s(i INT);
> +INSERT INTO t1s VALUES (10), (20), (30);
> +CREATE TABLE t2s(i INT);
> +INSERT INTO t2s VALUES (100), (200), (300);
> +
> +SELECT * FROM t1
> +WHERE t1.i NOT IN
> +(
> +  SELECT STRAIGHT_JOIN t2s.i
> +  FROM
> +  t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +  HAVING t2s.i = 999
> +);
> +
> +SELECT * FROM t1
> +WHERE t1.I IN
> +(
> +  SELECT STRAIGHT_JOIN t2s.i
> +  FROM
> +  t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +  HAVING t2s.i = 999
> +) IS UNKNOWN;
> +
> +SELECT * FROM t1
> +WHERE NOT t1.I = ANY
> +(
> +  SELECT STRAIGHT_JOIN t2s.i
> +  FROM
> +  t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +  HAVING t2s.i = 999
> +);
> +
> +SELECT * FROM t1
> + WHERE t1.i = ANY (
> +  SELECT STRAIGHT_JOIN t2s.i
> +  FROM
> +  t1s LEFT OUTER JOIN t2s ON t2s.i = t1s.i
> +  HAVING t2s.i = 999
> + ) IS UNKNOWN;
> +
> +DROP TABLE t1,t1s,t2s;
> +
>   --echo End of 5.1 tests
>
> === modified file 'sql/item_subselect.cc'
> --- a/sql/item_subselect.cc	2010-10-18 12:12:27 +0000
> +++ b/sql/item_subselect.cc	2010-12-08 15:34:17 +0000
> @@ -1097,6 +1097,16 @@ Item_in_subselect::single_value_transfor
>         select_lex->group_list.elements)
>     {
>       bool tmp;
> +    /*
> +      If 'having' condition may evaluate to 'unknown', we must ensure
> +      it 'IS TRUE' before we are allowed to continue into the AND'ed
> +      Item_ref_null_helper object.
> +    */
> +    Item *having= join->having;
> +    if (!abort_on_null&&  having&&  having->maybe_null)

I wonder if the !abort_on_null should be part of this expression. It applies to 
the subquery predicate, but the HAVING clause of the subquery should filter out 
NULL values regardless of the context where it is used. I tried removing it, and 
all tests still function correctly.

In the case it is needed, it should be commented why.
> +    {
> +      having= new Item_func_istrue(having);
> +    }
>       Item *item= func->create(expr,
>                                new Item_ref_null_helper(&select_lex->context,
>                                                         this,
> @@ -1118,7 +1128,7 @@ Item_in_subselect::single_value_transfor
>         we can assign select_lex->having here, and pass 0 as last
>         argument (reference) to fix_fields()
>       */
> -    select_lex->having= join->having= and_items(join->having, item);
> +    select_lex->having= join->having= and_items(having, item);
>       if (join->having == item)
>         item->name= (char*)in_having_cond;
>       select_lex->having_fix_field= 1;
Thanks,
Roy

Thread
bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#58818Ole John Aske8 Dec
  • Re: bzr commit into mysql-5.1 branch (ole.john.aske:3477) Bug#58818Roy Lyseng15 Dec