Skip to content

Commit

Permalink
Minor code cleanup: validation of options to member function.
Browse files Browse the repository at this point in the history
  • Loading branch information
@pastcompute
pastcompute committed Feb 28, 2015
1 parent fbcabb2 commit f8e0f1a
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 70 deletions.
93 changes: 64 additions & 29 deletions storage/oqgraph/ha_oqgraph.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,56 +434,91 @@ void ha_oqgraph::fprint_error(const char* fmt, ...)
va_end(ap);
}

/**
* Check that the currently referenced OQGRAPH table definition, on entry to open(), has sane OQGRAPH options.
* (This does not check the backing store, but the OQGRAPH virtual table options)
*
* @return true if OK, or false if an option is invalid.
*/
bool ha_oqgraph::validate_oqgraph_table_options()
{
// Note when called from open(), we should not expect this method to fail except in the case of bugs; the fact that it does is
// could be construed as a bug. I think in practice however, this is because CREATE TABLE calls both create() and open(),
// and it is possible to do something like ALTER TABLE x DESTID='y' to _change_ the options.
// Thus we need to sanity check from open() until and unless we get around to extending ha_oqgraph to properly handle ALTER TABLE,
// after which we could change things to call this method from create() and the ALTER TABLE handling code instead.
// It may still be sensible to call this from open() anyway, in case someone somewhere upgrades from a broken table definition...

ha_table_option_struct *options = table->s->option_struct;
// Catch cases where table was not constructed properly
// Note - need to return -1 so our error text gets reported
if (!options) {
// This should only happen if there is a bug elsewhere in the storage engine, because ENGINE itself is an attribute
fprint_error("Invalid OQGRAPH backing store (null attributes)");
}
else if (!options->table_name || !*options->table_name) {
// The first condition indicates no DATA_TABLE option, the second if the user specified DATA_TABLE=''
fprint_error("Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)");
// if table_name option is present but doesn't actually exist, we will fail later
}
else if (!options->origid || !*options->origid) {
// The first condition indicates no ORIGID option, the second if the user specified ORIGID=''
fprint_error("Invalid OQGRAPH backing store description (unspecified or empty origid attribute)");
// if ORIGID option is present but doesn't actually exist, we will fail later
}
else if (!options->destid || !*options->destid) {
// The first condition indicates no DESTID option, the second if the user specified DESTID=''
fprint_error("Invalid OQGRAPH backing store description (unspecified or empty destid attribute)");
// if DESTID option is present but doesn't actually exist, we will fail later
} else {
// weight is optional...
return true;
}
// Fault
return false;
}

/**
* Open the OQGRAPH engine 'table'.
*
* An OQGRAPH table is effectively similar to a view over the underlying backing table,
* attribute 'data_table', but where the result returned by a query depends on the
* value of the 'latch' column specified to the query. Therefore,
* when mysqld opens us, we need to open the corresponding backing table 'data_table'
* An OQGRAPH table is effectively similar to a view over the underlying backing table, attribute 'data_table', but where the
* result returned by a query depends on the value of the 'latch' column specified to the query.
* Therefore, when mysqld opens us, we need to open the corresponding backing table 'data_table'.
*
* Conceptually, the backing store could be any kind of object having queryable semantics, including a SQL VIEW.
* However, for that to work in practice would require us to hook into the right level of the MYSQL API.
* Presently, only objects that can be opened using the internal mechanisms can be used: INNODB, MYISAM, etc.
* The intention is to borrow from ha_connect and use the mysql client library to access the backing store.
*
*/
int ha_oqgraph::open(const char *name, int mode, uint test_if_locked)
{
DBUG_ENTER("ha_oqgraph::open");

DBUG_PRINT( "oq-debug", ("thd: 0x%lx; open(name=%s,mode=%d,test_if_locked=%u)", (long) current_thd, name, mode, test_if_locked));

// So, we took a peek inside handler::ha_open() and learned a few things:
// * this->table is set by handler::ha_open() before calling open().
// Note that from this we can only assume that MariaDB knows what it is doing and wont call open() other anything else
// relying on this-0>table, re-entrantly...
// * this->table_share should never be set back to NULL, an assertion checks for this in ha_open() after open()
// * this->table_share is initialised in the constructor of handler
// * this->table_share is only otherwise changed by this->change_table_ptr())
// We also discovered that an assertion is raised if table->s is not table_share before calling open())

DBUG_ASSERT(!have_table_share);
DBUG_ASSERT(graph == NULL);

THD* thd = current_thd;
thd_set_ha_data( thd, ht, (void*)1);
// Before doing anything, make sure we have DATA_TABLE, ORIGID and DESTID not empty
if (!validate_oqgraph_table_options()) { DBUG_RETURN(-1); }

ha_table_option_struct *options= table->s->option_struct;

// Catch cases where table was not constructed properly
// Note - need to return -1 so our error text gets reported
if (!options) {
fprint_error("Invalid OQGRAPH backing store (null attributes)");
DBUG_RETURN(-1);
}
if (!options->table_name || !*options->table_name) {
fprint_error("Invalid OQGRAPH backing store (unspecified or empty data_table attribute)");
// if table_name if present but doesnt actually exist, we will fail out below
// when we call open_table_def(). same probably applies for the id fields
DBUG_RETURN(-1);
}
if (!options->origid || !*options->origid) {
fprint_error("Invalid OQGRAPH backing store (unspecified or empty origid attribute)");
DBUG_RETURN(-1);
}
if (!options->destid || !*options->destid) {
fprint_error("Invalid OQGRAPH backing store (unspecified or empty destid attribute)");
DBUG_RETURN(-1);
}
// weight is optional

error_message.length(0);

origid= destid= weight= 0;

// Here we're abusing init_tmp_table_share() which is normally only works for thread-local shares.
THD* thd = current_thd;
init_tmp_table_share( thd, share, table->s->db.str, table->s->db.length, options->table_name, "");
// because of that, we need to reinitialize the memroot (to reset MY_THREAD_SPECIFIC flag)
DBUG_ASSERT(share->mem_root.used == NULL); // it's still empty
Expand Down
4 changes: 3 additions & 1 deletion storage/oqgraph/ha_oqgraph.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,10 @@ class ha_oqgraph: public handler
}

