Merge lp:~stewart/drizzle/table-proto-message-error-checking into lp:~drizzle-trunk/drizzle/development

[Stewart Smith]

[Stewart Smith (stewart)]

Fix error reporting on bad table protobuf messages.

Also introduce testing of this.

We use a storage engine that presents tables and definitions to test server
functions such as parse_table_proto() instead of writing the proto out to a
file as some versions of the protobuf lib won't let you write out a proto
message that's missing required fields but within the server, it's possible
to get a message that is missing these fields.

We load this plugin ONLY for the test suite. It is NEVER designed to be used
by end users *at all*.

[lp:~stewart/drizzle/table-proto-message-error-checking]

1315. By Stewart Smith on 2010-02-08

Merged enum_field_type_to_table_message_type into table-proto-message-error-checking.

=== modified file 'drizzled/error.cc'
--- drizzled/error.cc	2010-02-04 08:14:46 +0000
+++ drizzled/error.cc	2010-02-08 01:35:22 +0000
@@ -1418,7 +1418,9 @@
 /* ER_INVALID_ENUM_VALUE */
 N_("Received an invalid enum value '%s'."),
 /* ER_NO_PRIMARY_KEY_ON_REPLICATED_TABLE */
-N_("Tables which are replicated require a primary key.")
+N_("Tables which are replicated require a primary key."),
+/* ER_CORRUPT_TABLE_DEFINITION */
+N_("Corrupt or invalid table definition: %s")
 };
 
 const char * error_message(unsigned int code)

=== modified file 'drizzled/error.h'
--- drizzled/error.h	2010-02-04 08:14:46 +0000
+++ drizzled/error.h	2010-02-08 01:35:22 +0000
@@ -723,7 +723,8 @@
   ER_INVALID_TIME_VALUE,
   ER_INVALID_ENUM_VALUE,
   ER_NO_PRIMARY_KEY_ON_REPLICATED_TABLE,
-  ER_ERROR_LAST= ER_NO_PRIMARY_KEY_ON_REPLICATED_TABLE
+  ER_CORRUPT_TABLE_DEFINITION,
+  ER_ERROR_LAST= ER_CORRUPT_TABLE_DEFINITION
 };
 
 } /* namespace drizzled */

=== modified file 'drizzled/message/table_writer.cc'
--- drizzled/message/table_writer.cc	2009-12-11 21:54:18 +0000
+++ drizzled/message/table_writer.cc	2010-02-08 01:35:22 +0000
@@ -22,6 +22,7 @@
 #include <iostream>
 #include <fstream>
 #include <string>
+#include <getopt.h>
 #include <drizzled/message/table.pb.h>
 
 using namespace std;
@@ -78,6 +79,13 @@
   index->set_is_primary(is_primary);
   index->set_is_unique(is_unique);
 
+  int key_length= 0;
+
+  for(int i=0; i< num_index_parts; i++)
+    key_length+= compare_lengths[i];
+
+  index->set_key_length(key_length);
+
   while (x < num_index_parts)
   {
     index_part= index->add_index_part();
@@ -190,21 +198,77 @@
 
 }
 
