MDEV-5067: Valgrind warnings (Invalid read) in QPF_table_access::print_explain

- Query plan footprint (in new terms, "EXPLAIN structure") should always keep 
  a copy of key_name.  This is because the table might be a temporary table which
  may be already freed by the time we use query plan footprint.

mysql-test/t/explain_non_select.test

@@ -106,4 +106,12 @@ explain replace into t1 select * from t0;
 
 drop table t0, t1;
 
+--echo #
+--echo # MDEV-5067: Valgrind warnings (Invalid read) in QPF_table_access::print_explain
+--echo #
+CREATE TABLE t1 (i INT) ENGINE=MyISAM;
+INSERT INTO t1 VALUES (7),(0),(9);
+
+SELECT * FROM t1 INNER JOIN ( SELECT DISTINCT * FROM t1 ) AS sq ON (sq.i = t1.i);
 
+DROP TABLE t1;

sql/opt_qpf.cc

@@ -385,12 +385,12 @@ int QPF_table_access::print_explain(select_result_sink *output, uint8 explain_fl
 
   /* `key` */
   StringBuffer<64> key_str;
-  if (key.key_name)
+  if (key.get_key_name())
   {
     if (is_hj)
       key_str.append(hash_key_prefix, strlen(hash_key_prefix), cs);
 
-    key_str.append(key.key_name);
+    key_str.append(key.get_key_name());
 
     if (is_hj && type != JT_HASH)
       key_str.append(':');
@@ -403,7 +403,7 @@ int QPF_table_access::print_explain(select_result_sink *output, uint8 explain_fl
     key_str.append(buf2);
   }
   if (type == JT_HASH_NEXT)
-    key_str.append(hash_next_key.key_name);
+    key_str.append(hash_next_key.get_key_name());
   
   if (key_str.length() > 0)
     push_string(&item_list, &key_str);
@@ -413,11 +413,11 @@ int QPF_table_access::print_explain(select_result_sink *output, uint8 explain_fl
   /* `key_len` */
   StringBuffer<64> key_len_str;
 
-  if (key.key_len != (uint)-1)
+  if (key.get_key_len() != (uint)-1)
   {
     char buf[64];
     size_t length;
-    length= longlong10_to_str(key.key_len, buf, 10) - buf;
+    length= longlong10_to_str(key.get_key_len(), buf, 10) - buf;
     key_len_str.append(buf, length);
     if (is_hj && type != JT_HASH)
       key_len_str.append(':');
@@ -434,7 +434,7 @@ int QPF_table_access::print_explain(select_result_sink *output, uint8 explain_fl
   {
     char buf[64];
     size_t length;
-    length= longlong10_to_str(hash_next_key.key_len, buf, 10) - buf;
+    length= longlong10_to_str(hash_next_key.get_key_len(), buf, 10) - buf;
     key_len_str.append(buf, length);
   }
 
@@ -642,7 +642,7 @@ void QPF_quick_select::print_extra_recursive(String *str)
   if (quick_type == QUICK_SELECT_I::QS_TYPE_RANGE || 
       quick_type == QUICK_SELECT_I::QS_TYPE_RANGE_DESC)
   {
-    str->append(range.key_name);
+    str->append(range.get_key_name());
   }
   else
   {
@@ -695,7 +695,7 @@ void QPF_quick_select::print_key(String *str)
   {
     if (str->length() > 0)
       str->append(',');
-    str->append(range.key_name);
+    str->append(range.get_key_name());
   }
   else
   {
@@ -721,7 +721,7 @@ void QPF_quick_select::print_key_len(String *str)
   {
     char buf[64];
     size_t length;
-    length= longlong10_to_str(range.key_len, buf, 10) - buf;
+    length= longlong10_to_str(range.get_key_len(), buf, 10) - buf;
     if (str->length() > 0)
       str->append(',');
     str->append(buf, length);

sql/opt_qpf.h

@@ -310,10 +310,26 @@ typedef struct st_qpf_bka_type
 */
 class QPF_index_use : public Sql_alloc
 {
-public:
-  const char *key_name;
+  char *key_name;
   uint key_len;
   /* will add #keyparts here if we implement EXPLAIN FORMAT=JSON */
+public:
+
+  void set(MEM_ROOT *root, const char *key_name_arg, uint key_len_arg)
+  {
+    if (key_name_arg)
+    {
+      size_t name_len= strlen(key_name_arg);
+      if ((key_name= (char*)alloc_root(root, name_len+1)))
+        memcpy(key_name, key_name_arg, name_len+1);
+    }
+    else
+      key_name= NULL;
+    key_len= key_len_arg;
+  }
+
+  inline const char *get_key_name() { return key_name; }
+  inline uint get_key_len() { return key_len; }
 };
 
 

sql/opt_range.cc

@@ -11940,23 +11940,21 @@ void QUICK_SELECT_I::add_key_name(String *str, bool *first)
 }
  
 
-void QUICK_RANGE_SELECT::save_info(QPF_quick_select *qpf)
+void QUICK_RANGE_SELECT::save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)
 {
   qpf->quick_type= QS_TYPE_RANGE;
-  qpf->range.key_name= head->key_info[index].name;
-  qpf->range.key_len= max_used_key_length;
+  qpf->range.set(alloc, head->key_info[index].name, max_used_key_length);
 }
 
 
-void QUICK_GROUP_MIN_MAX_SELECT::save_info(QPF_quick_select *qpf)
+void QUICK_GROUP_MIN_MAX_SELECT::save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)
 {
   qpf->quick_type= QS_TYPE_GROUP_MIN_MAX;
-  qpf->range.key_name= head->key_info[index].name;
-  qpf->range.key_len= max_used_key_length;
+  qpf->range.set(alloc, head->key_info[index].name, max_used_key_length);
 }
 
 
-void QUICK_INDEX_SORT_SELECT::save_info(QPF_quick_select *qpf)
+void QUICK_INDEX_SORT_SELECT::save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)
 {
   qpf->quick_type= get_type();
 
@@ -11967,14 +11965,14 @@ void QUICK_INDEX_SORT_SELECT::save_info(QPF_quick_select *qpf)
   {
     child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    quick->save_info(child_qpf);
+    quick->save_info(alloc, child_qpf);
   }
 
   if (pk_quick_select)
   {
     child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    pk_quick_select->save_info(child_qpf);
+    pk_quick_select->save_info(alloc, child_qpf);
   }
 }
 
@@ -11982,7 +11980,7 @@ void QUICK_INDEX_SORT_SELECT::save_info(QPF_quick_select *qpf)
   Same as QUICK_INDEX_SORT_SELECT::save_info(), but primary key is printed
   first
 */
-void QUICK_INDEX_INTERSECT_SELECT::save_info(QPF_quick_select *qpf)
+void QUICK_INDEX_INTERSECT_SELECT::save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)
 {
   qpf->quick_type= get_type();
   QPF_quick_select *child_qpf;
@@ -11991,7 +11989,7 @@ void QUICK_INDEX_INTERSECT_SELECT::save_info(QPF_quick_select *qpf)
   {
     child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    pk_quick_select->save_info(child_qpf);
+    pk_quick_select->save_info(alloc, child_qpf);
   }
 
   QUICK_RANGE_SELECT *quick;
@@ -12000,13 +11998,13 @@ void QUICK_INDEX_INTERSECT_SELECT::save_info(QPF_quick_select *qpf)
   {
     child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    quick->save_info(child_qpf);
+    quick->save_info(alloc, child_qpf);
   }
 
 }
 
 
-void QUICK_ROR_INTERSECT_SELECT::save_info(QPF_quick_select *qpf)
+void QUICK_ROR_INTERSECT_SELECT::save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)
 {
   qpf->quick_type= get_type();
 
@@ -12016,19 +12014,19 @@ void QUICK_ROR_INTERSECT_SELECT::save_info(QPF_quick_select *qpf)
   {
     QPF_quick_select *child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    qr->quick->save_info(child_qpf);
+    qr->quick->save_info(alloc, child_qpf);
   }
 
   if (cpk_quick)
   {
     QPF_quick_select *child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    cpk_quick->save_info(child_qpf);
+    cpk_quick->save_info(alloc, child_qpf);
   }
 }
 
 
-void QUICK_ROR_UNION_SELECT::save_info(QPF_quick_select *qpf)
+void QUICK_ROR_UNION_SELECT::save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)
 {
   qpf->quick_type= get_type();
 
@@ -12038,7 +12036,7 @@ void QUICK_ROR_UNION_SELECT::save_info(QPF_quick_select *qpf)
   {
     QPF_quick_select *child_qpf= new QPF_quick_select;
     qpf->children.push_back(child_qpf);
-    quick->save_info(child_qpf);
+    quick->save_info(alloc, child_qpf);
   }
 }
 

sql/opt_range.h

@@ -346,7 +346,7 @@ public:
   void add_key_name(String *str, bool *first);
 
   /* Save information about quick select's query plan */
-  virtual void save_info(QPF_quick_select *qpf)= 0;
+  virtual void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf)= 0;
 
   /*
     Return 1 if any index used by this quick select
@@ -473,7 +473,7 @@ public:
   { file->position(record); }
   int get_type() { return QS_TYPE_RANGE; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
-  void save_info(QPF_quick_select *qpf);
+  void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif
@@ -610,7 +610,7 @@ public:
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
 #endif
-  void save_info(QPF_quick_select *qpf);
+  void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf);
 
   bool push_quick_back(QUICK_RANGE_SELECT *quick_sel_range);
 
@@ -674,7 +674,7 @@ public:
   int get_next();
   int get_type() { return QS_TYPE_INDEX_INTERSECT; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
-  void save_info(QPF_quick_select *qpf);
+  void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf);
 };
 
 
@@ -712,7 +712,7 @@ public:
   bool unique_key_range() { return false; }
   int get_type() { return QS_TYPE_ROR_INTERSECT; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
-  void save_info(QPF_quick_select *qpf);
+  void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf);
   bool is_keys_used(const MY_BITMAP *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
@@ -791,7 +791,7 @@ public:
   bool unique_key_range() { return false; }
   int get_type() { return QS_TYPE_ROR_UNION; }
   void add_keys_and_lengths(String *key_names, String *used_lengths);
-  void save_info(QPF_quick_select *qpf);
+  void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf);
   bool is_keys_used(const MY_BITMAP *fields);
 #ifndef DBUG_OFF
   void dbug_dump(int indent, bool verbose);
@@ -940,7 +940,7 @@ public:
 #endif
   bool is_agg_distinct() { return have_agg_distinct; }
   bool loose_scan_is_scanning() { return is_index_scan; }
-  void save_info(QPF_quick_select *qpf);
+  void save_info(MEM_ROOT *alloc, QPF_quick_select *qpf);
 };
 
 

sql/sql_select.cc

@@ -22599,8 +22599,7 @@ int JOIN::save_qpf(QPF_query *output, bool need_tmp_table, bool need_order,
 
       QPF_table_access *qpt= new (output->mem_root) QPF_table_access;
       qp_sel->add_table(qpt);
-      qpt->key.key_name= NULL;
-      qpt->key.key_len= (uint)-1;
+      qpt->key.set(thd->mem_root, NULL, (uint)-1);
       qpt->quick_info= NULL;
       
       /* id */
