List:Commits« Previous MessageNext Message »
From:Alexey Kopytov Date:November 9 2007 4:09pm
Subject:bk commit into 5.0 tree (kaa:1.2550)
View as plain text  
Below is the list of changes that have just been committed into a local
5.0 repository of kaa. When kaa 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-11-09 19:09:14+03:00, kaa@polly.(none) +3 -0
  Fix for bug 32202: ORDER BY not working with GROUP BY
  
  The bug is a regression introduced by the fix for bug30596. The problem
  was that in cases when groups in GROUP BY correspond to only one row,
  and there is ORDER BY, the GROUP BY was removed and the ORDER BY
  rewritten to ORDER BY <group_by_columns> without checking if the
  columns in GROUP BY and ORDER BY are compatible. This led to
  incorrect ordering of the result set as it was sorted using the
  GROUP BY columns. Additionaly, the code discarded ASC/DESC modifiers
  from ORDER BY even if its columns were compatible with the GROUP BY
  ones.
  
  This patch fixes the regression by checking if ORDER BY columns form a
  prefix of the GROUP BY ones, and rewriting ORDER BY only in that case,
  preserving the ASC/DESC modifiers. That check is sufficient, since the
  GROUP BY columns contain a unique index.

  mysql-test/r/group_by.result@stripped, 2007-11-09 19:09:08+03:00, kaa@polly.(none) +65 -0
    Added a test case for bug #32202.

  mysql-test/t/group_by.test@stripped, 2007-11-09 19:09:08+03:00, kaa@polly.(none) +35 -0
    Added a test case for bug #32202.

  sql/sql_select.cc@stripped, 2007-11-09 19:09:08+03:00, kaa@polly.(none) +12 -3
    In cases when groups in GROUP BY correspond to only one row and there
    is ORDER BY, rewrite the query to ORDER BY <group_by_columns> only if
    the columns in ORDER BY and GROUP BY are compatible, i.e. either one
    forms a prefix for another.

diff -Nrup a/mysql-test/r/group_by.result b/mysql-test/r/group_by.result
--- a/mysql-test/r/group_by.result	2007-08-27 19:33:38 +04:00
+++ b/mysql-test/r/group_by.result	2007-11-09 19:09:08 +03:00
@@ -1113,3 +1113,68 @@ c	b
 3	1
 3	2
 DROP TABLE t1;
+CREATE TABLE t1(
+id INT AUTO_INCREMENT PRIMARY KEY, 
+c1 INT NOT NULL, 
+c2 INT NOT NULL,
+UNIQUE KEY (c2,c1));
+INSERT INTO t1(c1,c2) VALUES (5,1), (4,1), (3,5), (2,3), (1,3);
+SELECT * FROM t1 ORDER BY c1;
+id	c1	c2
+5	1	3
+4	2	3
+3	3	5
+2	4	1
+1	5	1
+SELECT * FROM t1 GROUP BY id ORDER BY c1;
+id	c1	c2
+5	1	3
+4	2	3
+3	3	5
+2	4	1
+1	5	1
+SELECT * FROM t1 GROUP BY id ORDER BY id DESC;
+id	c1	c2
+5	1	3
+4	2	3
+3	3	5
+2	4	1
+1	5	1
+SELECT * FROM t1 GROUP BY c2 ,c1, id ORDER BY c2, c1;
+id	c1	c2
+2	4	1
+1	5	1
+5	1	3
+4	2	3
+3	3	5
+SELECT * FROM t1 GROUP BY c2, c1, id ORDER BY c2 DESC, c1;
+id	c1	c2
+3	3	5
+5	1	3
+4	2	3
+2	4	1
+1	5	1
+SELECT * FROM t1 GROUP BY c2, c1, id ORDER BY c2 DESC, c1 DESC;
+id	c1	c2
+3	3	5
+4	2	3
+5	1	3
+1	5	1
+2	4	1
+SELECT * FROM t1 GROUP BY c2  ORDER BY c2, c1;
+id	c1	c2
+1	5	1
+4	2	3
+3	3	5
+SELECT * FROM t1 GROUP BY c2  ORDER BY c2 DESC, c1;
+id	c1	c2
+3	3	5
+4	2	3
+1	5	1
+SELECT * FROM t1 GROUP BY c2  ORDER BY c2 DESC, c1 DESC;
+id	c1	c2
+3	3	5
+4	2	3
+1	5	1
+DROP TABLE t1;
+End of 5.0 tests
diff -Nrup a/mysql-test/t/group_by.test b/mysql-test/t/group_by.test
--- a/mysql-test/t/group_by.test	2007-08-27 19:33:38 +04:00
+++ b/mysql-test/t/group_by.test	2007-11-09 19:09:08 +03:00
@@ -815,3 +815,38 @@ EXPLAIN SELECT c,b   FROM t1 GROUP BY c,
 SELECT c,b   FROM t1 GROUP BY c,b;
 
 DROP TABLE t1;