+static void fill_table1(message::Table *table)
+{
+  message::Table::Field *field;
+  message::Table::TableOptions *tableopts;
+
+  table->set_name("t1");
+  table->set_type(message::Table::INTERNAL);
+
+  tableopts= table->mutable_options();
+  tableopts->set_comment("Table without a StorageEngine message");
+
+  {
+    field= table->add_field();
+    field->set_name("number");
+    field->set_type(message::Table::Field::INTEGER);
+  }
+
+}
+
+static void usage(char *argv0)
+{
+  cerr << "Usage:  " << argv0 << " [-t N] TABLE_NAME.dfe" << endl;
+  cerr << endl;
+  cerr << "-t N\tTable Number" << endl;
+  cerr << "\t0 - default" << endl;
+  cerr << endl;
+}
+
 int main(int argc, char* argv[])
 {
+  int opt;
+  int table_number= 0;
+
   GOOGLE_PROTOBUF_VERIFY_VERSION;
 
-  if (argc != 2)
+  while ((opt= getopt(argc, argv, "t:")) != -1)
   {
-    cerr << "Usage:  " << argv[0] << " SCHEMA" << endl;
-    return -1;
+    switch (opt)
+    {
+    case 't':
+      table_number= atoi(optarg);
+      break;
+    default:
+      usage(argv[0]);
+      exit(EXIT_FAILURE);
+    }
+  }
+
+  if (optind >= argc) {
+    fprintf(stderr, "Expected Table name argument\n\n");
+    usage(argv[0]);
+    exit(EXIT_FAILURE);
   }
 
   message::Table table;
 
-  fill_table(&table, "example_table");
+  switch (table_number)
+  {
+  case 0:
+    fill_table(&table, "example_table");
+    break;
+  case 1:
+    fill_table1(&table);
+    break;
+  default:
+    fprintf(stderr, "Invalid table number.\n\n");
+    usage(argv[0]);
+    exit(EXIT_FAILURE);
+  }
 
-  fstream output(argv[1], ios::out | ios::trunc | ios::binary);
+  fstream output(argv[optind], ios::out | ios::trunc | ios::binary);
   if (!table.SerializeToOstream(&output))
   {
     cerr << "Failed to write schema." << endl;

=== modified file 'drizzled/plugin/storage_engine.cc'
--- drizzled/plugin/storage_engine.cc	2010-02-04 08:14:46 +0000
+++ drizzled/plugin/storage_engine.cc	2010-02-08 01:35:22 +0000
@@ -579,7 +579,7 @@
     if (ret != ENOENT)
       *err= ret;
 
-    return *err == EEXIST;
+    return *err == EEXIST || *err != ENOENT;
   }
 };
 
@@ -705,6 +705,13 @@
                                                          identifier,
                                                          &src_proto);
 
+  if (error_proto == ER_CORRUPT_TABLE_DEFINITION)
+  {
+    my_error(ER_CORRUPT_TABLE_DEFINITION, MYF(0),
+             src_proto.InitializationErrorString().c_str());
+    return ER_CORRUPT_TABLE_DEFINITION;
+  }
+
   engine= plugin::StorageEngine::findByName(session,
                                             src_proto.engine().name());
 
@@ -732,6 +739,14 @@
   if (error_proto && error == 0)
     return 0;
 
