MDEV-6796: Unable to skip filesort when using implicit extended key

Re-work test_if_order_by_key() to work correctly for extended indexes.

mysql-test/r/order_by_optimizer_innodb.result

@@ -46,3 +46,45 @@ EXPLAIN SELECT * FROM t2                       WHERE pk1=9 AND fd5 < 500 ORDER B
 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;
+#
+# MDEV-6796: Unable to skip filesort when using implicit extended key
+#
+CREATE TABLE t1 (
+pk1 int(11) NOT NULL,
+pk2 varchar(64) NOT NULL,
+col1 varchar(16) DEFAULT NULL,
+PRIMARY KEY (pk1,pk2),
+KEY key1 (pk1,col1)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+CREATE TABLE t2 (
+pk1 int(11) NOT NULL,
+pk2 varchar(64) NOT NULL,
+col1 varchar(16) DEFAULT NULL,
+PRIMARY KEY (pk1,pk2),
+KEY key1 (pk1,col1,pk2)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+INSERT INTO `t1` VALUES 
+(12321321,'a8f5f167f44f4964e6c998dee827110c','video'),
+(12321321,'d77a17a3659ffa60c54e0ea17b6c6d16','video'),
+(12321321,'wwafdsafdsafads','video'),
+(12321321,'696aa249f0738e8181957dd57c2d7d0b','video-2014-09-23'),
+(12321321,'802f9f29584b486f356693e3aa4ef0af','video=sdsd'),
+(12321321,'2f94543ff74aab82e9a058b4e8316d75','video=sdsdsds'),
+(12321321,'c1316b9df0d203fd1b9035308de52a0a','video=sdsdsdsdsd');
+insert into t2 select * from t1;
+# this must not use filesort:
+explain SELECT pk2
+FROM t1 USE INDEX(key1)
+WHERE pk1 = 123
+AND col1 = 'video'
+ORDER BY pk2 DESC LIMIT 21;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	ref	key1	key1	55	const,const	1	Using where; Using index
+# this must not use filesort, either:
+explain SELECT pk2
+FROM t2 USE INDEX(key1)
+WHERE pk1 = 123 AND col1 = 'video'
+ORDER BY pk2 DESC LIMIT 21;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t2	ref	key1	key1	55	const,const	1	Using where; Using index
+drop table t1, t2;

mysql-test/t/order_by_optimizer_innodb.test

@@ -41,3 +41,50 @@ EXPLAIN SELECT * FROM t2 USE INDEX(ux_pk1_fd5) WHERE pk1=9 AND fd5 < 500 ORDER B
 EXPLAIN SELECT * FROM t2                       WHERE pk1=9 AND fd5 < 500 ORDER BY fd5 DESC LIMIT 10;
 
 drop table t0,t1, t2;
+
+--echo #
+--echo # MDEV-6796: Unable to skip filesort when using implicit extended key
+--echo #
+
+CREATE TABLE t1 (
+  pk1 int(11) NOT NULL,
+  pk2 varchar(64) NOT NULL,
+  col1 varchar(16) DEFAULT NULL,
+  PRIMARY KEY (pk1,pk2),
+  KEY key1 (pk1,col1)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+CREATE TABLE t2 (
+  pk1 int(11) NOT NULL,
+  pk2 varchar(64) NOT NULL,
+  col1 varchar(16) DEFAULT NULL,
+  PRIMARY KEY (pk1,pk2),
+  KEY key1 (pk1,col1,pk2)
+) ENGINE=InnoDB DEFAULT CHARSET=utf8;
+
+INSERT INTO `t1` VALUES 
+(12321321,'a8f5f167f44f4964e6c998dee827110c','video'),
+(12321321,'d77a17a3659ffa60c54e0ea17b6c6d16','video'),
+(12321321,'wwafdsafdsafads','video'),
+(12321321,'696aa249f0738e8181957dd57c2d7d0b','video-2014-09-23'),
+(12321321,'802f9f29584b486f356693e3aa4ef0af','video=sdsd'),
+(12321321,'2f94543ff74aab82e9a058b4e8316d75','video=sdsdsds'),
+(12321321,'c1316b9df0d203fd1b9035308de52a0a','video=sdsdsdsdsd');
+
+insert into t2 select * from t1;
+
+--echo # this must not use filesort:
+explain SELECT pk2
+FROM t1 USE INDEX(key1)
+WHERE pk1 = 123
+AND col1 = 'video'
+ORDER BY pk2 DESC LIMIT 21;
+
+--echo # this must not use filesort, either:
+explain SELECT pk2
+FROM t2 USE INDEX(key1)
+WHERE pk1 = 123 AND col1 = 'video'
+ORDER BY pk2 DESC LIMIT 21;
+
+drop table t1, t2;
+

sql/field.h

@@ -281,6 +281,12 @@ public:
   LEX_STRING	comment;
   /* Field is part of the following keys */
   key_map	key_start, part_of_key, part_of_key_not_clustered;
+
+  /*
+    Bitmap of indexes that have records ordered by col1, ... this_field, ...
+
+    For example, INDEX (col(prefix_n)) is not present in col.part_of_sortkey.
+  */
   key_map       part_of_sortkey;
   /*
     We use three additional unireg types for TIMESTAMP to overcome limitation

sql/sql_select.cc

@@ -15044,6 +15044,11 @@ remove_eq_conds(THD *thd, COND *cond, Item::cond_result *cond_value)
   @retval true    can be used
   @retval false   cannot be used
 */
+
+/*
+  psergey-todo: this returns false for int_column='1234' (here '1234' is a
+  constant. Need to discuss this with Bar).
+*/
 static bool
 test_if_equality_guarantees_uniqueness(Item *l, Item *r)
 {
@@ -19676,12 +19681,21 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
 {
   KEY_PART_INFO *key_part,*key_part_end;
   key_part=table->key_info[idx].key_part;
-  key_part_end=key_part+table->key_info[idx].user_defined_key_parts;
+  key_part_end=key_part + table->key_info[idx].ext_key_parts;
   key_part_map const_key_parts=table->const_key_parts[idx];
+  uint user_defined_kp= table->key_info[idx].user_defined_key_parts;
   int reverse=0;
   uint key_parts;
-  my_bool on_pk_suffix= FALSE;
+  bool have_pk_suffix= false;
+  uint pk= table->s->primary_key;
   DBUG_ENTER("test_if_order_by_key");
+ 
+  if ((table->file->ha_table_flags() & HA_PRIMARY_KEY_IN_READ_INDEX) && 
+      table->key_info[idx].ext_key_part_map &&
+      pk != MAX_KEY && pk != idx)
+  {
+    have_pk_suffix= true;
+  }
 
   for (; order ; order=order->next, const_key_parts>>=1)
   {
@@ -19694,58 +19708,37 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
     */
     for (; const_key_parts & 1 ; const_key_parts>>= 1)
       key_part++; 
+    
+    /*
+      This check was in this function historically (although I think it's
+      better to check it outside of this function):
 
-    if (key_part >= key_part_end)
-    {
-      /* 
-        We are at the end of the key. Check if the engine has the primary
-        key as a suffix to the secondary keys. If it has continue to check
-        the primary key as a 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)
-      {
-        KEY_PART_INFO *start,*end;
-        uint pk_part_idx= 0;
-        on_pk_suffix= TRUE;
-        start= key_part= table->key_info[table->s->primary_key].key_part;
-        const_key_parts=table->const_key_parts[table->s->primary_key];
-
-        /*
-          Calculate true key_part_end and const_key_parts
-          (we have to stop as first not continous primary key part)
-        */
-        for (key_part_end= key_part,
-             end= key_part+table->key_info[table->s->primary_key].user_defined_key_parts;
-             key_part_end < end; key_part_end++, pk_part_idx++)
-        {
-          /* Found hole in the pk_parts; Abort */
-          if (!(table->key_info[idx].ext_key_part_map &
-                (((key_part_map) 1) << pk_part_idx)))
-            break;
-        }
+      "Test if the primary key parts were all const (i.e. there's one row).
+       The sorting doesn't matter"
 
-        /* Adjust const_key_parts */
-        const_key_parts&= (((key_part_map) 1) << pk_part_idx) -1;
+       So, we're checking that 
+       (1) this is an extended key
+       (2) we've reached its end
+    */
+    key_parts= (key_part - table->key_info[idx].key_part);
+    if (have_pk_suffix &&
+        reverse == 0 && // all were =const so far
+        key_parts == table->key_info[idx].ext_key_parts && 
+        table->const_key_parts[pk] == PREV_BITS(uint, 
+                                                table->key_info[pk].
+                                                user_defined_key_parts))
+    {
+      key_parts= 0;
+      reverse= 1;                           // Key is ok to use
+      goto ok;
+    }
 
-        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].user_defined_key_parts &&
-            reverse == 0)
-        {
-          key_parts= 0;
-          reverse= 1;                           // Key is ok to use
-          goto ok;
-        }
-      }
-      else
-        DBUG_RETURN(0);
+    if (key_part == key_part_end)
+    {
+      /*
+        There are some items left in ORDER BY that we don't
+      */
+      DBUG_RETURN(0);
     }
 
     if (key_part->field != field || !field->part_of_sortkey.is_set(idx))
@@ -19760,27 +19753,20 @@ static int test_if_order_by_key(ORDER *order, TABLE *table, uint idx,
     if (key_part < key_part_end)
       key_part++;
   }
-  if (on_pk_suffix)
-  {
-    uint used_key_parts_secondary= table->key_info[idx].user_defined_key_parts;
-    uint used_key_parts_pk=
-      (uint) (key_part - table->key_info[table->s->primary_key].key_part);
-    key_parts= used_key_parts_pk + used_key_parts_secondary;
 
-    if (reverse == -1 &&
-        (!(table->file->index_flags(idx, used_key_parts_secondary - 1, 1) &
-           HA_READ_PREV) ||
-         !(table->file->index_flags(table->s->primary_key,
-                                    used_key_parts_pk - 1, 1) & HA_READ_PREV)))
-      reverse= 0;                               // Index can't be used
-  }
-  else
+  key_parts= (uint) (key_part - table->key_info[idx].key_part);
+
+  if (reverse == -1 && 
+      !(table->file->index_flags(idx, user_defined_kp, 1) & HA_READ_PREV))
+    reverse= 0;                               // Index can't be used
+  
+  if (have_pk_suffix && reverse == -1)
   {
-    key_parts= (uint) (key_part - table->key_info[idx].key_part);
-    if (reverse == -1 && 
-        !(table->file->index_flags(idx, key_parts-1, 1) & HA_READ_PREV))
+    uint pk_parts= table->key_info[pk].user_defined_key_parts;
+    if (!table->file->index_flags(pk, pk_parts, 1) & HA_READ_PREV)
       reverse= 0;                               // Index can't be used
   }
+
 ok:
   if (used_key_parts != NULL)
     *used_key_parts= key_parts;