handlerton::discover_table_names() can not discover only

"unknown" tables. Duplicates are possible - deal with them.

sql/discover.cc

@@ -232,11 +232,10 @@ int extension_based_table_discovery(MY_DIR *dirp, const char *ext_meta,
   will ignore them. Anyone still having these files, should disable
   discovering engines, and rename these invalid table files.
 */
-int ext_table_discovery_simple(MY_DIR *dirp, const char *ext_meta,
+int ext_table_discovery_simple(MY_DIR *dirp,
                                handlerton::discovered_list *result)
 {
   CHARSET_INFO *cs= character_set_filesystem;
-  size_t ext_meta_len= strlen(ext_meta);
   FILEINFO *cur, *end;
   
   cur= dirp->dir_entry;
@@ -248,7 +247,7 @@ int ext_table_discovery_simple(MY_DIR *dirp, const char *ext_meta,
     if (ext && !is_prefix(cur->name, tmp_file_prefix))
     {
       if (my_strnncoll(cs, (uchar*)ext, strlen(ext),
-                           (uchar*)ext_meta, ext_meta_len) == 0)
+                           (uchar*)reg_ext, reg_ext_length) == 0)
       {
         *ext = 0;
         if (result->add_file(cur->name))

sql/discover.h

@@ -25,7 +25,7 @@ int extension_based_table_discovery(MY_DIR *dirp, const char *ext,
                                     handlerton::discovered_list *tl);
 
 #ifdef MYSQL_SERVER
-int ext_table_discovery_simple(MY_DIR *dirp, const char *ext_meta,
+int ext_table_discovery_simple(MY_DIR *dirp,
                                handlerton::discovered_list *result);
 #endif
 

sql/handler.cc

@@ -383,12 +383,36 @@ static int ha_finish_errors(void)
   return 0;
 }
 
-volatile int32 need_full_discover_for_existence= 0;
+static volatile int32 need_full_discover_for_existence= 0;
+static volatile int32 engines_with_discover_table_names= 0;
+
+
 static int full_discover_for_existence(handlerton *, const char *, const char *)
 { return 1; }
+
 static int ext_based_existence(handlerton *, const char *, const char *)
 { return 1; }
 
+static int hton_ext_based_table_discovery(handlerton *hton, LEX_STRING *db,
+                             MY_DIR *dir, handlerton::discovered_list *result)
+{
+  /*
+    tablefile_extensions[0] is the metadata file, see
+    the comment above tablefile_extensions declaration
+  */
+  return extension_based_table_discovery(dir, hton->tablefile_extensions[0],
+                                         result);
+}
+
+static void update_discovery_counters(handlerton *hton, int val)
+{
+  if (hton->discover_table_existence == full_discover_for_existence)
+    my_atomic_add32(&need_full_discover_for_existence,  val);
+
+  if (hton->discover_table_names)
+    my_atomic_add32(&engines_with_discover_table_names, val);
+}
+
 int ha_finalize_handlerton(st_plugin_int *plugin)
 {
   handlerton *hton= (handlerton *)plugin->data;
@@ -438,11 +462,7 @@ int ha_finalize_handlerton(st_plugin_int *plugin)
     hton2plugin[hton->slot]= NULL;
   }
 
-  if (hton->discover_table_existence == full_discover_for_existence)
-    my_atomic_add32(&need_full_discover_for_existence, -1);
-
-  if (hton->discover_table_names)
-    my_atomic_add32(&engines_with_discover_table_names, -1);
+  update_discovery_counters(hton, -1);
 
   my_free(hton);
 
@@ -451,13 +471,6 @@ int ha_finalize_handlerton(st_plugin_int *plugin)
 }
 
 
-static int hton_ext_based_table_discovery(handlerton *hton, LEX_STRING *db,
-                             MY_DIR *dir, handlerton::discovered_list *result)
-{
-  return extension_based_table_discovery(dir, hton->tablefile_extensions[0],
-                                         result);
-}
-
 int ha_initialize_handlerton(st_plugin_int *plugin)
 {
   handlerton *hton;
@@ -474,6 +487,9 @@ int ha_initialize_handlerton(st_plugin_int *plugin)
     goto err_no_hton_memory;
   }
 
+  hton->tablefile_extensions= no_exts;
+  hton->discover_table_names= hton_ext_based_table_discovery;
+
   hton->slot= HA_SLOT_UNDEF;
   /* Historical Requirement */
   plugin->data= hton; // shortcut for the future
@@ -484,15 +500,11 @@ int ha_initialize_handlerton(st_plugin_int *plugin)
     goto err;
   }
 
-  // default list file extensions: empty
-  if (!hton->tablefile_extensions)
-    hton->tablefile_extensions= no_exts;
-
-  // if the enfine can discover a single table and it is file-based
-  // then it can use a default file-based table names discovery
-  if (!hton->discover_table_names &&
-      hton->discover_table && hton->tablefile_extensions[0])
-    hton->discover_table_names= hton_ext_based_table_discovery;
+  // hton_ext_based_table_discovery() works only when discovery
+  // is supported and the engine if file-based.
+  if (hton->discover_table_names == hton_ext_based_table_discovery &&
+      (!hton->discover_table || !hton->tablefile_extensions[0]))
+    hton->discover_table_names= NULL;
 
   // default discover_table_existence implementation
   if (!hton->discover_table_existence && hton->discover_table)
@@ -500,10 +512,7 @@ int ha_initialize_handlerton(st_plugin_int *plugin)
     if (hton->tablefile_extensions[0])
       hton->discover_table_existence= ext_based_existence;
     else
-    {
       hton->discover_table_existence= full_discover_for_existence;
-      my_atomic_add32(&need_full_discover_for_existence, 1);
-    }
   }
 
   /*
@@ -595,8 +604,7 @@ int ha_initialize_handlerton(st_plugin_int *plugin)
     break;
   };
 
-  if (hton->discover_table_names)
-    my_atomic_add32(&engines_with_discover_table_names, 1);
+  update_discovery_counters(hton, 1);
 
   DBUG_RETURN(0);
 
@@ -4474,13 +4482,88 @@ bool ha_table_exists(THD *thd, const char *db, const char *table_name,
 /**
   Discover all table names in a given database
 */
-volatile int32 engines_with_discover_table_names= 0;
+extern "C" {
+
+static int cmp_file_names(const void *a, const void *b)
+{
+  CHARSET_INFO *cs= character_set_filesystem;
+  char *aa= ((FILEINFO *)a)->name;
+  char *bb= ((FILEINFO *)b)->name;
+  return my_strnncoll(cs, (uchar*)aa, strlen(aa), (uchar*)bb, strlen(bb));
+}
+
+static int cmp_table_names(LEX_STRING * const *a, LEX_STRING * const *b)
+{
+  return my_strnncoll(&my_charset_bin, (uchar*)((*a)->str), (*a)->length,
+                                       (uchar*)((*b)->str), (*b)->length);
+}
+
+}
+
+Discovered_table_list::Discovered_table_list(THD *thd_arg,
+                 Dynamic_array<LEX_STRING*> *tables_arg,
+                 const LEX_STRING *wild_arg)
+{
+  thd= thd_arg;
+  tables= tables_arg;
+  if (wild_arg->str && wild_arg->str[0])
+  {
+    wild= wild_arg->str;
+    wend= wild + wild_arg->length;
+  }
+  else
+    wild= 0;
+}
+
+bool Discovered_table_list::add_table(const char *tname, size_t tlen)
+{
+  if (wild && my_wildcmp(files_charset_info, tname, tname + tlen, wild, wend,
+                         wild_prefix, wild_one, wild_many))
+      return 0;
+
+  LEX_STRING *name= thd->make_lex_string(tname, tlen);
+  if (!name || tables->append(name))
+    return 1;
+  return 0;
+}
+
+bool Discovered_table_list::add_file(const char *fname)
+{
+  char tname[SAFE_NAME_LEN + 1];
+  size_t tlen= filename_to_tablename(fname, tname, sizeof(tname));
+  return add_table(tname, tlen);
+}
+
+
+void Discovered_table_list::sort()
+{
+  tables->sort(cmp_table_names);
+}
+
+void Discovered_table_list::remove_duplicates()
+{
+  LEX_STRING **src= tables->front();
+  LEX_STRING **dst= src;
+  while (++dst < tables->back())
+  {
+    LEX_STRING *s= *src, *d= *dst;
+    DBUG_ASSERT(strncmp(s->str, d->str, min(s->length, d->length)) <= 0);
+    if ((s->length != d->length || strncmp(s->str, d->str, d->length)))
+    {
+      src++;
+      if (src != dst)
+        *src= *dst;
+    }
+  }
+  tables->set_elements(src - tables->front() + 1);
+}
 
 struct st_discover_names_args
 {
   LEX_STRING *db;
   MY_DIR *dirp;
-  handlerton::discovered_list *result;
+  Discovered_table_list *result;
+  uint possible_duplicates;
 };
 
 static my_bool discover_names(THD *thd, plugin_ref plugin,
@@ -4488,28 +4571,51 @@ static my_bool discover_names(THD *thd, plugin_ref plugin,
 {
   st_discover_names_args *args= (st_discover_names_args *)arg;
   handlerton *ht= plugin_data(plugin, handlerton *);
-  if (ht->state == SHOW_OPTION_YES && ht->discover_table_names &&
-      ht->discover_table_names(ht, args->db, args->dirp, args->result))
-    return 1;
+
+  if (ht->state == SHOW_OPTION_YES && ht->discover_table_names)
+  {
+    uint old_elements= args->result->tables->elements();
+    if (ht->discover_table_names(ht, args->db, args->dirp, args->result))
+      return 1;
+
+    /*
+      hton_ext_based_table_discovery never discovers a table that has
+      a corresponding .frm file; but custom engine discover methods might
+    */
+    if (ht->discover_table_names != hton_ext_based_table_discovery)
+      args->possible_duplicates+= args->result->tables->elements() - old_elements;
+  }
 
   return 0;
 }
 
 int ha_discover_table_names(THD *thd, LEX_STRING *db, MY_DIR *dirp,
-                            handlerton::discovered_list *result)
+                            Discovered_table_list *result)
 {
   int error;
   DBUG_ENTER("ha_discover_table_names");
-  st_discover_names_args args= {db, dirp, result};
 
   if (engines_with_discover_table_names == 0)
-    DBUG_RETURN(ext_table_discovery_simple(dirp, reg_ext, result));
+  {
+    error= ext_table_discovery_simple(dirp, result);
+    result->sort();
+  }
+  else
+  {
+    st_discover_names_args args= {db, dirp, result, 0};
 
-  error= extension_based_table_discovery(dirp, reg_ext, result);
+    /* extension_based_table_discovery relies on dirp being sorted */
+    my_qsort(dirp->dir_entry, dirp->number_of_files,
+             sizeof(FILEINFO), cmp_file_names);
 
-  if (!error)
-    error= plugin_foreach(thd, discover_names, MYSQL_STORAGE_ENGINE_PLUGIN,
-                          &args);
+    error= extension_based_table_discovery(dirp, reg_ext, result) ||
+           plugin_foreach(thd, discover_names,
+                            MYSQL_STORAGE_ENGINE_PLUGIN, &args);
+    result->sort();
+
+    if (args.possible_duplicates > 0)
+      result->remove_duplicates();
+  }
 
   DBUG_RETURN(error);
 }

sql/handler.h

@@ -31,6 +31,7 @@
 #include "thr_lock.h"          /* thr_lock_type, THR_LOCK_DATA */
 #include "sql_cache.h"
 #include "structs.h"                            /* SHOW_COMP_OPTION */
+#include "sql_array.h"          /* Dynamic_array<> */
 
 #include <my_compare.h>
 #include <ft_global.h>
@@ -3101,14 +3102,30 @@ int ha_delete_table(THD *thd, handlerton *db_type, const char *path,
 bool ha_show_status(THD *thd, handlerton *db_type, enum ha_stat_type stat);
 
 /* discovery */
+#ifdef MYSQL_SERVER
+class Discovered_table_list: public handlerton::discovered_list
+{
+  THD *thd;
+  const char *wild, *wend;
+public:
+  Dynamic_array<LEX_STRING*> *tables;
+
+  Discovered_table_list(THD *thd_arg, Dynamic_array<LEX_STRING*> *tables_arg,
+                        const LEX_STRING *wild_arg);
+  ~Discovered_table_list() {}
+
+  bool add_table(const char *tname, size_t tlen);
+  bool add_file(const char *fname);
+
+  void sort();
+  void remove_duplicates(); // assumes that the list is sorted
+};
+
 int ha_discover_table(THD *thd, TABLE_SHARE *share);
 int ha_discover_table_names(THD *thd, LEX_STRING *db, MY_DIR *dirp,
-                            handlerton::discovered_list *result);
+                            Discovered_table_list *result);
 bool ha_table_exists(THD *thd, const char *db, const char *table_name,
                      handlerton **hton= 0);
-
-#ifdef MYSQL_SERVER
-extern volatile int32 engines_with_discover_table_names;
 #endif
 
 /* key cache */

sql/item_subselect.cc

@@ -2935,7 +2935,7 @@ bool Item_exists_subselect::exists2in_processor(uchar *opt_arg)
         res= TRUE;
         goto out;
       }
-      for (int i= 0; i < eqs.elements(); i++)
+      for (size_t i= 0; i < eqs.elements(); i++)
       {
         if (optimizer->arguments()[0]->maybe_null)
         {

sql/sql_array.h

@@ -104,7 +104,7 @@ public:
                           MYF(MY_THREAD_SPECIFIC));
   }
 
-  Elem& at(int idx)
+  Elem& at(size_t idx)
   {
     return *(((Elem*)array.buffer) + idx);
   }
@@ -129,11 +129,16 @@ public:
     return (insert_dynamic(&array, (uchar*)&el));
   }
 
