[prev in list] [next in list] [prev in thread] [next in thread] 

List:       mysql-internals
Subject:    ha_table_exists_in_engine (5.1)
From:       Stewart Smith <stewart () mysql ! com>
Date:       2007-04-13 6:50:19
Message-ID: 1176447019.5634.269.camel () localhost ! localdomain
[Download RAW message or body]


(resending to internals for others to look at too... not just a few of
us devs)

So... the story so far....

We have a patch that changes ha_table_exists_in_engine to always return
a HA_ERR code so we can easily get back to user things like "Cluster
failure", "not connected" instead of "table exists... even if it doesn't
- you go figure out why".

In 5.0, we just have a hardcoded call to NDB. All is well in the world.

5.1 we have pluggable and recently Brian "fixed" (well.. fixed for
pluggable, not for sanity) the
ha_table_exists_in_engine... which works for the old way of doing things
- a "boolean" result.

Of course, the comment there was crap (like comments are).


I propose we change the interface to something like what I have below
(untested patch... no doubt i misspelt something).

Basically:
- the ha_table_exists_in_engine returns a HA_ERR code such as
HA_ERR_TABLE_EXISTS or HA_ERR_TABLE_NOT_EXISTS or HA_ERR_NO_CONNECTION
or HA_ERR_END_OF_WORLD (whatever is relevant).
- the handler.cc code iterates through all storage engine plugins
calling ha_table_exists_in_engine (if implemented) and grabs the first
error that isn't TABLE_NOT_EXISTS and returns that to the user.


A general note is that for anything engine related a boolean return code
is useless and doesn't let us convey anything sensible back to the
user.. HA_ERR codes much better.

Thoughts appreciated - but I think gni should continue to fix this bug
with this kind of architecture so we get consistent error messages in
5.0 and 5.1 (where both are correct.. not just 5.0)

--- ndb-work.orig/sql/handler.cc        2007-04-13 16:22:31.270979750 +1000
+++ ndb-work/sql/handler.cc     2007-04-13 16:28:54.806949250 +1000
@@ -2858,20 +2858,11 @@ ha_find_files(THD *thd,const char *db,co
   DBUG_RETURN(error);
 }
 
-
-/** @brief
-  Ask handler if the table exists in engine
-
-  RETURN
-    0                   Table does not exist
-    1                   Table exists
-    #                   Error code
-
-*/
 struct st_table_exists_in_engine_args
 {
   const char *db;
   const char *name;
+  int err;
 };
 
 static my_bool table_exists_in_engine_handlerton(THD *thd, st_plugin_int *plugin,
@@ -2880,23 +2871,26 @@ static my_bool table_exists_in_engine_ha
   st_table_exists_in_engine_args *vargs= (st_table_exists_in_engine_args *)arg;
   handlerton *hton= (handlerton *)plugin->data;
 
+  int err= HA_ERR_TABLE_NOT_EXISTS;
+
   if (hton->state == SHOW_OPTION_YES && hton->table_exists_in_engine)
-    if ((hton->table_exists_in_engine(hton, thd, vargs->db, vargs->name)) == 1)
-      return TRUE;
+    err= hton->table_exists_in_engine(hton, thd, vargs->db, vargs->name);
 
-  return FALSE;
+  if(vargs->err==HA_ERR_TABLE_NOT_EXISTS)
+    vargs->err= err;
+
+  return TRUE;
 }
 
 int ha_table_exists_in_engine(THD* thd, const char* db, const char* name)
 {
-  int error= 0;
   DBUG_ENTER("ha_table_exists_in_engine");
   DBUG_PRINT("enter", ("db: %s, name: %s", db, name));
-  st_table_exists_in_engine_args args= {db, name};
-  error= plugin_foreach(thd, table_exists_in_engine_handlerton,
+  st_table_exists_in_engine_args args= {db, name, HA_ERR_TABLE_NOT_EXIST};
+  plugin_foreach(thd, table_exists_in_engine_handlerton,
                  MYSQL_STORAGE_ENGINE_PLUGIN, &args);
   DBUG_PRINT("exit", ("error: %d", error));
-  DBUG_RETURN(error);
+  DBUG_RETURN(args.err);
 }
 
 #ifdef HAVE_NDB_BINLOG

-- 
Stewart Smith, Software Engineer
MySQL AB, www.mysql.com
Office: +14082136540 Ext: 6616
VoIP: 6616@sip.us.mysql.com
Mobile: +61 4 3 8844 332

Jumpstart your cluster:
http://www.mysql.com/consulting/packaged/cluster.html

["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic