From c763bd65405dd8aadd90881a46a739dc40b14d6d Mon Sep 17 00:00:00 2001 From: Jay Edgar Date: Wed, 13 Apr 2016 14:44:00 -0700 Subject: [PATCH] Add an error when attempting to create foreign keys Summary: Currently if you attempt to create foreign keys when using the RocksDB engine no error is generated and the foreign key constraint is ignore (though a key is created). Add an error so that this will not be allowed - any create table (or alter table) that adds a constraint will generate an error. Issue 205 Test Plan: MTR Reviewers: yoshinorim, hermanlee4 Reviewed By: hermanlee4 Subscribers: webscalesql-eng Differential Revision: https://reviews.facebook.net/D56703 --- mysql-test/suite/rocksdb/r/foreign_key.result | 25 ++ mysql-test/suite/rocksdb/t/foreign_key.test | 45 ++++ storage/rocksdb/CMakeLists.txt | 1 + storage/rocksdb/ha_rocksdb.cc | 65 ++++- storage/rocksdb/ha_rocksdb.h | 2 + storage/rocksdb/rdb_utils.cc | 239 ++++++++++++++++++ storage/rocksdb/rdb_utils.h | 26 ++ 7 files changed, 402 insertions(+), 1 deletion(-) create mode 100644 mysql-test/suite/rocksdb/r/foreign_key.result create mode 100644 mysql-test/suite/rocksdb/t/foreign_key.test create mode 100644 storage/rocksdb/rdb_utils.cc diff --git a/mysql-test/suite/rocksdb/r/foreign_key.result b/mysql-test/suite/rocksdb/r/foreign_key.result new file mode 100644 index 000000000000..483be726bb32 --- /dev/null +++ b/mysql-test/suite/rocksdb/r/foreign_key.result @@ -0,0 +1,25 @@ +DROP TABLE IF EXISTS t1, t2; +CREATE TABLE t1 (b INT PRIMARY KEY); +CREATE TABLE t2 (a INT NOT NULL, b INT NOT NULL, FOREIGN KEY (b) REFERENCES t1(b)); +ERROR 42000: MyRocks does not currently support foreign key constraints +CREATE TABLE t2 (a INT NOT NULL, bforeign INT NOT NULL); +DROP TABLE t2; +CREATE TABLE t2 (a INT NOT NULL, foreignkey INT NOT NULL); +DROP TABLE t2; +CREATE TABLE t2 (a INT NOT NULL, bforeign INT not null, FOREIGN KEY (bforeign) REFERENCES t1(b)); +ERROR 42000: MyRocks does not currently support foreign key constraints +CREATE TABLE t2 (a INT NOT NULL, b INT NOT NULL); +ALTER TABLE t2 ADD FOREIGN KEY (b) REFERENCES t1(b); +ERROR 42000: MyRocks does not currently support foreign key constraints +DROP TABLE t2; +CREATE TABLE t2 (a INT NOT NULL); +ALTER TABLE t2 ADD bforeign INT NOT NULL; +DROP TABLE t2; +CREATE TABLE t2 (a INT NOT NULL); +ALTER TABLE t2 ADD foreignkey INT NOT NULL; +DROP TABLE t2; +CREATE TABLE t2 (a INT NOT NULL); +ALTER TABLE t2 ADD bforeign INT NOT NULL, ADD FOREIGN KEY (bforeign) REFERENCES t1(b); +ERROR 42000: MyRocks does not currently support foreign key constraints +DROP TABLE t2; +DROP TABLE t1; diff --git a/mysql-test/suite/rocksdb/t/foreign_key.test b/mysql-test/suite/rocksdb/t/foreign_key.test new file mode 100644 index 000000000000..bd8071b1b5ea --- /dev/null +++ b/mysql-test/suite/rocksdb/t/foreign_key.test @@ -0,0 +1,45 @@ +--disable_warnings +DROP TABLE IF EXISTS t1, t2; +--enable_warnings + +CREATE TABLE t1 (b INT PRIMARY KEY); + +# Try simple foreign key - should fail +--error ER_NOT_SUPPORTED_YET +CREATE TABLE t2 (a INT NOT NULL, b INT NOT NULL, FOREIGN KEY (b) REFERENCES t1(b)); + +# Try simple valid syntax with 'foreign' as part - should succeed +CREATE TABLE t2 (a INT NOT NULL, bforeign INT NOT NULL); +DROP TABLE t2; + +# Try simple valid syntax with 'foreign' and 'key' as part (with no space) - should succeed +CREATE TABLE t2 (a INT NOT NULL, foreignkey INT NOT NULL); +DROP TABLE t2; + +# Try with valid id containing 'foreign' and then a foreign key - should fail +--error ER_NOT_SUPPORTED_YET +CREATE TABLE t2 (a INT NOT NULL, bforeign INT not null, FOREIGN KEY (bforeign) REFERENCES t1(b)); + +CREATE TABLE t2 (a INT NOT NULL, b INT NOT NULL); +# Alter with foreign key - should fail +--error ER_NOT_SUPPORTED_YET +ALTER TABLE t2 ADD FOREIGN KEY (b) REFERENCES t1(b); +DROP TABLE t2; + +# Alter with valid syntax that contains 'foreign' - should succeed +CREATE TABLE t2 (a INT NOT NULL); +ALTER TABLE t2 ADD bforeign INT NOT NULL; +DROP TABLE t2; + +# Alter with valid syntax that contains 'foreign' and 'key' (no space) - should succeed +CREATE TABLE t2 (a INT NOT NULL); +ALTER TABLE t2 ADD foreignkey INT NOT NULL; +DROP TABLE t2; + +# Alter with valid syntax that contains 'foreign' and then foreign key - should fail +CREATE TABLE t2 (a INT NOT NULL); +--error ER_NOT_SUPPORTED_YET +ALTER TABLE t2 ADD bforeign INT NOT NULL, ADD FOREIGN KEY (bforeign) REFERENCES t1(b); +DROP TABLE t2; + +DROP TABLE t1; diff --git a/storage/rocksdb/CMakeLists.txt b/storage/rocksdb/CMakeLists.txt index c117688c9fd7..51538dc7c82e 100644 --- a/storage/rocksdb/CMakeLists.txt +++ b/storage/rocksdb/CMakeLists.txt @@ -35,6 +35,7 @@ SET(ROCKSDB_SOURCES rdb_i_s.cc rdb_i_s.h rdb_perf_context.cc rdb_perf_context.h rdb_mutex_wrapper.cc rdb_mutex_wrapper.h + rdb_utils.cc rdb_utils.h ${ROCKSDB_LIB_SOURCES} ) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index f95bf961aa2c..c8b2fc02c9d9 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -4114,6 +4114,61 @@ int rocksdb_normalize_tablename(const char *tablename, return 0; } +/* + Check to see if the user's original statement includes foreign key + references +*/ +bool ha_rocksdb::contains_foreign_key(THD* thd) +{ + bool success; + const char* str = thd_query_string(thd)->str; + + DBUG_ASSERT(str != nullptr); + + while (*str != '\0') + { + // Scan from our current pos looking for 'FOREIGN' + str = rdb_find_in_string(str, "FOREIGN", &success); + if (!success) + { + return false; + } + + // Skip past the found "FOREIGN' + str = rdb_check_next_token(&my_charset_bin, str, "FOREIGN", &success); + DBUG_ASSERT(success); + + if (!my_isspace(&my_charset_bin, *str)) + { + return false; + } + + // See if the next token is 'KEY' + str = rdb_check_next_token(&my_charset_bin, str, "KEY", &success); + if (!success) + { + continue; + } + + // See if the next token is '(' + str = rdb_check_next_token(&my_charset_bin, str, "(", &success); + if (!success) + { + // There is an optional index id after 'FOREIGN KEY', skip it + str = rdb_skip_id(&my_charset_bin, str); + + // Now check for '(' again + str = rdb_check_next_token(&my_charset_bin, str, "(", &success); + } + + // If we have found 'FOREIGN KEY [] (' we can be confident we have + // a foreign key clause. + return success; + } + + // We never found a valid foreign key clause + return false; +} /** @brief @@ -4188,8 +4243,9 @@ int ha_rocksdb::create(const char *name, TABLE *table_arg, StringBuffer<256> strbuf; DBUG_ENTER("ha_rocksdb::create"); + THD *thd = my_core::thd_get_current_thd(); + if (get_table_if_exists(name)) { - THD *thd = my_core::thd_get_current_thd(); if (thd->lex->sql_command == SQLCOM_TRUNCATE) { if (delete_table(name)) { DBUG_RETURN(HA_ERR_INTERNAL_ERROR); @@ -4215,6 +4271,13 @@ int ha_rocksdb::create(const char *name, TABLE *table_arg, if (rocksdb_normalize_tablename(name, &strbuf)) DBUG_RETURN(HA_ERR_INTERNAL_ERROR); + if (contains_foreign_key(thd)) + { + my_printf_error(ER_NOT_SUPPORTED_YET, "MyRocks does not currently support " + "foreign key constraints", MYF(0)); + DBUG_RETURN(HA_ERR_INTERNAL_ERROR); + } + /* TODO(alexyang): Temporarily disable unique indexes support when there is no Primary Key diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h index fb24fa424b67..3b377c585187 100644 --- a/storage/rocksdb/ha_rocksdb.h +++ b/storage/rocksdb/ha_rocksdb.h @@ -733,6 +733,8 @@ class ha_rocksdb: public handler __attribute__((__nonnull__, __warn_unused_result__)); void read_thd_vars(THD *thd) __attribute__((__nonnull__)); + bool contains_foreign_key(THD* thd) + __attribute__((__nonnull__, __warn_unused_result__)); public: int index_init(uint idx, bool sorted) __attribute__((__warn_unused_result__)); int index_end() __attribute__((__warn_unused_result__)); diff --git a/storage/rocksdb/rdb_utils.cc b/storage/rocksdb/rdb_utils.cc new file mode 100644 index 000000000000..1cd1e1459486 --- /dev/null +++ b/storage/rocksdb/rdb_utils.cc @@ -0,0 +1,239 @@ +/* + Copyright (c) 2016, Facebook, Inc. + + 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; version 2 of the License. + + 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., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ + +/* This C++ file's header */ +#include "./rdb_utils.h" + +/* C++ standard header files */ +#include + +/* C standard header files */ +#include + +namespace myrocks { + +/* + Skip past any spaces in the input +*/ +const char* rdb_skip_spaces(struct charset_info_st* cs, const char *str) +{ + DBUG_ASSERT(cs != nullptr); + DBUG_ASSERT(str != nullptr); + + while (my_isspace(cs, *str)) + { + str++; + } + + return str; +} + +/* + Compare (ignoring case) to see if str2 is the next data in str1. + Note that str1 can be longer but we only compare up to the number + of characters in str2. +*/ +bool rdb_compare_strings_ic(const char *str1, const char *str2) +{ + DBUG_ASSERT(str1 != nullptr); + DBUG_ASSERT(str2 != nullptr); + + // Scan through the strings + size_t ii; + for (ii = 0; str2[ii]; ii++) + { + if (toupper(static_cast(str1[ii])) != + toupper(static_cast(str2[ii]))) + { + return false; + } + } + + return true; +} + +/* + Scan through an input string looking for pattern, ignoring case + and skipping all data enclosed in quotes. +*/ +const char* rdb_find_in_string(const char *str, const char *pattern, + bool *succeeded) +{ + char quote = '\0'; + bool escape = false; + + DBUG_ASSERT(str != nullptr); + DBUG_ASSERT(pattern != nullptr); + DBUG_ASSERT(succeeded != nullptr); + + *succeeded = false; + + for ( ; *str; str++) + { + /* If we found a our starting quote character */ + if (*str == quote) + { + /* If it was escaped ignore it */ + if (escape) + { + escape = false; + } + /* Otherwise we are now outside of the quoted string */ + else + { + quote = '\0'; + } + } + /* Else if we are currently inside a quoted string? */ + else if (quote != '\0') + { + /* If so, check for the escape character */ + escape = !escape && *str == '\\'; + } + /* Else if we found a quote we are starting a quoted string */ + else if (*str == '"' || *str == '\'' || *str == '`') + { + quote = *str; + } + /* Else we are outside of a quoted string - look for our pattern */ + else + { + if (rdb_compare_strings_ic(str, pattern)) + { + *succeeded = true; + return str; + } + } + } + + // Return the character after the found pattern or the null terminateor + // if the pattern wasn't found. + return str; +} + +/* + See if the next valid token matches the specified string +*/ +const char* rdb_check_next_token(struct charset_info_st* cs, const char *str, + const char *pattern, bool *succeeded) +{ + DBUG_ASSERT(cs != nullptr); + DBUG_ASSERT(str != nullptr); + DBUG_ASSERT(pattern != nullptr); + DBUG_ASSERT(succeeded != nullptr); + + // Move past any spaces + str = rdb_skip_spaces(cs, str); + + // See if the next characters match the pattern + if (rdb_compare_strings_ic(str, pattern)) + { + *succeeded = true; + return str + strlen(pattern); + } + + *succeeded = false; + return str; +} + +/* + Parse id +*/ +const char* rdb_parse_id(struct charset_info_st* cs, const char *str, + std::string *id) +{ + DBUG_ASSERT(cs != nullptr); + DBUG_ASSERT(str != nullptr); + + // Move past any spaces + str = rdb_skip_spaces(cs, str); + + if (*str == '\0') + { + return str; + } + + char quote = '\0'; + if (*str == '`' || *str == '"') + { + quote = *str++; + } + + size_t len = 0; + const char* start = str; + + if (quote != '\0') + { + for ( ; ; ) + { + if (*str == '\0') + { + return str; + } + + if (*str == quote) + { + str++; + if (*str != quote) + { + break; + } + } + + str++; + len++; + } + } + else + { + while (!my_isspace(cs, *str) && *str != '(' && *str != ')' && + *str != '.' && *str != ',' && *str != '\0') + { + str++; + len++; + } + } + + // If the user requested the id create it and return it + if (id != nullptr) + { + *id = std::string(""); + id->reserve(len); + while (len--) + { + *id += *start; + if (*start++ == quote) + { + start++; + } + } + } + + return str; +} + +/* + Skip id +*/ +const char* rdb_skip_id(struct charset_info_st* cs, const char *str) +{ + DBUG_ASSERT(cs != nullptr); + DBUG_ASSERT(str != nullptr); + + return rdb_parse_id(cs, str, nullptr); +} + +} // namespace myrocks diff --git a/storage/rocksdb/rdb_utils.h b/storage/rocksdb/rdb_utils.h index d343d28fd2a6..4980cf833edf 100644 --- a/storage/rocksdb/rdb_utils.h +++ b/storage/rocksdb/rdb_utils.h @@ -15,6 +15,9 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ #pragma once +/* C++ header files */ +#include + /* MySQL header files */ #include "./sql_string.h" @@ -57,4 +60,27 @@ inline uchar* rdb_str_to_uchar_ptr(String * str) return reinterpret_cast(str->c_ptr()); } +/* + Helper function to parse strings +*/ +const char* rdb_skip_spaces(struct charset_info_st* cs, const char *str) + __attribute__((__nonnull__, __warn_unused_result__)); + +bool rdb_compare_strings_ic(const char *str1, const char *str2) + __attribute__((__nonnull__, __warn_unused_result__)); + +const char* rdb_find_in_string(const char *str, const char *pattern, + bool *succeeded) + __attribute__((__nonnull__, __warn_unused_result__)); + +const char* rdb_check_next_token(struct charset_info_st* cs, const char *str, + const char *pattern, bool *succeeded) + __attribute__((__nonnull__, __warn_unused_result__)); + +const char* rdb_parse_id(struct charset_info_st* cs, const char *str, + std::string *id) + __attribute__((__nonnull__(1, 2), __warn_unused_result__)); + +const char* rdb_skip_id(struct charset_info_st* cs, const char *str) + __attribute__((__nonnull__, __warn_unused_result__)); } // namespace myrocks