Fixed MDEV-5424: SELECT using ORDER BY DESC and LIMIT produces unexpected results (InnoDB/XtraDB)

This only happend when using an ORDER BY on a primary key part, where all other key parts where constant.
Remove of duplicated expressions in ORDER BY (as the old code did this in some strange cases)


mysql-test/r/group_by.result:
  Fixed results to take into account that duplicate order by parts are now deleted
mysql-test/r/group_by_innodb.result:
  Ensure extended keys are on
mysql-test/r/innodb_ext_key.result:
  More tests
mysql-test/r/order_by.result:
  More tests
mysql-test/t/group_by.test:
  Fixed results to take into account that duplicate order by parts are now deleted
mysql-test/t/group_by_innodb.test:
  Ensure extended keys are on
mysql-test/t/innodb_ext_key.test:
  More tests
mysql-test/t/order_by.test:
  More tests
sql/sql_select.cc:
  Fixed bug where we looked at extended key parts when we shouldn't
  Remove of duplicated expressions in ORDER BY
sql/table.cc:
  Indentation fixes

mysql-test/r/group_by.result

@@ -1957,12 +1957,12 @@ UNIQUE INDEX idx (col1));
 INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10),
 (11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
 EXPLAIN SELECT col1 AS field1, col1 AS field2
-FROM t1 GROUP BY field1, field2;;
+FROM t1 GROUP BY field1, field2+0;;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index; Using temporary; Using filesort
 FLUSH STATUS;
 SELECT col1 AS field1, col1 AS field2
-FROM t1 GROUP BY field1, field2;;
+FROM t1 GROUP BY field1, field2+0;;
 field1	field2
 1	1
 2	2
@@ -2054,8 +2054,12 @@ field1	field2
 explain
 select col1 f1, col1 f2 from t1 order by f2, f1;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index
+explain
+select col1 f1, col1 f2 from t1 order by f2, f1+0;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index; Using filesort
-select col1 f1, col1 f2 from t1 order by f2, f1;
+select col1 f1, col1 f2 from t1 order by f2, f1+0;
 f1	f2
 1	1
 2	2
@@ -2080,7 +2084,7 @@ f1	f2
 explain
 select col1 f1, col1 f2 from t1 group by f2 order by f2, f1;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
-1	SIMPLE	t1	range	NULL	idx	5	NULL	7	Using index for group-by; Using temporary; Using filesort
+1	SIMPLE	t1	range	NULL	idx	5	NULL	7	Using index for group-by
 select col1 f1, col1 f2 from t1 group by f2 order by f2, f1;
 f1	f2
 1	1
@@ -2106,7 +2110,7 @@ f1	f2
 explain
 select col1 f1, col1 f2 from t1 group by f1, f2 order by f2, f1;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
-1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index; Using temporary; Using filesort
+1	SIMPLE	t1	index	NULL	idx	5	NULL	20	Using index
 select col1 f1, col1 f2 from t1 group by f1, f2 order by f2, f1;
 f1	f2
 1	1
@@ -2137,10 +2141,10 @@ INSERT INTO t2(col1, col2) VALUES
 (1,20),(2,19),(3,18),(4,17),(5,16),(6,15),(7,14),(8,13),(9,12),(10,11),
 (11,10),(12,9),(13,8),(14,7),(15,6),(16,5),(17,4),(18,3),(19,2),(20,1);
 explain
-select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3;
+select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t2	index	NULL	idx	10	NULL	20	Using index; Using temporary; Using filesort
-select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3;
+select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
 f1	f2	f3
 1	20	1
 2	19	2
@@ -2163,10 +2167,10 @@ f1	f2	f3
 19	2	19
 20	1	20
 explain
-select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3;
+select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t2	index	NULL	idx	10	NULL	20	Using index; Using filesort
-select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3;
+select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;
 f1	f2	f3
 1	20	1
 2	19	2

mysql-test/r/group_by_innodb.result

+set @save_ext_key_optimizer_switch=@@optimizer_switch;
 #
 # MDEV-3992 Server crash or valgrind errors in test_if_skip_sort_order/test_if_cheaper_ordering
 # on GROUP BY with indexes on InnoDB table
@@ -7,13 +8,14 @@ pk INT PRIMARY KEY,
 a VARCHAR(1) NOT NULL,
 KEY (pk)
 ) ENGINE=InnoDB;
+set optimizer_switch='extended_keys=on';
 INSERT INTO t1 VALUES (1,'a'),(2,'b');
 EXPLAIN
 SELECT COUNT(*), pk field1, pk AS field2
 FROM t1 WHERE a = 'r' OR pk = 183 
 GROUP BY field1, field2;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
