Fixed bug in extended key handling when there is no primary key

Extended keys works by first checking if the engine supports extended
keys.
If yes, it extends secondary key with primary key components and mark the
secondary keys as HA_EXT_NOSAME (unique).
If we later notice that there where no primary key, the extended key
information for secondary keys in share->key_info is reset. However the
key_info->flag HA_EXT_NOSAME was not reset!

This causes some strange things to happen:
- Tables that have no primary key or secondary index that contained the
  primary key would be wrongly optimized as the secondary key could be
  thought to be unique when it was not and not unique when it was.
- The problem was not shown in EXPLAIN because of a bug in
  create_ref_for_key() that caused EQ_REF to be displayed by EXPLAIN as REF
  when extended keys where used and the secondary key contained the primary
  key.

This is fixed with:
- Removed wrong test in make_join_select() which did not detect that key
  where unique when a secondary key contains the primary.
- Moved initialization of extended keys from create_key_infos() to
  init_from_binary_frm_image() after we know if there is a usable primary
  key or not. One disadvantage with this approach is that
  key_info->key_parts may have not used slots (for keys we thought could
  be extended but could not). Fixed by adding a check for unused key_parts
  to copy_keys_from_share().

Other things:
- Simplified copying of first key part in create_key_infos().
- Added a lot of code comments in code that I had to check as part of
  finding the issue.
- Fixed some indentation.
- Replaced a couple of looks using references to pointers in C
  context where the reference does not give any benefit.
- Updated Aria and Maria to not assume the all key_info->rec_per_key
  are in one memory block (this could happen when using dervived
  tables with many keys).
- Fixed a bug where key_info->rec_per_key where not allocated
- Optimized TABLE::add_tmp_key() to only call alloc() once.
  (No logic changes)

Test case changes:
- innodb_mysql.test changed index as an index the optimizer thought
  was unique, was not. (Table had no primary key)

TODO:
- Move code that checks for partial or too long keys to the primary loop
  earlier that initally decides if we should add extended key fields.
  This is needed to ensure that HA_EXT_NOSAME is not set for partial or
  too long keys. It will also shorten the current code notable.

mysql-test/suite/innodb/r/innodb_mysql.result