private:
int oqgraph_check_table_structure (TABLE *table_arg);

// Various helper functions
int oqgraph_check_table_structure (TABLE *table_arg);
bool validate_oqgraph_table_options();
void update_key_stats();
String error_message;
};
40 changes: 20 additions & 20 deletions storage/oqgraph/mysql-test/oqgraph/create_attr.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,49 +19,49 @@ PRIMARY KEY (id),
KEY name (info)
) DEFAULT CHARSET=latin1;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH;
# Expect: 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)'
# Expect: 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, ORIGID='id', DESTID='id2';
# Expect: 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)'
# Expect: 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='', ORIGID='id', DESTID='id2';
# Expect: 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)'
# Expect: 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='', ORIGID='id';
# Expect: 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)'
# Expect: 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='', DESTID='id2';
# Expect: 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)'
# Expect: 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty data_table attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty data_table attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='bogus', ORIGID='id', DESTID='id2';
# Expect: 'Table 'test.bogus' doesn't exist''
DESCRIBE oqtable;
ERROR 42S02: Table 'test.bogus' doesn't exist
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='not_backing';
# Expect 'Invalid OQGRAPH backing store (unspecified or empty origid attribute)'
# Expect 'Invalid OQGRAPH backing store description (unspecified or empty origid attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty origid attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty origid attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='backing', DESTID='id2';
# Expect 'Invalid OQGRAPH backing store (unspecified or empty origid attribute)'
# Expect 'Invalid OQGRAPH backing store description (unspecified or empty origid attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty origid attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty origid attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='backing',ORIGID='', DESTID='id2';
# Expect 'Invalid OQGRAPH backing store (unspecified or empty origid attribute)'
# Expect 'Invalid OQGRAPH backing store description (unspecified or empty origid attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty origid attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty origid attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='backing',ORIGID='bogus', DESTID='id2';
# Expect Invalid OQGRAPH backing store ('/oqtable'.origid attribute not set to a valid column of 'backing')'
Expand All @@ -74,14 +74,14 @@ DESCRIBE oqtable;
ERROR HY000: Got error -1 'Column 'backing.not_id_type' (origid) is not a not-null integer type' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='backing',ORIGID='id';
# Expect 'Invalid OQGRAPH backing store (unspecified or empty destid attribute)'
# Expect 'Invalid OQGRAPH backing store description (unspecified or empty destid attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty destid attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty destid attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='backing',ORIGID='id',DESTID='';
# Expect 'Invalid OQGRAPH backing store (unspecified or empty destid attribute)'
# Expect 'Invalid OQGRAPH backing store description (unspecified or empty destid attribute)'
DESCRIBE oqtable;
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store (unspecified or empty destid attribute)' from OQGRAPH
ERROR HY000: Got error -1 'Invalid OQGRAPH backing store description (unspecified or empty destid attribute)' from OQGRAPH
DROP TABLE IF EXISTS oqtable;
CREATE TABLE oqtable ( latch varchar(32) NULL, origid BIGINT UNSIGNED NULL, destid BIGINT UNSIGNED NULL, weight DOUBLE NULL, seq BIGINT UNSIGNED NULL, linkid BIGINT UNSIGNED NULL, KEY (latch, origid, destid) USING HASH, KEY (latch, destid, origid) USING HASH ) ENGINE=OQGRAPH, DATA_TABLE='backing',ORIGID='id',DESTID='bogus';
# Expect Invalid OQGRAPH backing store ('/oqtable'.destid attribute not set to a valid column of 'backing')'
Expand Down
Loading

0 comments on commit f8e0f1a

Please sign in to comment.