@@ -22694,13 +22693,12 @@ int JOIN::save_qpf(QPF_query *output, bool need_tmp_table, bool need_order,
       if (tab->select && tab->select->quick && tab_type != JT_CONST)
       {
         qpt->quick_info= new QPF_quick_select;
-        tab->select->quick->save_info(qpt->quick_info);
+        tab->select->quick->save_info(thd->mem_root, qpt->quick_info);
       }
 
       if (key_info) /* 'index' or 'ref' access */
       {
-        qpt->key.key_name= key_info->name;
-        qpt->key.key_len=  key_len;
+        qpt->key.set(thd->mem_root, key_info->name, key_len);
 
         if (tab->ref.key_parts && tab_type != JT_FT)
 	{
@@ -22723,8 +22721,9 @@ int JOIN::save_qpf(QPF_query *output, bool need_tmp_table, bool need_order,
   
       if (tab_type == JT_HASH_NEXT) /* full index scan + hash join */
       {
-        qpt->hash_next_key.key_name= table->key_info[tab->index].name;
-        qpt->hash_next_key.key_len=  table->key_info[tab->index].key_length;
+        qpt->hash_next_key.set(thd->mem_root, 
+                               table->key_info[tab->index].name, 
+                               table->key_info[tab->index].key_length);
       }
 
       if (key_info)
@@ -22763,14 +22762,8 @@ int JOIN::save_qpf(QPF_query *output, bool need_tmp_table, bool need_order,
             key_name_buf.append(tmp_buff, strlen(tmp_buff), cs);
           }
 
-          size_t len;
-          if ((len= key_name_buf.length()))
-          {
-            char *ptr= (char*)thd->alloc(len+1);
-            memcpy(ptr, key_name_buf.c_ptr_safe(), len+1);
-            qpt->key.key_name= ptr;
-            qpt->key.key_len= -1;
-          }
+          if (key_name_buf.length())
+            qpt->key.set(thd->mem_root, key_name_buf.c_ptr_safe(), -1);
         }
 	qpt->ref_set= false;
       }