-  int elements()
+  size_t elements()
   {
     return array.elements;
   }
 
+  void set_elements(size_t n)
+  {
+    array.elements= n;
+  }
+
   ~Dynamic_array()
   {
     delete_dynamic(&array);

sql/sql_show.cc

@@ -687,59 +687,6 @@ db_name_is_in_ignore_db_dirs_list(const char *directory)
   return my_hash_search(&ignore_db_dirs_hash, (uchar *) buff, buff_len)!=NULL;
 }
 
-class Discovered_table_list: public handlerton::discovered_list
-{
-  THD *thd;
-  const char *wild;
-  size_t wild_length;
-  Dynamic_array<LEX_STRING*> *tables;
-
-public:
-  Discovered_table_list(THD *thd_arg, Dynamic_array<LEX_STRING*> *tables_arg,
-                        const char *wild_arg)
-  {
-    thd= thd_arg;
-    tables= tables_arg;
-    if (wild_arg && wild_arg[0])
-    {
-      wild= wild_arg;
-      wild_length= strlen(wild_arg);
-    }
-    else
-      wild= 0;
-  }
-  ~Discovered_table_list() {}
-
-  bool add_table(const char *tname, size_t tlen)
-  {
-    if (wild && my_wildcmp(files_charset_info, tname, tname + tlen,
-                           wild, wild + wild_length,
-                           wild_prefix, wild_one, wild_many))
-        return 0;
-
-    LEX_STRING *name= thd->make_lex_string(tname, tlen);
-    if (!name || tables->append(name))
-      return 1;
-    return 0;
-  }
-
-  bool add_file(const char *fname)
-  {
-    char tname[SAFE_NAME_LEN + 1];
-    size_t tlen= filename_to_tablename(fname, tname, sizeof(tname));
-    return add_table(tname, tlen);
-  }
-};
-
-extern "C" {
-static int cmp_table_names(LEX_STRING * const *a, LEX_STRING * const *b)
-{
-  return my_strnncoll(&my_charset_bin,
-                      (const uchar*)((*a)->str), (*a)->length,
-                      (const uchar*)((*b)->str), (*b)->length);
-}
-}
-
 enum find_files_result {
   FIND_FILES_OK,
   FIND_FILES_OOM,
@@ -767,20 +714,13 @@ enum find_files_result {
 
 static find_files_result
 find_files(THD *thd, Dynamic_array<LEX_STRING*> *files, LEX_STRING *db,
-           const char *path, const char *wild)
+           const char *path, const LEX_STRING *wild)
 {
   MY_DIR *dirp;
-  myf my_dir_flags= MY_THREAD_SPECIFIC;
   Discovered_table_list tl(thd, files, wild);
   DBUG_ENTER("find_files");
 
-  if (!db)
-    my_dir_flags|= MY_WANT_STAT;
-
-  if (engines_with_discover_table_names)
-    my_dir_flags|= MY_WANT_SORT;
-
-  if (!(dirp = my_dir(path, my_dir_flags)))
+  if (!(dirp = my_dir(path, MY_THREAD_SPECIFIC | (db ? 0 : MY_WANT_STAT))))
   {
     if (my_errno == ENOENT)
       my_error(ER_BAD_DB_ERROR, MYF(ME_BELL | ME_WAITTANG), db->str);
@@ -819,6 +759,7 @@ find_files(THD *thd, Dynamic_array<LEX_STRING*> *files, LEX_STRING *db,
       if (tl.add_file(file->name))
         goto err;
     }
+    tl.sort();
   }
   else
   {
@@ -826,11 +767,9 @@ find_files(THD *thd, Dynamic_array<LEX_STRING*> *files, LEX_STRING *db,
       goto err;
   }
 
-  DBUG_PRINT("info",("found: %d files", files->elements()));
+  DBUG_PRINT("info",("found: %zu files", files->elements()));
   my_dirend(dirp);
 
-  files->sort(cmp_table_names);
-
   DBUG_RETURN(FIND_FILES_OK);
 
 err:
@@ -3716,7 +3655,7 @@ int make_db_list(THD *thd, Dynamic_array<LEX_STRING*> *files,
         return 1;
     }
     return find_files(thd, files, 0, mysql_data_home,
-                      lookup_field_vals->db_value.str);
+                      &lookup_field_vals->db_value);
   }
 
 
@@ -3747,7 +3686,7 @@ int make_db_list(THD *thd, Dynamic_array<LEX_STRING*> *files,
   */
   if (files->append_val(&INFORMATION_SCHEMA_NAME))
     return 1;
-  return find_files(thd, files, 0, mysql_data_home, NullS);
+  return find_files(thd, files, 0, mysql_data_home, &null_lex_str);
 }
 
 
@@ -3891,7 +3830,7 @@ make_table_name_list(THD *thd, Dynamic_array<LEX_STRING*> *table_names,
                               lookup_field_vals->table_value.str));
 
   find_files_result res= find_files(thd, table_names, db_name, path,
-                                    lookup_field_vals->table_value.str);
+                                    &lookup_field_vals->table_value);
   if (res != FIND_FILES_OK)
   {
     /*
@@ -4633,7 +4572,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond)
 
   if (make_db_list(thd, &db_names, &lookup_field_vals))
     goto err;
-  for (int i=0; i < db_names.elements(); i++)
+  for (size_t i=0; i < db_names.elements(); i++)
   {
     LEX_STRING *db_name= db_names.at(i);
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
@@ -4652,7 +4591,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond)
       if (res)
         goto err;
 
-      for (int i=0; i < table_names.elements(); i++)
+      for (size_t i=0; i < table_names.elements(); i++)
       {
         LEX_STRING *table_name= table_names.at(i);
 
@@ -4800,7 +4739,7 @@ int fill_schema_schemata(THD *thd, TABLE_LIST *tables, COND *cond)
       DBUG_RETURN(0);
   }
 
-  for (int i=0; i < db_names.elements(); i++)
+  for (size_t i=0; i < db_names.elements(); i++)
   {
     LEX_STRING *db_name= db_names.at(i);
     if (db_name == &INFORMATION_SCHEMA_NAME)

storage/sphinx/ha_sphinx.cc

@@ -1285,7 +1285,7 @@ CSphSEQuery::~CSphSEQuery ()
 	SafeDeleteArray ( m_sQueryBuffer );
 	SafeDeleteArray ( m_pWeights );
 	SafeDeleteArray ( m_pBuf );
-	for ( int i=0; i<m_dOverrides.elements(); i++ )
+	for ( size_t i=0; i<m_dOverrides.elements(); i++ )
 		SafeDelete ( m_dOverrides.at(i) );
 	SPH_VOID_RET();
 }
@@ -1865,7 +1865,7 @@ int CSphSEQuery::BuildRequest ( char ** ppBuffer )
 		iReqSize += 8 + strlen(m_sFieldWeight[i] );
 	// overrides
 	iReqSize += 4;
-	for ( int i=0; i<m_dOverrides.elements(); i++ )
+	for ( size_t i=0; i<m_dOverrides.elements(); i++ )
 	{
 		CSphSEQuery::Override_t * pOverride = m_dOverrides.at(i);
 		const uint32 uSize = pOverride->m_iType==SPH_ATTR_BIGINT ? 16 : 12; // id64 + value
@@ -1972,13 +1972,13 @@ int CSphSEQuery::BuildRequest ( char ** ppBuffer )
 
 	// overrides
 	SendInt ( m_dOverrides.elements() );
-	for ( int i=0; i<m_dOverrides.elements(); i++ )
+	for ( size_t i=0; i<m_dOverrides.elements(); i++ )
 	{
 		CSphSEQuery::Override_t * pOverride = m_dOverrides.at(i);
 		SendString ( pOverride->m_sName );
 		SendDword ( pOverride->m_iType );
 		SendInt ( pOverride->m_dIds.elements() );
-		for ( int j=0; j<pOverride->m_dIds.elements(); j++ )
+		for ( size_t j=0; j<pOverride->m_dIds.elements(); j++ )
 		{
 			SendUint64 ( pOverride->m_dIds.at(j) );
 			if ( pOverride->m_iType==SPH_ATTR_FLOAT )