-1	SIMPLE	t1	index	PRIMARY,pk	pk	4	NULL	2	Using where
+1	SIMPLE	t1	index	PRIMARY,pk	PRIMARY	4	NULL	2	Using where
 SELECT COUNT(*), pk field1, pk AS field2
 FROM t1 WHERE a = 'r' OR pk = 183 
 GROUP BY field1, field2;
@@ -22,9 +24,10 @@ EXPLAIN
 SELECT COUNT(*), pk field1 FROM t1
 WHERE a = 'r' OR pk = 183 GROUP BY field1, field1;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
-1	SIMPLE	t1	index	PRIMARY,pk	pk	4	NULL	2	Using where
+1	SIMPLE	t1	index	PRIMARY,pk	PRIMARY	4	NULL	2	Using where
 SELECT COUNT(*), pk field1 FROM t1
 WHERE a = 'r' OR pk = 183 GROUP BY field1, field1;
 COUNT(*)	field1
 drop table t1;
 End of 5.5 tests
+set optimizer_switch=@save_ext_key_optimizer_switch;

mysql-test/r/innodb_ext_key.result

@@ -987,6 +987,54 @@ id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t2	ref	page_timestamp	page_timestamp	4	const	10	Using where
 1	SIMPLE	t3	eq_ref	PRIMARY	PRIMARY	4	test.t2.rev_text_id	1	
 DROP TABLE t1,t2,t3;
+#
+# MDEV-5424 SELECT using ORDER BY DESC and LIMIT produces unexpected
+# results (InnoDB/XtraDB)
+#
+create table t1 (a bigint not null unique auto_increment, b varchar(10), primary key (a), key (b(2))) engine = myisam default character set utf8;
+create table t2 (a bigint not null unique auto_increment, b varchar(10), primary key (a), key (b(2))) engine = innodb default character set utf8;
+insert into t1 (b) values (null), (null), (null);
+insert into t2 (b) values (null), (null), (null);
+set optimizer_switch='extended_keys=on';
+explain select a from t1 where b is null order by a desc limit 2;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	ref	b	b	9	const	2	Using where; Using filesort
+select a from t1 where b is null order by a desc limit 2;
+a
+3
+2
+explain select a from t2 where b is null order by a desc limit 2;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	b	b	9	NULL	3	Using where; Using filesort
+select a from t2 where b is null order by a desc limit 2;
+a
+3
+2
+set optimizer_switch='extended_keys=off';
+explain select a from t2 where b is null order by a desc limit 2;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	b	b	9	NULL	3	Using where; Using filesort
+select a from t2 where b is null order by a desc limit 2;
+a
+3
+2
+explain select a from t2 where b is null order by a desc;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	index	b	PRIMARY	8	NULL	3	Using where
+select a from t2 where b is null order by a desc;
+a
+3
+2
+1
+explain select a from t2 where b is null order by a desc,a,a;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	index	b	PRIMARY	8	NULL	3	Using where
+select a from t2 where b is null order by a desc,a,a;
+a
+3
+2
+1
+drop table t1, t2;
 set optimizer_switch=@save_optimizer_switch;
 set optimizer_switch=@save_ext_key_optimizer_switch;
 SET SESSION STORAGE_ENGINE=DEFAULT;

mysql-test/r/order_by.result

@@ -357,6 +357,12 @@ id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 explain select * from t1 where a = 1 order by b desc;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	SIMPLE	t1	ref	a	a	4	const	5	Using where; Using index
+explain select * from t1 where a = 2 and b > 0 order by a desc,b desc,b,a;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	range	a	a	9	NULL	5	Using where; Using index
+explain select * from t1 where a = 2 and b < 2 order by a desc,a,b desc,a,b;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	range	a	a	9	NULL	2	Using where; Using index
 select * from t1 where a = 1 order by b desc;
 a	b	c
 1	3	b

mysql-test/t/group_by.test

@@ -1336,7 +1336,7 @@ INSERT INTO t1 VALUES (1),(2),(3),(4),(5),(6),(7),(8),(9),(10),
    (11),(12),(13),(14),(15),(16),(17),(18),(19),(20);
 
 let $query0=SELECT col1 AS field1, col1 AS field2
-            FROM t1 GROUP BY field1, field2;
+            FROM t1 GROUP BY field1, field2+0;
 
 # Needs to be range to exercise bug
 --eval EXPLAIN $query0;
@@ -1366,7 +1366,9 @@ LIMIT 3;
 
 explain
 select col1 f1, col1 f2 from t1 order by f2, f1;
-select col1 f1, col1 f2 from t1 order by f2, f1;
+explain
+select col1 f1, col1 f2 from t1 order by f2, f1+0;
+select col1 f1, col1 f2 from t1 order by f2, f1+0;
 
 explain
 select col1 f1, col1 f2 from t1 group by f2 order by f2, f1;
