MDEV-6402: Optimizer doesn't choose best execution plan when composite...bb-10.1-mdev6402

Fix test_if_skip_sort_order() logic:
WHEN
we use index X, which doesn't produce needed ordering, but there is
an index Y which does and has the same prefix as used prefix of X
THEN
don't just switch to using ref access on Y. If range(Y) would use more
key parts, use range(Y).

3 files changed, 149 insertions, 28 deletions

diff --git a/mysql-test/r/order_by_optimizer_innodb.result b/mysql-test/r/order_by_optimizer_innodb.result
new file mode 100644
index 00000000000..212139a287e
--- /dev/null
+++ b/mysql-test/r/order_by_optimizer_innodb.result
@@ -0,0 +1,48 @@
+drop table if exists t0,t1,t2,t3;
+#
+# MDEV-6402: Optimizer doesn't choose best execution plan when composite key is used
+#
+create table t0(a int);
+insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
+create table t1(a int);
+insert into t1 select A.a + B.a* 10 + C.a * 100 from t0 A, t0 B, t0 C;
+CREATE TABLE t2 (
+pk1 int(11) NOT NULL,
+pk2 int(11) NOT NULL,
+fd5 bigint(20) DEFAULT NULL,
+filler1 char(200),
+filler2 char(200),
+PRIMARY KEY (pk1,pk2),
+UNIQUE KEY ux_pk1_fd5 (pk1,fd5)
+) ENGINE=InnoDB;
+insert into t2 
+select 
+round(log(2,t1.a+1)),
+t1.a,
+t1.a,
+REPEAT('filler-data-', 10),
+REPEAT('filler-data-', 10)
+from 
+t1;
+select pk1, count(*) from t2 group by pk1;
+pk1	count(*)
+0	1
+1	1
+2	3
+3	6
+4	11
+5	23
+6	45
+7	91
+8	181
+9	362
+10	276
+# The following should use range(ux_pk1_fd5), two key parts (key_len=5+8=13)
+EXPLAIN SELECT * FROM t2 USE INDEX(ux_pk1_fd5) WHERE pk1=9 AND fd5 < 500 ORDER BY fd5 DESC LIMIT 10;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	ux_pk1_fd5	ux_pk1_fd5	13	NULL	137	Using where
+# This also must use range, not ref. key_len must be 13
+EXPLAIN SELECT * FROM t2                       WHERE pk1=9 AND fd5 < 500 ORDER BY fd5 DESC LIMIT 10;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	range	PRIMARY,ux_pk1_fd5	ux_pk1_fd5	13	NULL	137	Using where
+drop table t0,t1, t2;
diff --git a/mysql-test/t/order_by_optimizer_innodb.test b/mysql-test/t/order_by_optimizer_innodb.test
new file mode 100644
index 00000000000..d3c71e8772f
--- /dev/null
+++ b/mysql-test/t/order_by_optimizer_innodb.test
@@ -0,0 +1,43 @@
+--source include/have_innodb.inc
+
+--disable_warnings
+drop table if exists t0,t1,t2,t3;
+--enable_warnings
+
+--echo #
+--echo # MDEV-6402: Optimizer doesn't choose best execution plan when composite key is used
+--echo #
+create table t0(a int);
+insert into t0 values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9);
+
+create table t1(a int);
+insert into t1 select A.a + B.a* 10 + C.a * 100 from t0 A, t0 B, t0 C;
+
+CREATE TABLE t2 (
+   pk1 int(11) NOT NULL,
+   pk2 int(11) NOT NULL,
+   fd5 bigint(20) DEFAULT NULL,
+   filler1 char(200),
+   filler2 char(200),
+   PRIMARY KEY (pk1,pk2),
+   UNIQUE KEY ux_pk1_fd5 (pk1,fd5)
+ ) ENGINE=InnoDB;
+
+insert into t2 
+select 
+  round(log(2,t1.a+1)),
+  t1.a,
+  t1.a,
+  REPEAT('filler-data-', 10),
+  REPEAT('filler-data-', 10)
+from 
+  t1;
+
+select pk1, count(*) from t2 group by pk1;
+
+--echo # The following should use range(ux_pk1_fd5), two key parts (key_len=5+8=13)
+EXPLAIN SELECT * FROM t2 USE INDEX(ux_pk1_fd5) WHERE pk1=9 AND fd5 < 500 ORDER BY fd5 DESC LIMIT 10;
+--echo # This also must use range, not ref. key_len must be 13
+EXPLAIN SELECT * FROM t2                       WHERE pk1=9 AND fd5 < 500 ORDER BY fd5 DESC LIMIT 10;
+
+drop table t0,t1, t2;
diff --git a/sql/sql_select.cc b/sql/sql_select.cc
index 9c87ddcc4b4..442f8536fb6 100644
--- a/sql/sql_select.cc
+++ b/sql/sql_select.cc
@@ -19868,7 +19868,12 @@ test_if_subkey(ORDER *order, TABLE *table, uint ref, uint ref_key_parts,
   uint best= MAX_KEY;
   KEY_PART_INFO *ref_key_part= table->key_info[ref].key_part;
   KEY_PART_INFO *ref_key_part_end= ref_key_part + ref_key_parts;
-
+  
+  /*
+    Find the shortest key that
+    - produces the required ordering
+    - has key #ref (up to ref_key_parts) as its subkey.
+  */
   for (nr= 0 ; nr < table->s->keys ; nr++)
   {
     if (usable_keys->is_set(nr) &&
@@ -20061,7 +20066,8 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
     been taken into account.
   */
   usable_keys= *map;
-
+  
+  /* Find indexes that cover all ORDER/GROUP BY fields */
   for (ORDER *tmp_order=order; tmp_order ; tmp_order=tmp_order->next)
   {
     Item *item= (*tmp_order->item)->real_item();
@@ -20081,6 +20087,10 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
   {
     ref_key=	   tab->ref.key;
     ref_key_parts= tab->ref.key_parts;
+    /* 
+      todo: why does JT_REF_OR_NULL mean filesort? We could find another index
+      that satisfies the ordering. I would just set ref_key=MAX_KEY here...
+    */
     if (tab->type == JT_REF_OR_NULL || tab->type == JT_FT)
       goto use_filesort;
   }
@@ -20107,15 +20117,12 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
 
   if (ref_key >= 0 && ref_key != MAX_KEY)
   {
-    /*
-      We come here when there is a REF key.
-    */
+    /* Current access method uses index ref_key with ref_key_parts parts */
     if (!usable_keys.is_set(ref_key))
     {
-      /*
-	We come here when ref_key is not among usable_keys
-      */
+      /* However, ref_key doesn't match the needed ordering */
       uint new_ref_key;
+
       /*
 	If using index only read, only consider other possible index only
 	keys
@@ -20131,27 +20138,23 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
       if ((new_ref_key= test_if_subkey(order, table, ref_key, ref_key_parts,
 				       &usable_keys)) < MAX_KEY)
       {
-	if (tab->ref.key >= 0)
-	{
-          /*
-            We'll use ref access method on key new_ref_key. In general case 
-            the index search tuple for new_ref_key will be different (e.g.
-            when one index is defined as (part1, part2, ...) and another as
-            (part1, part2(N), ...) and the WHERE clause contains 
-            "part1 = const1 AND part2=const2". 
-            So we build tab->ref from scratch here.
-          */
-          KEYUSE *keyuse= tab->keyuse;
-          while (keyuse->key != new_ref_key && keyuse->table == tab->table)
-            keyuse++;
-          if (create_ref_for_key(tab->join, tab, keyuse, FALSE,
-                                 (tab->join->const_table_map |
-                                  OUTER_REF_TABLE_BIT)))
-            goto use_filesort;
+        /*
+          Index new_ref_key 
+          - produces the required ordering, 
+          - also has the same columns as ref_key for #ref_key_parts (this
+            means we will read the same number of rows as with ref_key).
+        */
 
-          pick_table_access_method(tab);
-	}
-	else
+        /*
+          If new_ref_key allows to construct a quick select which uses more key
+          parts than ref(new_ref_key) would, do that.
+
+          Otherwise, construct a ref access (todo: it's not clear what is the
+          win in using ref access when we could use quick select also?)
+        */
+        if ((table->quick_keys.is_set(new_ref_key) && 
+             table->quick_key_parts[new_ref_key] > ref_key_parts) ||
+             !(tab->ref.key >= 0))
 	{
           /*
             The range optimizer constructed QUICK_RANGE for ref_key, and
@@ -20183,12 +20186,39 @@ test_if_skip_sort_order(JOIN_TAB *tab,ORDER *order,ha_rows select_limit,
             select->cond= save_cond;
             goto use_filesort;
           }
+          DBUG_ASSERT(tab->select->quick);
+          tab->type= JT_ALL;
+          tab->ref.key= -1;
+          tab->ref.key_parts= 0;
+          tab->use_quick= 1;
           /*
             We don't restore select->cond as we want to use the
             original condition as index condition pushdown is not
             active for the new index.
+            todo: why not perform index condition pushdown for the new index?
+          */
+	}
+        else
+	{
+          /*
+            We'll use ref access method on key new_ref_key. In general case 
+            the index search tuple for new_ref_key will be different (e.g.
+            when one index is defined as (part1, part2, ...) and another as
+            (part1, part2(N), ...) and the WHERE clause contains 
+            "part1 = const1 AND part2=const2". 
+            So we build tab->ref from scratch here.
           */
+          KEYUSE *keyuse= tab->keyuse;
+          while (keyuse->key != new_ref_key && keyuse->table == tab->table)
+            keyuse++;
+          if (create_ref_for_key(tab->join, tab, keyuse, FALSE,
+                                 (tab->join->const_table_map |
+                                  OUTER_REF_TABLE_BIT)))
+            goto use_filesort;
+
+          pick_table_access_method(tab);
 	}
+
         ref_key= new_ref_key;
         changed_key= true;
      }