+
+#
+# Bug #32202: ORDER BY not working with GROUP BY
+#
+
+CREATE TABLE t1(
+  id INT AUTO_INCREMENT PRIMARY KEY, 
+  c1 INT NOT NULL, 
+  c2 INT NOT NULL,
+  UNIQUE KEY (c2,c1));
+
+INSERT INTO t1(c1,c2) VALUES (5,1), (4,1), (3,5), (2,3), (1,3);
+
+# Show that the test cases from the bug report pass
+SELECT * FROM t1 ORDER BY c1;
+SELECT * FROM t1 GROUP BY id ORDER BY c1;
+
+# Show that DESC is handled correctly
+SELECT * FROM t1 GROUP BY id ORDER BY id DESC;
+
+# Show that results are correctly ordered when ORDER BY fields
+# are a subset of GROUP BY ones
+SELECT * FROM t1 GROUP BY c2 ,c1, id ORDER BY c2, c1;
+SELECT * FROM t1 GROUP BY c2, c1, id ORDER BY c2 DESC, c1;
+SELECT * FROM t1 GROUP BY c2, c1, id ORDER BY c2 DESC, c1 DESC;
+
+# Show that results are correctly ordered when GROUP BY fields
+# are a subset of ORDER BY ones
+SELECT * FROM t1 GROUP BY c2  ORDER BY c2, c1;
+SELECT * FROM t1 GROUP BY c2  ORDER BY c2 DESC, c1;
+SELECT * FROM t1 GROUP BY c2  ORDER BY c2 DESC, c1 DESC;
+
+DROP TABLE t1;
+
+--echo End of 5.0 tests
diff -Nrup a/sql/sql_select.cc b/sql/sql_select.cc
--- a/sql/sql_select.cc	2007-10-02 18:45:48 +04:00
+++ b/sql/sql_select.cc	2007-11-09 19:09:08 +03:00
@@ -1057,10 +1057,19 @@ JOIN::optimize()
         We have found that grouping can be removed since groups correspond to
         only one row anyway, but we still have to guarantee correct result
         order. The line below effectively rewrites the query from GROUP BY
-        <fields> to ORDER BY <fields>. One exception is if skip_sort_order is
-        set (see above), then we can simply skip GROUP BY.
+        <fields> to ORDER BY <fields>. There are two exceptions:
+        - if skip_sort_order is set (see above), then we can simply skip
+          GROUP BY;
+        - we can only rewrite ORDER BY if the ORDER BY fields are 'compatible'
+          with the GROUP BY ones, i.e. either one is a prefix of another.
+          We only check if the ORDER BY is a prefix of GROUP BY. In this case
+          test_if_subpart() copies the ASC/DESC attributes from the original
+          ORDER BY fields.
+          If GROUP BY is a prefix of ORDER BY, then it is safe to leave
+          'order' as is.
        */
-      order= skip_sort_order ? 0 : group_list;
+      if (!order || test_if_subpart(group_list, order))
+          order= skip_sort_order ? 0 : group_list;
       group_list= 0;
       group= 0;
     }
Thread
bk commit into 5.0 tree (kaa:1.2550)Alexey Kopytov9 Nov