@@ -1386,12 +1388,12 @@ INSERT INTO t2(col1, col2) VALUES
    (11,10),(12,9),(13,8),(14,7),(15,6),(16,5),(17,4),(18,3),(19,2),(20,1);
 
 explain
-select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3;
-select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3;
+select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
+select col1 f1, col2 f2, col1 f3 from t2 group by f1, f2, f3+0;
 
 explain
-select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3;
-select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3;
+select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;
+select col1 f1, col2 f2, col1 f3 from t2 order by f1, f2, f3+0;
 
 DROP VIEW v1;
 DROP TABLE t1, t2;

mysql-test/t/group_by_innodb.test

@@ -4,6 +4,8 @@
 
 --source include/have_innodb.inc
 
+set @save_ext_key_optimizer_switch=@@optimizer_switch;
+
 --echo #
 --echo # MDEV-3992 Server crash or valgrind errors in test_if_skip_sort_order/test_if_cheaper_ordering
 --echo # on GROUP BY with indexes on InnoDB table
@@ -15,6 +17,8 @@ CREATE TABLE t1 (
   KEY (pk)
 ) ENGINE=InnoDB;
 
+set optimizer_switch='extended_keys=on';
+
 INSERT INTO t1 VALUES (1,'a'),(2,'b');
 
 EXPLAIN
@@ -36,3 +40,5 @@ WHERE a = 'r' OR pk = 183 GROUP BY field1, field1;
 drop table t1;
 
 --echo End of 5.5 tests
+
+set optimizer_switch=@save_ext_key_optimizer_switch;

mysql-test/t/innodb_ext_key.test

@@ -662,6 +662,34 @@ ORDER BY rev_timestamp ASC LIMIT 10;
 
 DROP TABLE t1,t2,t3;
 
+--echo #
+--echo # MDEV-5424 SELECT using ORDER BY DESC and LIMIT produces unexpected
+--echo # results (InnoDB/XtraDB)
+--echo #
+
+create table t1 (a bigint not null unique auto_increment, b varchar(10), primary key (a), key (b(2))) engine = myisam default character set utf8;
+create table t2 (a bigint not null unique auto_increment, b varchar(10), primary key (a), key (b(2))) engine = innodb default character set utf8;
+
+insert into t1 (b) values (null), (null), (null);
+insert into t2 (b) values (null), (null), (null);
+
+set optimizer_switch='extended_keys=on';
+explain select a from t1 where b is null order by a desc limit 2;
+select a from t1 where b is null order by a desc limit 2;
+explain select a from t2 where b is null order by a desc limit 2;
+select a from t2 where b is null order by a desc limit 2;
+set optimizer_switch='extended_keys=off';
+explain select a from t2 where b is null order by a desc limit 2;
+select a from t2 where b is null order by a desc limit 2;
+
+explain select a from t2 where b is null order by a desc;
+select a from t2 where b is null order by a desc;
+
+explain select a from t2 where b is null order by a desc,a,a;
+select a from t2 where b is null order by a desc,a,a;
+
+drop table t1, t2;
+
 set optimizer_switch=@save_optimizer_switch;
 
 set optimizer_switch=@save_ext_key_optimizer_switch;

mysql-test/t/order_by.test

@@ -261,6 +261,9 @@ desc,b desc;
 explain select * from t1 where a = 2 and b > 0 order by a desc,b desc;
 explain select * from t1 where a = 2 and b < 2 order by a desc,b desc;
 explain select * from t1 where a = 1 order by b desc;
+explain select * from t1 where a = 2 and b > 0 order by a desc,b desc,b,a;
+explain select * from t1 where a = 2 and b < 2 order by a desc,a,b desc,a,b;
+
 select * from t1 where a = 1 order by b desc;
 #
 # Test things when we don't have NULL keys

sql/sql_select.cc

@@ -11038,6 +11038,8 @@ static void update_depend_map_for_order(JOIN *join, ORDER *order)
   Remove all constants and check if ORDER only contains simple
   expressions.
 
+  We also remove all duplicate expressions, keeping only the first one.
+
   simple_order is set to 1 if sort_order only uses fields from head table
   and the head table is not a LEFT JOIN table.
 
@@ -11045,9 +11047,10 @@ static void update_depend_map_for_order(JOIN *join, ORDER *order)
   @param first_order		List of SORT or GROUP order
   @param cond			WHERE statement
   @param change_list		Set to 1 if we should remove things from list.
-                               If this is not set, then only simple_order is
-                               calculated.
-  @param simple_order		Set to 1 if we are only using simple expressions
+                                If this is not set, then only simple_order is
+                                calculated.
+  @param simple_order		Set to 1 if we are only using simple
+				expressions.
 
   @return
     Returns new sort order
@@ -11060,7 +11063,7 @@ remove_const(JOIN *join,ORDER *first_order, COND *cond,
   if (join->table_count == join->const_tables)
     return change_list ? 0 : first_order;		// No need to sort
 
-  ORDER *order,**prev_ptr;
+  ORDER *order,**prev_ptr, *tmp_order;
   table_map first_table;
   table_map not_const_tables= ~join->const_table_map;
   table_map ref;
@@ -11074,7 +11077,6 @@ remove_const(JOIN *join,ORDER *first_order, COND *cond,
     first_is_base_table= TRUE;
   }
   
-
   /*
     Cleanup to avoid interference of calls of this function for
     ORDER BY and GROUP BY
@@ -11143,6 +11145,17 @@ remove_const(JOIN *join,ORDER *first_order, COND *cond,
 	}
       }
     }
+    /* Remove ORDER BY entries that we have seen before */
+    for (tmp_order= first_order;
+         tmp_order != order;
+         tmp_order= tmp_order->next)
+    {
+      if (tmp_order->item[0]->eq(order->item[0],1))
+        break;
+    }
+    if (tmp_order != order)
+      continue;                                // Duplicate order by. Remove
+    
     if (change_list)
       *prev_ptr= order;				// use this entry
     prev_ptr= &order->next;
@@ -18829,7 +18842,7 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
         key as a suffix to the secondary keys. If it has continue to check
         the primary key as a suffix.
       */
-      if (!on_pk_suffix &&
+      if (!on_pk_suffix && (table->key_info[idx].ext_key_part_map & 1) &&
           (table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) &&
           table->s->primary_key != MAX_KEY &&
           table->s->primary_key != idx)
@@ -18853,20 +18866,22 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
                 (((key_part_map) 1) << pk_part_idx)))
             break;
         }
+
         /* Adjust const_key_parts */
         const_key_parts&= (((key_part_map) 1) << pk_part_idx) -1;
 
-         for (; const_key_parts & 1 ; const_key_parts>>= 1)
-           key_part++; 
+        for (; const_key_parts & 1 ; const_key_parts>>= 1)
+          key_part++;
         /*
           Test if the primary key parts were all const (i.e. there's one row).
           The sorting doesn't matter.
         */
-        if (key_part == start+table->key_info[table->s->primary_key].key_parts &&
+        if (key_part ==
+            start+table->key_info[table->s->primary_key].key_parts &&
             reverse == 0)
         {
           key_parts= 0;
-          reverse= 1;
+          reverse= 1;                           // Key is ok to use
           goto ok;
         }
       }

sql/table.cc

@@ -734,14 +734,10 @@ err_not_open:
 }
 
 
-static bool create_key_infos(uchar *strpos, uint keys, KEY *keyinfo, uint new_frm_ver,
+static bool create_key_infos(uchar *strpos, uint keys, KEY *keyinfo,
+                             uint new_frm_ver,
                              uint &ext_key_parts, TABLE_SHARE *share, uint len,
-                             KEY *first_keyinfo, char* &keynames
-                             
-                             
-                             
-                             
-                             )
+                             KEY *first_keyinfo, char* &keynames)
 {
   uint i, j, n_length;
   KEY_PART_INFO *key_part= NULL;
@@ -848,7 +844,6 @@ static bool create_key_infos(uchar *strpos, uint keys, KEY *keyinfo, uint new_fr
     keyinfo->ext_key_part_map= 0;
     if (share->use_ext_keys && i)
     {
-      keyinfo->ext_key_part_map= 0;
       for (j= 0; 
            j < first_key_parts && keyinfo->ext_key_parts < MAX_REF_PARTS;
            j++)
@@ -1148,7 +1143,8 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
 
     share->set_use_ext_keys_flag(share->db_type()->flags & HTON_EXTENDED_KEYS);
 
-    if (create_key_infos(disk_buff + 6, keys, keyinfo, new_frm_ver, ext_key_parts,
+    if (create_key_infos(disk_buff + 6, keys, keyinfo, new_frm_ver,
+                         ext_key_parts,
                          share, len, &first_keyinfo, keynames))
       goto err;
 
@@ -1242,7 +1238,8 @@ static int open_binary_frm(THD *thd, TABLE_SHARE *share, uchar *head,
   }
   else
   {
-    if (create_key_infos(disk_buff + 6, keys, keyinfo, new_frm_ver, ext_key_parts,
+    if (create_key_infos(disk_buff + 6, keys, keyinfo, new_frm_ver,
+                         ext_key_parts,
                          share, len, &first_keyinfo, keynames))
       goto err;
   }