+  if (((error_proto != EEXIST && error_proto != ENOENT)
+      && !engine && generate_warning)
+      | ( error && !engine && generate_warning))
+  {
+    my_error(ER_GET_ERRNO, MYF(0), error_proto);
+    return error_proto;
+  }
+
   if (error && generate_warning)
   {
     /*

=== modified file 'drizzled/table.cc'
--- drizzled/table.cc	2010-02-08 01:35:22 +0000
+++ drizzled/table.cc	2010-02-08 01:35:22 +0000
@@ -262,6 +262,12 @@
 {
   int error= 0;
 
+  if (! table.IsInitialized())
+  {
+    my_error(ER_CORRUPT_TABLE_DEFINITION, MYF(0), table.InitializationErrorString().c_str());
+    return ER_CORRUPT_TABLE_DEFINITION;
+  }
+
   share->setTableProto(new(nothrow) message::Table(table));
 
   share->storage_engine= plugin::StorageEngine::findByName(session, table.engine().name());

=== modified file 'plugin/blackhole/ha_blackhole.cc'
--- plugin/blackhole/ha_blackhole.cc	2010-02-04 08:14:46 +0000
+++ plugin/blackhole/ha_blackhole.cc	2010-02-08 01:35:22 +0000
@@ -259,6 +259,13 @@
   {
     close(fd);
     delete input;
+    if (! table_proto->IsInitialized())
+    {
+      my_error(ER_CORRUPT_TABLE_DEFINITION, MYF(0),
+               table_proto->InitializationErrorString().c_str());
+      return ER_CORRUPT_TABLE_DEFINITION;
+    }
+
     return HA_ERR_CRASHED_ON_USAGE;
   }
 

=== added directory 'plugin/tableprototester'
=== added file 'plugin/tableprototester/plugin.ini'
--- plugin/tableprototester/plugin.ini	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/plugin.ini	2010-02-08 01:35:22 +0000
@@ -0,0 +1,5 @@
+[plugin]
+title=Table Proto Message Tester
+description=Used only to test other bits of the server.
+sources=tableprototester.cc
+headers=tableprototester.h

=== added file 'plugin/tableprototester/tableprototester.cc'
--- plugin/tableprototester/tableprototester.cc	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tableprototester.cc	2010-02-08 01:35:22 +0000
@@ -0,0 +1,292 @@
+/*
+  Copyright (C) 2010 Stewart Smith
+
+  This program is free software; you can redistribute it and/or
+  modify it under the terms of the GNU General Public License
+  as published by the Free Software Foundation; either version 2
+  of the License, or (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+*/
+
+#include "config.h"
+#include <drizzled/table.h>
+#include <drizzled/error.h>
+#include "drizzled/internal/my_pthread.h"
+
+#include "tableprototester.h"
+
+#include <fcntl.h>
+
+#include <string>
+#include <map>
+#include <fstream>
+#include <drizzled/message/table.pb.h>
+#include "drizzled/internal/m_string.h"
+
+#include "drizzled/global_charset_info.h"
+
+
+using namespace std;
+using namespace google;
+using namespace drizzled;
+
+#define TABLEPROTOTESTER_EXT ".TBT"
+
+static const char *TableProtoTesterCursor_exts[] = {
+  NULL
+};
+
+class TableProtoTesterEngine : public drizzled::plugin::StorageEngine
+{
+public:
+  TableProtoTesterEngine(const string &name_arg)
+   : drizzled::plugin::StorageEngine(name_arg,
+                                     HTON_NULL_IN_KEY |
+                                     HTON_CAN_INDEX_BLOBS |
+                                     HTON_SKIP_STORE_LOCK |
+                                     HTON_AUTO_PART_KEY |
+                                     HTON_HAS_DATA_DICTIONARY)
+  {
+    table_definition_ext= TABLEPROTOTESTER_EXT;
+  }
+
+  virtual Cursor *create(TableShare &table,
+                         drizzled::memory::Root *mem_root)
+  {
+    return new (mem_root) TableProtoTesterCursor(*this, table);
+  }
+
+  const char **bas_ext() const {
+    return TableProtoTesterCursor_exts;
+  }
+
+  int doCreateTable(Session*,
+                    const char *,
+                    Table&,
+                    drizzled::message::Table&);
+
+  int doDropTable(Session&, const string table_name);
+
+  int doGetTableDefinition(Session& session,
+                           const char* path,
+                           const char *db,
+                           const char *table_name,
+                           const bool is_tmp,
+                           drizzled::message::Table *table_proto);
+
+  void doGetTableNames(drizzled::CachedDirectory &directory,
+                       string&, set<string>& set_of_names)
+  {
+    (void)directory;
+    set_of_names.insert("t1");
+
+  }
+
+  /* The following defines can be increased if necessary */
+  uint32_t max_supported_keys()          const { return 64; }
+  uint32_t max_supported_key_length()    const { return 1000; }
+  uint32_t max_supported_key_part_length() const { return 1000; }
+
+  uint32_t index_flags(enum  ha_key_alg) const
+  {
+    return (HA_READ_NEXT |
+            HA_READ_PREV |
+            HA_READ_RANGE |
+            HA_READ_ORDER |
+            HA_KEYREAD_ONLY);
+  }
+
+};
+
+TableProtoTesterCursor::TableProtoTesterCursor(drizzled::plugin::StorageEngine &engine_arg,
+                           TableShare &table_arg)
+  :Cursor(engine_arg, table_arg)
+{ }
+
+int TableProtoTesterCursor::open(const char *, int, uint32_t)
+{
+  return(0);
+}
+
+int TableProtoTesterCursor::close(void)
+{
+  return 0;
+}
+
+int TableProtoTesterEngine::doCreateTable(Session*, const char *,
+                                   Table&,
+                                   drizzled::message::Table&)
+{
+  return EEXIST;
+}
+
+
+int TableProtoTesterEngine::doDropTable(Session&, const string)
+{
+  return EPERM;
+}
+
+static void fill_table1(message::Table *table)
+{
+  message::Table::Field *field;
+  message::Table::TableOptions *tableopts;
+
+  table->set_name("t1");
+  table->set_type(message::Table::INTERNAL);
+
+  tableopts= table->mutable_options();
+  tableopts->set_comment("Table without a StorageEngine message");
+
+  {
+    field= table->add_field();
+    field->set_name("number");
+    field->set_type(message::Table::Field::INTEGER);
+  }
+
+}
+int TableProtoTesterEngine::doGetTableDefinition(Session&,
+                                          const char* path,
+                                          const char *,
+                                          const char *,
+                                          const bool,
+                                          drizzled::message::Table *table_proto)
+{
+  if (strcmp(path, "./test/t1") == 0)
+  {
+    if (table_proto)
+      fill_table1(table_proto);
+    return EEXIST;
+  }
+  return ENOENT;
+}
+
+const char *TableProtoTesterCursor::index_type(uint32_t)
+{
+  return("BTREE");
+}
+
+int TableProtoTesterCursor::write_row(unsigned char *)
+{
+  return(table->next_number_field ? update_auto_increment() : 0);
+}
+
+int TableProtoTesterCursor::rnd_init(bool)
+{
+  return(0);
+}
+
+
+int TableProtoTesterCursor::rnd_next(unsigned char *)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::rnd_pos(unsigned char *, unsigned char *)
+{
+  assert(0);
+  return(0);
+}
+
+
+void TableProtoTesterCursor::position(const unsigned char *)
+{
+  assert(0);
+  return;
+}
+
+
+int TableProtoTesterCursor::info(uint32_t flag)
+{
+  memset(&stats, 0, sizeof(stats));
+  if (flag & HA_STATUS_AUTO)
+    stats.auto_increment_value= 1;
+  return(0);
+}
+
+
+int TableProtoTesterCursor::index_read_map(unsigned char *, const unsigned char *,
+                                 key_part_map, enum ha_rkey_function)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::index_read_idx_map(unsigned char *, uint32_t, const unsigned char *,
+                                     key_part_map, enum ha_rkey_function)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::index_read_last_map(unsigned char *, const unsigned char *, key_part_map)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::index_next(unsigned char *)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::index_prev(unsigned char *)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::index_first(unsigned char *)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+
+int TableProtoTesterCursor::index_last(unsigned char *)
+{
+  return(HA_ERR_END_OF_FILE);
+}
+
+static drizzled::plugin::StorageEngine *tableprototester_engine= NULL;
+
+static int tableprototester_init(drizzled::plugin::Registry &registry)
+{
+
+  tableprototester_engine= new TableProtoTesterEngine("TABLEPROTOTESTER");
+  registry.add(tableprototester_engine);
+
+  return 0;
+}
+
+static int tableprototester_fini(drizzled::plugin::Registry &registry)
+{
+  registry.remove(tableprototester_engine);
+  delete tableprototester_engine;
+
+  return 0;
+}
+
+DRIZZLE_DECLARE_PLUGIN
+{
+  DRIZZLE_VERSION_ID,
+  "TABLEPROTOTESTER",
+  "1.0",
+  "Stewart Smith",
+  "Used to test rest of server with various table proto messages",
+  PLUGIN_LICENSE_GPL,
+  tableprototester_init,     /* Plugin Init */
+  tableprototester_fini,     /* Plugin Deinit */
+  NULL,               /* status variables */
+  NULL,               /* system variables */
+  NULL                /* config options   */
+}
+DRIZZLE_DECLARE_PLUGIN_END;

=== added file 'plugin/tableprototester/tableprototester.h'
--- plugin/tableprototester/tableprototester.h	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tableprototester.h	2010-02-08 01:35:22 +0000
@@ -0,0 +1,58 @@
+/*
+  Copyright (C) 2010 Stewart Smith
+
+  This program is free software; you can redistribute it and/or
+  modify it under the terms of the GNU General Public License
+  as published by the Free Software Foundation; either version 2
+  of the License, or (at your option) any later version.
+
+  This program is distributed in the hope that it will be useful,
+  but WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+  GNU General Public License for more details.
+
+  You should have received a copy of the GNU General Public License
+  along with this program; if not, write to the Free Software
+  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+*/
+
+#ifndef PLUGIN_TABLEPROTOTESTER_TABLEPROTOTESTER_H
+#define PLUGIN_TABLEPROTOTESTER_TABLEPROTOTESTER_H
+
+#include <drizzled/cursor.h>
+
+class TableProtoTesterCursor: public drizzled::Cursor
+{
+public:
+  TableProtoTesterCursor(drizzled::plugin::StorageEngine &engine, drizzled::TableShare &table_arg);
+  ~TableProtoTesterCursor()
+  {}
+
+  /*
+    The name of the index type that will be used for display
+    don't implement this method unless you really have indexes
+  */
+  const char *index_type(uint32_t key_number);
+  uint32_t index_flags(uint32_t inx) const;
+  int open(const char *name, int mode, uint32_t test_if_locked);
+  int close(void);
+  int write_row(unsigned char * buf);
+  int rnd_init(bool scan);
+  int rnd_next(unsigned char *buf);
+  int rnd_pos(unsigned char * buf, unsigned char *pos);
+
+  int index_read_map(unsigned char * buf, const unsigned char * key, drizzled::key_part_map keypart_map,
+                     drizzled::ha_rkey_function find_flag);
+  int index_read_idx_map(unsigned char * buf, uint32_t idx, const unsigned char * key,
+                         drizzled::key_part_map keypart_map,
+                         drizzled::ha_rkey_function find_flag);
+  int index_read_last_map(unsigned char * buf, const unsigned char * key, drizzled::key_part_map keypart_map);
+  int index_next(unsigned char * buf);
+  int index_prev(unsigned char * buf);
+  int index_first(unsigned char * buf);
+  int index_last(unsigned char * buf);
+  void position(const unsigned char *record);
+  int info(uint32_t flag);
+};
+
+#endif /* PLUGIN_TABLEPROTOTESTER_TABLEPROTOTESTER_H */

=== added directory 'plugin/tableprototester/tests'
=== added directory 'plugin/tableprototester/tests/r'
=== added file 'plugin/tableprototester/tests/r/basic.result'
--- plugin/tableprototester/tests/r/basic.result	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/r/basic.result	2010-02-08 01:35:22 +0000
@@ -0,0 +1,3 @@
+SHOW TABLES;
+Tables_in_test
+t1

=== added file 'plugin/tableprototester/tests/r/drop_table.result'
--- plugin/tableprototester/tests/r/drop_table.result	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/r/drop_table.result	2010-02-08 01:35:22 +0000
@@ -0,0 +1,2 @@
+DROP TABLE t1;
+ERROR HY000: Got error 17 from storage engine

=== added file 'plugin/tableprototester/tests/r/missing_engine.result'
--- plugin/tableprototester/tests/r/missing_engine.result	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/r/missing_engine.result	2010-02-08 01:35:22 +0000
@@ -0,0 +1,5 @@
+SHOW TABLES;
+Tables_in_test
+t1
+SHOW CREATE TABLE t1;
+ERROR HY000: Corrupt or invalid table definition: engine

=== added directory 'plugin/tableprototester/tests/t'
=== added file 'plugin/tableprototester/tests/t/basic-master.opt'
--- plugin/tableprototester/tests/t/basic-master.opt	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/t/basic-master.opt	2010-02-08 01:35:22 +0000
@@ -0,0 +1,1 @@
+--plugin_add=tableprototester

=== added file 'plugin/tableprototester/tests/t/basic.test'
--- plugin/tableprototester/tests/t/basic.test	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/t/basic.test	2010-02-08 01:35:22 +0000
@@ -0,0 +1,1 @@
+SHOW TABLES;

=== added file 'plugin/tableprototester/tests/t/drop_table-master.opt'
--- plugin/tableprototester/tests/t/drop_table-master.opt	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/t/drop_table-master.opt	2010-02-08 01:35:22 +0000
@@ -0,0 +1,1 @@
+--plugin_add=tableprototester

=== added file 'plugin/tableprototester/tests/t/drop_table.test'
--- plugin/tableprototester/tests/t/drop_table.test	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/t/drop_table.test	2010-02-08 01:35:22 +0000
@@ -0,0 +1,2 @@
+--error 1030
+DROP TABLE t1;

=== added file 'plugin/tableprototester/tests/t/missing_engine-master.opt'
--- plugin/tableprototester/tests/t/missing_engine-master.opt	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/t/missing_engine-master.opt	2010-02-08 01:35:22 +0000
@@ -0,0 +1,1 @@
+--plugin_add=tableprototester

=== added file 'plugin/tableprototester/tests/t/missing_engine.test'
--- plugin/tableprototester/tests/t/missing_engine.test	1970-01-01 00:00:00 +0000
+++ plugin/tableprototester/tests/t/missing_engine.test	2010-02-08 01:35:22 +0000
@@ -0,0 +1,5 @@
+
+SHOW TABLES;
+
+--error 1693
+SHOW CREATE TABLE t1;