Re: bk commit into 5.0 tree (kaa:1.2606) BUG#35298

[Sergey Petrunia]

Hi,

On Thu, Apr 17, 2008 at 11:18:50AM +0400, Alexey Kopytov wrote:
> ChangeSet@stripped, 2008-04-17 11:18:47+04:00, kaa@kaamos.(none) +3 -0
>   Fix for bug #35298: GROUP_CONCAT with DISTINCT can crash the server
>   
>   The bug is a regression introduced by the patch for bug32798.
>   
>   The code in Item_func_group_concat::clear() relied on the 'distinct'
>   variable to check if 'unique_filter' was initialized. That, however,
>   is not always valid because Item_func_group_concat::setup() can do
>   shortcuts in some cases w/o initializing 'unique_filter'.
>   
>   Fixed by checking the value of 'unique_filter' instead of 'distinct'
>   before dereferencing.
> 
>   mysql-test/r/func_gconcat.result@stripped, 2008-04-17 11:18:45+04:00, kaa@kaamos.(none)
> +26 -0
>     Added test cases for bugs #35298 and #36024.
> 
>   mysql-test/t/func_gconcat.test@stripped, 2008-04-17 11:18:45+04:00, kaa@kaamos.(none)
> +36 -0
>     Added test cases for bugs #35298 and #36024.
> 
>   sql/item_sum.cc@stripped, 2008-04-17 11:18:45+04:00, kaa@kaamos.(none) +1 -1
>     Check if unique_filter != NULL before dereferencing it. Non-zero value
>     of distinct does not always mean that unique_filter is initialized
>     because Item_func_group_concat::setup() can do shortcuts is some cases
>
> diff -Nrup a/mysql-test/r/func_gconcat.result b/mysql-test/r/func_gconcat.result
> --- a/mysql-test/r/func_gconcat.result	2008-02-28 14:31:18 +03:00
> +++ b/mysql-test/r/func_gconcat.result	2008-04-17 11:18:45 +04:00
> @@ -946,4 +946,30 @@ GROUP BY 1
>  d1
>  NULL
>  DROP TABLE t1;
> +CREATE TABLE t1 (a INT);
> +CREATE TABLE t2 (a INT);
> +INSERT INTO t1 VALUES(1);
> +SELECT GROUP_CONCAT(DISTINCT t2.a) FROM t1 LEFT JOIN t2 ON t2.a = t1.a GROUP BY
> t1.a;
> +GROUP_CONCAT(DISTINCT t2.a)
> +NULL
> +DROP TABLE t1, t2;
> +CREATE TABLE t1 (a INT);
> +CREATE TABLE t2 (b INT);
> +INSERT INTO t1 VALUES (NULL), (8), (2);
> +INSERT INTO t2 VALUES (4), (10);
> +SELECT 1 FROM t1 WHERE t1.a NOT IN
> +(
> +SELECT GROUP_CONCAT(DISTINCT t1.a)
> +FROM  t1 WHERE t1.a IN   
> +(
> +SELECT b FROM t2
> +) 
> +AND NOT t1.a >= (SELECT t1.a FROM t1 LIMIT 1)
> +GROUP BY t1.a
> +);
> +1
> +1
> +1
> +1
> +DROP TABLE t1, t2;
>  End of 5.0 tests
> diff -Nrup a/mysql-test/t/func_gconcat.test b/mysql-test/t/func_gconcat.test
> --- a/mysql-test/t/func_gconcat.test	2008-02-28 14:31:18 +03:00
> +++ b/mysql-test/t/func_gconcat.test	2008-04-17 11:18:45 +04:00
> @@ -657,4 +657,40 @@ SELECT s1.d1 FROM
>  ) AS s1;
>  DROP TABLE t1;
>  
> +#
> +# Bug #35298: GROUP_CONCAT with DISTINCT can crash the server
> +#
> +
> +CREATE TABLE t1 (a INT);
> +CREATE TABLE t2 (a INT);
> +
> +INSERT INTO t1 VALUES(1);
> +
> +SELECT GROUP_CONCAT(DISTINCT t2.a) FROM t1 LEFT JOIN t2 ON t2.a = t1.a GROUP BY
> t1.a;
> +
> +DROP TABLE t1, t2;
> +
> +#
> +# Bug #36024: group_concat distinct in subquery crash
> +#
> +
> +CREATE TABLE t1 (a INT);
> +CREATE TABLE t2 (b INT);
> +
> +INSERT INTO t1 VALUES (NULL), (8), (2);
> +INSERT INTO t2 VALUES (4), (10);
> +
> +SELECT 1 FROM t1 WHERE t1.a NOT IN
> +(
> +  SELECT GROUP_CONCAT(DISTINCT t1.a)
> +  FROM  t1 WHERE t1.a IN   
> +  (
> +    SELECT b FROM t2
> +  ) 
> +  AND NOT t1.a >= (SELECT t1.a FROM t1 LIMIT 1)
> +  GROUP BY t1.a
> +);
> +
> +DROP TABLE t1, t2;
> +
>  --echo End of 5.0 tests
> diff -Nrup a/sql/item_sum.cc b/sql/item_sum.cc
> --- a/sql/item_sum.cc	2008-03-28 14:31:48 +03:00
> +++ b/sql/item_sum.cc	2008-04-17 11:18:45 +04:00
> @@ -3222,7 +3222,7 @@ void Item_func_group_concat::clear()
>    no_appended= TRUE;
>    if (tree)
>      reset_tree(tree);
> -  if (distinct)
> +  if (unique_filter)
>      unique_filter->reset();
>    /* No need to reset the table as we never call write_row */
>  }
>
The commited testcase doesn't demonstrate the problem, for it we have 
 
  distinct!=NULL && unique!=NULL 

at all times. Please use the test case provided by the bug reporter. it 
has COUNT(DISTINCT const_column_that_is_always_null) for which we really 
can have (distinct=TRUE && unique_filter==NULL)

Ok to push after the above is addressed.

BR
 Sergey
-- 
Sergey Petrunia, Lead Software Engineer
MySQL AB, www.mysql.com
Office: N/A
Blog: http://s.petrunia.net/blog