Re: bk commit into 5.0 tree (mhansson:1.2494) BUG#30596

[Timour Katchaounov]

Martin,

Please take into account the following minor comments.
Have you tested this fix with 5.1?

Timour

> Date: August 23 2007 5:10pm
> Subject: bk commit into 5.0 tree (mhansson:1.2494) BUG#30596
> 
> Below is the list of changes that have just been committed into a local
> 5.0 repository of martin. When martin does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
> 
> ChangeSet@stripped, 2007-08-23 17:10:44+02:00, mhansson@stripped +5 -0
>   Bug #30596  	GROUP BY optimization gives wrong result order
>   
>   The optimization that uses a unique index to remove GROUP BY, did not 
>   ensure that the index was actually used, thus violating the ORDER BY
>   that is impled by GROUP BY.
>   Fixed by replacing GROUP BY with ORDER BY if the GROUP BY clause contains
>   a unique index. In case GROUP BY ... ORDER BY null is used, GROUP BY is
>   simply removed.
> 
>   BitKeeper/etc/ignore@stripped, 2007-08-23 17:10:42+02:00, mhansson@stripped +5
> -0
>     Added support-files/mysqld_multi.server tests/bug25714 cscope.in.out cscope.out
> cscope.po.out to the ignore list
> 
>   mysql-test/r/distinct.result@stripped, 2007-08-23 17:10:42+02:00,
> mhansson@stripped +3 -3
>     Bug#30596: Changed test case
> 
>   mysql-test/r/group_by.result@stripped, 2007-08-23 17:10:42+02:00,
> mhansson@stripped +23 -0
>     Bug#30596: Correct result
> 
>   mysql-test/t/group_by.test@stripped, 2007-08-23 17:10:42+02:00,
> mhansson@stripped +20 -0
>     Bug#30596: Test case
> 
>   sql/sql_select.cc@stripped, 2007-08-23 17:10:42+02:00, mhansson@stripped +1 -0
>     Bug#30596: The fix, replacing GROUP BY with ORDER BY
> 
> diff -Nrup a/BitKeeper/etc/ignore b/BitKeeper/etc/ignore
> --- a/BitKeeper/etc/ignore	2007-07-25 19:18:10 +02:00
> +++ b/BitKeeper/etc/ignore	2007-08-23 17:10:42 +02:00
> @@ -1345,3 +1345,8 @@ zlib/*.vcproj
>  debian/control
>  debian/defs.mk
>  include/abi_check
> +support-files/mysqld_multi.server
> +tests/bug25714
> +cscope.in.out
> +cscope.out
> +cscope.po.out
> diff -Nrup a/mysql-test/r/distinct.result b/mysql-test/r/distinct.result
> --- a/mysql-test/r/distinct.result	2007-04-26 22:10:37 +02:00
> +++ b/mysql-test/r/distinct.result	2007-08-23 17:10:42 +02:00
> @@ -526,10 +526,10 @@ id	select_type	table	type	possible_keys	
>  1	SIMPLE	t1	index	NULL	PRIMARY	4	NULL	3	Using index
>  EXPLAIN SELECT a,b FROM t1 GROUP BY a,b;
>  id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> -1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	Using filesort

Please explain, why the above plan changes to a less efficient one.
Was the original plan wrong? Did it deliver results in wrong order?

>  EXPLAIN SELECT DISTINCT a,b FROM t1 GROUP BY a,b;
>  id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> -1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	Using filesort

Same question as above.

>  CREATE TABLE t2(a INT, b INT NOT NULL, c INT NOT NULL, d INT, 
>  PRIMARY KEY (a,b));
>  INSERT INTO t2 VALUES (1,1,1,50), (1,2,3,40), (2,1,3,4);
> @@ -554,7 +554,7 @@ id	select_type	table	type	possible_keys	
>  CREATE UNIQUE INDEX c_b_unq ON t2 (c,b);
>  EXPLAIN SELECT DISTINCT a,b,d FROM t2 GROUP BY c,b,d;
>  id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> -1	SIMPLE	t2	ALL	NULL	NULL	NULL	NULL	3	
> +1	SIMPLE	t2	ALL	NULL	NULL	NULL	NULL	3	Using filesort

Same question as above.

>  DROP TABLE t1,t2;
>  create table t1 (id int, dsc varchar(50));
>  insert into t1 values (1, "line number one"), (2, "line number two"), (3, "line
> number three");
> diff -Nrup a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result
> --- a/mysql-test/r/group_by.result	2007-07-31 08:09:58 +02:00
> +++ b/mysql-test/r/group_by.result	2007-08-23 17:10:42 +02:00
> @@ -1064,3 +1064,26 @@ select t1.f1,t.* from t1, t1 t group by 
>  ERROR 42000: 'test.t.f1' isn't in GROUP BY
>  drop table t1;
>  SET SQL_MODE = '';
> +CREATE TABLE t1(
> +a INT, 
> +b INT NOT NULL, 
> +c INT NOT NULL, 
> +d INT, 
> +UNIQUE KEY (c,b)
> +);
> +INSERT INTO t1 VALUES (1,1,1,50), (1,2,3,40), (2,1,3,4);
> +SELECT c,b,d FROM t1 GROUP BY c,b,d;
> +c	b	d
> +1	1	50
> +3	1	4
> +3	2	40
> +EXPLAIN SELECT c,b,d FROM t1 GROUP BY c,b,d;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	Using filesort
> +EXPLAIN SELECT c,b,d FROM t1 GROUP BY c,b,d ORDER BY NULL;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	
> +EXPLAIN SELECT c,b,d FROM t1 ORDER BY c,b,d;
> +id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
> +1	SIMPLE	t1	ALL	NULL	NULL	NULL	NULL	3	Using filesort
> +DROP TABLE t1;
> diff -Nrup a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test
> --- a/mysql-test/t/group_by.test	2007-07-31 08:03:13 +02:00
> +++ b/mysql-test/t/group_by.test	2007-08-23 17:10:42 +02:00
> @@ -788,3 +788,23 @@ select * from t1 group by f1, f2;
>  select t1.f1,t.* from t1, t1 t group by 1;
>  drop table t1;
>  SET SQL_MODE = '';
> +
> +#
> +# Bug#30596: GROUP BY optimization gives wrong result order
> +#
> +CREATE TABLE t1(
> +  a INT, 
> +  b INT NOT NULL, 
> +  c INT NOT NULL, 
> +  d INT, 
> +  UNIQUE KEY (c,b)
> +);
> +
> +INSERT INTO t1 VALUES (1,1,1,50), (1,2,3,40), (2,1,3,4);
> +
> +SELECT c,b,d FROM t1 GROUP BY c,b,d;
> +EXPLAIN SELECT c,b,d FROM t1 GROUP BY c,b,d;
> +EXPLAIN SELECT c,b,d FROM t1 GROUP BY c,b,d ORDER BY NULL;

Please add a select for the statement above so we test result
order. If the test is properly designed.

> +EXPLAIN SELECT c,b,d FROM t1 ORDER BY c,b,d;

The last statement is the same as the second one.

> +
> +DROP TABLE t1;
> diff -Nrup a/sql/sql_select.cc b/sql/sql_select.cc
> --- a/sql/sql_select.cc	2007-07-31 13:24:13 +02:00
> +++ b/sql/sql_select.cc	2007-08-23 17:10:42 +02:00
> @@ -1030,6 +1030,7 @@ JOIN::optimize()
>                                   find_field_in_order_list,
>                                   (void *) group_list))
>      {

Please add a comment here because the effect of this statement is
not obvious.

> +      order= skip_sort_order ? 0 : group_list;
>        group_list= 0;
>        group= 0;
>      }