-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stewart, all,

Here is some code from handler.cc which is responsible for deleting a table:

/**
  This should return ENOENT if the file doesn't exists.
  The .frm file will be deleted only if we return 0 or ENOENT
*/
int ha_delete_table(Session *session
                  , const char *path
                  , const char *db
                  , const char *alias
                  , bool generate_warning)
{
<snip>
  plugin_foreach(NULL, deletetable_handlerton,
DRIZZLE_STORAGE_ENGINE_PLUGIN, &dtargs);
<snip>
}

I've snipped all the non-relevant-to-this-discussion code from the
ha_delete_table() method of the handler.  What you will notice is that
the method for deleting a table in Drizzle simply has each storage
engine plugin call the deletetable_handlerton callback.

Here is the implementation of that callback in its entirety:

static bool deletetable_handlerton(Session *,
                                   plugin_ref plugin,
                                   void *args)
{
  struct handlerton_delete_table_args *dtargs=
   (struct handlerton_delete_table_args *) args;

  Session *session= dtargs->session;
  const char *path= dtargs->path;

  handler *file;
  char tmp_path[FN_REFLEN];

  if(dtargs->error!=ENOENT) /* already deleted table */
    return false;

  handlerton *table_type= plugin_data(plugin, handlerton *);

  if(!table_type)
    return false;

  if(!(table_type->state == SHOW_OPTION_YES && table_type->create))
    return false;

  if ((file= table_type->create(table_type, NULL, session->mem_root)))
    file->init();
  else
    return false;

  path= check_lowercase_names(file, path, tmp_path);
  int error= file->ha_delete_table(path);

  if(error!=ENOENT)
  {
    dtargs->error= error;
    if(dtargs->file)
      delete dtargs->file;
    dtargs->file= file;
    return true;
  }
  else
    delete file;

  return false;
}

For those unfamiliar with how our plugin interface works, this is what
is happening in plain English:

for every storage engine plugin:
  1) check to see if the storage engine is "active"
  2) ask the storage engine to create a "handler"
  3) initialize the handler
  4) lowercase the path to the table proto (used to be frm file)
  5) ask the handler to delete the table
  6) if the table exists in the storage engine, the handler
     will return ENOENT, and if that happens, we delete the handler
  7) if the table does exist in the storage engine, and the handler
     was able to delete the table, return 0
  8) if the table does exist, but the handler was not able to delete
     the table, return something other than ENOENT

All of the above is just to delete a single table.  The same kind of
looping process occurs for most other table operations.

I find the above to be incredibly inefficient.  Let's say we have 6
storage engines, and that the table we want to delete is managed by a
storage engine which is "last" in the list of storage engine plugins.
Every time we delete a table, we will need to allocate and initialize 6
different storage engine handlers, all which will be immediately deleted
after returning a ENOENT back to the calling loop.

I think a more efficient manner of dealing with these kind of operations
is to have each table managed by the server *know* what it's managing
storage engine is, and simply call the handler to delete the table
instead of looping over all storage engine plugins.

Now, I understand that we don't have a global map of tables to their
managing storage engines, but we *do* have the ability for each Session
to understand that a particular storage engine manages a particular
table "in use" by the Session, no?

Couldn't we technically change the ha_delete_table() code to something like:

int ha_delete_table(Session *session
                  , Table *table)
{
handler *myhandler= table->file;
int error= file->ha_delete_table(table);
// do error checking...
}

This requires us to stop passing around "file paths" for a table all
over the place (a holdover from the FRM stuff) and instead pass a
pointer to a Table object to the storage engine handler which can then
delete the table using its own implementation?

Thoughts on this most appreciated...

Cheers,

Jay
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAknLnUMACgkQ2upbWsB4UtGNvgCeJmelx65oIi9eTClrdhJxKBK0
v9QAn0+gSfalhF3xodzniVNKNLnbAX9F
=Or7R
-----END PGP SIGNATURE-----

_______________________________________________
Mailing list: https://launchpad.net/~drizzle-discuss
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~drizzle-discuss
More help   : https://help.launchpad.net/ListHelp

Reply via email to