@@ -2193,7 +2193,7 @@ SELECT 1 FROM (SELECT COUNT(DISTINCT c1)
 FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	PRIMARY	<derived2>	ALL	NULL	NULL	NULL	NULL	2	
-2	DERIVED	t1	ref	c3,c2	c2	10	const,const	1	
+2	DERIVED	t1	ref	c3,c2	c3	5	const	2	Using where
 DROP TABLE t1;
 CREATE TABLE t1 (c1 REAL, c2 REAL, c3 REAL, KEY (c3), KEY (c2, c3))
 ENGINE=InnoDB;
@@ -2207,7 +2207,7 @@ SELECT 1 FROM (SELECT COUNT(DISTINCT c1)
 FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	PRIMARY	<derived2>	ALL	NULL	NULL	NULL	NULL	2	
-2	DERIVED	t1	ref	c3,c2	c2	18	const,const	1	
+2	DERIVED	t1	ref	c3,c2	c3	9	const	2	Using where
 DROP TABLE t1;
 CREATE TABLE t1 (c1 DECIMAL(12,2), c2 DECIMAL(12,2), c3 DECIMAL(12,2), 
 KEY (c3), KEY (c2, c3))
@@ -2222,7 +2222,7 @@ SELECT 1 FROM (SELECT COUNT(DISTINCT c1)
 FROM t1 WHERE c2 IN (1, 1) AND c3 = 2 GROUP BY c2) x;
 id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
 1	PRIMARY	<derived2>	ALL	NULL	NULL	NULL	NULL	2	
-2	DERIVED	t1	ref	c3,c2	c2	14	const,const	1	
+2	DERIVED	t1	ref	c3,c2	c3	7	const	2	Using where
 DROP TABLE t1;
 End of 5.1 tests
 #

sql/sql_class.h

@@ -6248,6 +6248,7 @@ class TMP_TABLE_PARAM :public Sql_alloc
   Item	    **items_to_copy;			/* Fields in tmp table */
   TMP_ENGINE_COLUMNDEF *recinfo, *start_recinfo;
   KEY *keyinfo;
+  ulong *rec_per_key;
   ha_rows end_write_records;
   /**
     Number of normal fields in the query, including those referred to

sql/sql_select.cc

@@ -12533,10 +12533,8 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j,
                ((key_flags & HA_NOSAME) && keyparts == not_null_keyparts)
                )) ||
              /* true only for extended keys */
-              (keyparts > keyinfo->user_defined_key_parts &&
-               MY_TEST(key_flags & HA_EXT_NOSAME) &&
-               keyparts == keyinfo->ext_key_parts)
-            ) ||
+             (MY_TEST(key_flags & HA_EXT_NOSAME) &&
+              keyparts == keyinfo->ext_key_parts) ) ||
            null_ref_key)
   {
     /* Must read with repeat */
@@ -20435,6 +20433,7 @@ TABLE *Create_tmp_table::start(THD *thd,
                         sizeof(*m_key_part_info)*(param->group_parts+1),
                         &param->start_recinfo,
                         sizeof(*param->recinfo)*(field_count*2+4),
+                        &param->rec_per_key, sizeof(ulong)*param->group_parts,
                         &tmpname, (uint) strlen(path)+1,
                         &m_group_buff, (m_group && ! m_using_unique_constraint ?
                                       param->group_length : 0),
@@ -20967,13 +20966,15 @@ bool Create_tmp_table::finalize(THD *thd,
     keyinfo->usable_key_parts=keyinfo->user_defined_key_parts=
       param->group_parts;
     keyinfo->ext_key_parts= keyinfo->user_defined_key_parts;
+    share->ext_key_parts= share->key_parts= keyinfo->ext_key_parts;
     keyinfo->key_length=0;
-    keyinfo->rec_per_key=NULL;
+    keyinfo->rec_per_key= param->rec_per_key;
     keyinfo->read_stats= NULL;
     keyinfo->collected_stats= NULL;
     keyinfo->algorithm= HA_KEY_ALG_UNDEF;
     keyinfo->is_statistics_from_stat_tables= FALSE;
     keyinfo->name= group_key;
+    keyinfo->comment.str= 0;
     ORDER *cur_group= m_group;
     for (; cur_group ; cur_group= cur_group->next, m_key_part_info++)
     {
@@ -21074,6 +21075,7 @@ bool Create_tmp_table::finalize(THD *thd,
     keyinfo->usable_key_parts= keyinfo->user_defined_key_parts;
     table->distinct= 1;
     share->keys= 1;
+    share->ext_key_parts= share->key_parts= keyinfo->ext_key_parts;
     if (!(m_key_part_info= (KEY_PART_INFO*)
           alloc_root(&table->mem_root,
                      keyinfo->user_defined_key_parts * sizeof(KEY_PART_INFO))))
@@ -21570,6 +21572,7 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo,
 
       /* Can't create a key; Make a unique constraint instead of a key */
       share->keys=    0;
+      share->key_parts= share->ext_key_parts= 0;
       share->uniques= 1;
       using_unique_constraint=1;
       bzero((char*) &uniquedef,sizeof(uniquedef));
@@ -21766,6 +21769,7 @@ bool create_internal_tmp_table(TABLE *table, KEY *keyinfo,
     {
       /* Can't create a key; Make a unique constraint instead of a key */
       share->keys=    0;
+      share->key_parts= share->ext_key_parts= 0;
       share->uniques= 1;
       using_unique_constraint=1;
       bzero((char*) &uniquedef,sizeof(uniquedef));

sql/sql_table.cc

@@ -2105,7 +2105,8 @@ static int sort_keys(KEY *a, KEY *b)
       return -1;
     /*
       Long Unique keys should always be last unique key.
-      Before this patch they used to change order wrt to partial keys (MDEV-19049)
+      Before this patch they used to change order wrt to partial keys
+      (MDEV-19049)
     */
     if (a->algorithm == HA_KEY_ALG_LONG_HASH)
       return 1;

storage/maria/ha_maria.cc

@@ -2727,12 +2727,18 @@ int ha_maria::info(uint flag)
     share->db_record_offset= maria_info.record_offset;
     if (share->key_parts)
     {
-      ulong *to= table->key_info[0].rec_per_key, *end;
       double *from= maria_info.rec_per_key;
-      for (end= to+ share->key_parts ; to < end ; to++, from++)
+      KEY *key, *key_end;
+      for (key= table->key_info, key_end= key + share->keys;
+           key < key_end ; key++)
+      {
+        ulong *to= key->rec_per_key;
+        for (ulong *end= to+ key->user_defined_key_parts ;
+             to < end ;
+             to++, from++)
           *to= (ulong) (*from + 0.5);
       }
-
+    }
     /*
       Set data_file_name and index_file_name to point at the symlink value
       if table is symlinked (Ie;  Real name is not same as generated name)

storage/myisam/ha_myisam.cc

@@ -2152,9 +2152,17 @@ int ha_myisam::info(uint flag)
     share->keys_for_keyread.intersect(share->keys_in_use);
     share->db_record_offset= misam_info.record_offset;
     if (share->key_parts)
-      memcpy((char*) table->key_info[0].rec_per_key,
-	     (char*) misam_info.rec_per_key,
-             sizeof(table->key_info[0].rec_per_key[0])*share->key_parts);
+    {
+      ulong *from= misam_info.rec_per_key;
+      KEY *key, *key_end;
+      for (key= table->key_info, key_end= key + share->keys;
+           key < key_end ; key++)
+      {
+        memcpy(key->rec_per_key, from,
+               key->user_defined_key_parts * sizeof(*from));
+        from+= key->user_defined_key_parts;
+      }
+    }
     if (table_share->tmp_table == NO_TMP_TABLE)
       mysql_mutex_unlock(&table_share->LOCK_share);
   }