From: Roy Lyseng
Date: March 4 2011 2:16pm
Subject: Re: bzr commit into mysql-trunk branch (jorgen.loland:3724) Bug#11766317
List-Archive: http://lists.mysql.com/commits/132464
Message-Id: <4D70F45B.9000503@oracle.com>
MIME-Version: 1.0
Content-Type: multipart/alternative;
 boundary="------------050303090705050707070307"

--------------050303090705050707070307
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 8bit

Hi Jørgen,

thank you for the bug fix.

The patch is approved.

Roy

On 04.03.11 11.49, Jorgen Loland wrote:
> #At file:///export/home/jl208045/mysql/mysql-trunk-59405/ based on revid:mattias.jonsson@stripped
>
>   3724 Jorgen Loland	2011-03-04
>        BUG#11766317: FIND_IN_SET won't work normaly after upgrade
>                      from 5.1 to 5.5
>
>        (Former 59405)
>
>        In this bug, args[0] in an Item_func_find_in_set stored an
>        Item_func_weekday that was constant. In
>        Item_func_find_in_set::fix_length_and_dec(), args[0]->val_str()
>        was called. Later, when Item_func_find_in_set::val_int() was
>        called, args[0]->null_value was checked. However, the
>        Item_func_weekday in args[0] had now been replaced with an
>        Item_cache. No val_*() calls had been made to this Item_cache,
>        thus null_value was incorrectly 'true', resulting in missing
>        rows in the result set.
>
>        enum_value gets a value in fix_length_and_dec() iff args[0]
>        is both constant and non-null. It is therefore unnecessary
>        to check the null_value of args[0] in val_int().
>
>        An alternative fix would be to call args[0]->val_int() inside
>        Item_func_find_in_set::val_int(). This would ensure
>        args[0]->null_value was set correctly (always false in this case),
>        but that would have to be done for every record this const value
>        is checked against.
>       @ mysql-test/r/func_set.result
>          Add test for BUG#59405
>       @ mysql-test/t/func_set.test
>          Add test for BUG#59405
>       @ sql/item_func.cc
>          Add variable Item_func_find_in_set::args_0_null_value
>
>      modified:
>        mysql-test/r/func_set.result
>        mysql-test/t/func_set.test
>        sql/item_func.cc
> === modified file 'mysql-test/r/func_set.result'
> --- a/mysql-test/r/func_set.result	2009-06-16 14:36:15 +0000
> +++ b/mysql-test/r/func_set.result	2011-03-04 10:49:01 +0000
> @@ -159,3 +159,45 @@ SELECT CONVERT( a USING latin1 ) FROM t2
>   CONVERT( a USING latin1 )
>
>   DROP TABLE t1, t2;
> +#
> +# BUG#59405: FIND_IN_SET won't work normaly after upgrade from 5.1 to 5.5
> +#
> +CREATE TABLE t1(days set('1','2','3','4','5','6','7'));
> +INSERT INTO t1 VALUES('1,2,3,4,5,6,7'), (NULL), ('1,2,3,4,5,6,7');
> +
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), days);
> +days
> +1,2,3,4,5,6,7
> +1,2,3,4,5,6,7
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), days) IS UNKNOWN;
> +days
> +NULL
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), NULL);
> +days
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), NULL) IS UNKNOWN;
> +days
> +1,2,3,4,5,6,7
> +NULL
> +1,2,3,4,5,6,7
> +SELECT * FROM t1 WHERE FIND_IN_SET(7, days);
> +days
> +1,2,3,4,5,6,7
> +1,2,3,4,5,6,7
> +SELECT * FROM t1 WHERE FIND_IN_SET(8, days);
> +days
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, days);
> +days
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, days) IS UNKNOWN;
> +days
> +1,2,3,4,5,6,7
> +NULL
> +1,2,3,4,5,6,7
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, NULL);
> +days
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, NULL) IS UNKNOWN;
> +days
> +1,2,3,4,5,6,7
> +NULL
> +1,2,3,4,5,6,7
> +
> +DROP TABLE t1;
>
> === modified file 'mysql-test/t/func_set.test'
> --- a/mysql-test/t/func_set.test	2009-06-16 14:36:15 +0000
> +++ b/mysql-test/t/func_set.test	2011-03-04 10:49:01 +0000
> @@ -97,3 +97,25 @@ SELECT CONVERT( a USING latin1 ) FROM t1
>   SELECT CONVERT( a USING latin1 ) FROM t2;
>
>   DROP TABLE t1, t2;
> +
> +--echo #
> +--echo # BUG#59405: FIND_IN_SET won't work normaly after upgrade from 5.1 to 5.5
> +--echo #
> +
> +CREATE TABLE t1(days set('1','2','3','4','5','6','7'));
> +INSERT INTO t1 VALUES('1,2,3,4,5,6,7'), (NULL), ('1,2,3,4,5,6,7');
> +
> +--echo
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), days);
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), days) IS UNKNOWN;
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), NULL);
> +SELECT * FROM t1 WHERE FIND_IN_SET(DAYOFWEEK(CURRENT_DATE()), NULL) IS UNKNOWN;
> +SELECT * FROM t1 WHERE FIND_IN_SET(7, days);
> +SELECT * FROM t1 WHERE FIND_IN_SET(8, days);
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, days);
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, days) IS UNKNOWN;
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, NULL);
> +SELECT * FROM t1 WHERE FIND_IN_SET(NULL, NULL) IS UNKNOWN;
> +
> +--echo
> +DROP TABLE t1;
>
> === modified file 'sql/item_func.cc'
> --- a/sql/item_func.cc	2011-03-01 14:47:01 +0000
> +++ b/sql/item_func.cc	2011-03-04 10:49:01 +0000
> @@ -3021,6 +3021,8 @@ void Item_func_find_in_set::fix_length_a
>         String *find=args[0]->val_str(&value);
>         if (find)
>         {
> +        // find is not NULL pointer so args[0] is not a null-value
> +        DBUG_ASSERT(!args[0]->null_value);
>   	enum_value= find_type(((Field_enum*) field)->typelib,find->ptr(),
>   			      find->length(), 0);
>   	enum_bit=0;
> @@ -3039,11 +3041,22 @@ longlong Item_func_find_in_set::val_int(
>     DBUG_ASSERT(fixed == 1);
>     if (enum_value)
>     {
> -    ulonglong tmp=(ulonglong) args[1]->val_int();
> -    if (!(null_value=args[1]->null_value || args[0]->null_value))
> +    // enum_value is set iff args[0]->const_item() in fix_length_and_dec().
> +    DBUG_ASSERT(args[0]->const_item());
> +
> +    ulonglong tmp= (ulonglong) args[1]->val_int();
> +    null_value= args[1]->null_value;
> +    /*
> +      No need to check args[0]->null_value since enum_value is set iff
> +      args[0] is a non-null const item. Note: no DBUG_ASSERT on
> +      args[0]->null_value here because args[0] may have been replaced
> +      by an Item_cache on which val_int() has not been called. See
> +      BUG#11766317
> +    */
> +    if (!null_value)
>       {
>         if (tmp&  enum_bit)
> -	return enum_value;
> +        return enum_value;
>       }
>       return 0L;
>     }
>
>
>
>


--------------050303090705050707070307--