From: Roman Khabibov <roman.habibov@tarantool.org> To: tarantool-patches@freelists.org Cc: korablev@tarantool.org Subject: [tarantool-patches] [PATCH] sql: check constraint name duplication Date: Tue, 23 Jul 2019 14:28:03 +0300 [thread overview] Message-ID: <20190723112803.37978-1-roman.habibov@tarantool.org> (raw) Check constraint name duplication in a single <CREATE TABLE> statement. Closes #3503 --- Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3503-constr Issue: https://github.com/tarantool/tarantool/issues/3503 src/box/sql/build.c | 68 ++++++++++++++- test/sql-tap/table.test.lua | 164 +++++++++++++++++++++++++++++++++++- test/sql/checks.result | 6 +- test/sql/checks.test.lua | 1 - 4 files changed, 232 insertions(+), 7 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 396de63fd..120df9f8c 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -228,6 +228,58 @@ sql_space_column_is_in_pk(struct space *space, uint32_t column) return key_def_find_by_fieldno(key_def, column) != NULL; return false; } +/** + * Check if create_table_def.new_space in @a parser already has a + * PRIMARY KEY/UNIQUE/CHECK/FOREIGN KEY constraint with @a name. + * + * The routine is used during CREATE TABLE only. + * + * @param parser Parser context. + * @param name Name to check. + * + * @retval true Has constraint with @a name. + * @retval false Has not. + */ +static bool +sql_has_constr_with_name(struct Parse *parser, const char *name) +{ + /* Replace with sql_space_index_by_name() after #3788. */ + struct space *space = parser->create_table_def.new_space; + assert(name != NULL); + const char *pk_name = tt_sprintf("pk_%s", name); + const char *un_name = tt_sprintf("unique_%s", name); + for (uint32_t i = 0; i < space->index_count; ++i) { + struct index *idx = space->index[i]; + if (memcmp(pk_name, idx->def->name, strlen(pk_name)) == 0 || + memcmp(un_name, idx->def->name, strlen(un_name)) == 0) + goto has; + } + + struct ck_constraint_parse *ck_constr_p = NULL; + uint32_t name_len = strlen(name); + rlist_foreach_entry(ck_constr_p, &parser->create_table_def.new_check, + link) { + if (strlen(ck_constr_p->ck_def->name) == name_len && + memcmp(ck_constr_p->ck_def->name, name, name_len) == 0) + goto has; + } + struct fk_constraint_parse *fk_constr_p = NULL; + rlist_foreach_entry(fk_constr_p, &parser->create_table_def.new_fkey, + link) { + /* Skip current fk. */ + if (fk_constr_p->fk_def == NULL) + continue; + if (strlen(fk_constr_p->fk_def->name) == name_len && + memcmp(fk_constr_p->fk_def->name, name, name_len) == 0) + goto has; + } + return false; +has: + diag_set(ClientError, ER_CREATE_CK_CONSTRAINT, space->def->name, + tt_sprintf("Constraints can\'t have the same name: %s", name)); + parser->is_aborted = true; + return true; +} /* * This routine is used to check if the UTF-8 string zName is a legal @@ -602,6 +654,8 @@ sqlAddPrimaryKey(struct Parse *pParse) sql_create_index(pParse); if (db->mallocFailed) goto primary_key_exit; + if (pParse->is_aborted) + goto primary_key_exit; } else if (pParse->create_table_def.has_autoinc) { diag_set(ClientError, ER_CREATE_SPACE, space->def->name, "AUTOINCREMENT is only allowed on an INTEGER PRIMARY "\ @@ -698,6 +752,10 @@ sql_create_check_contraint(struct Parse *parser) parser->is_aborted = true; return; } + if (is_alter == false) { + if (sql_has_constr_with_name(parser, name) == true) + return; + } } else { assert(! is_alter); uint32_t ck_idx = ++parser->create_table_def.check_count; @@ -1948,8 +2006,13 @@ sql_create_foreign_key(struct Parse *parse_context) } else { constraint_name = sql_name_from_token(db, &create_def->name); - if (constraint_name == NULL) + if (constraint_name == NULL) { parse_context->is_aborted = true; + goto tnt_error; + } + if (sql_has_constr_with_name(parse_context, + constraint_name) == true) + goto tnt_error; } } else { constraint_name = sql_name_from_token(db, &create_def->name); @@ -2454,6 +2517,9 @@ sql_create_index(struct Parse *parse) { parse->is_aborted = true; goto exit_create_index; } + if (sql_has_constr_with_name(parse, + constraint_name) == true) + goto exit_create_index; } /* diff --git a/test/sql-tap/table.test.lua b/test/sql-tap/table.test.lua index a539f5222..4de8eff5c 100755 --- a/test/sql-tap/table.test.lua +++ b/test/sql-tap/table.test.lua @@ -1,6 +1,6 @@ #!/usr/bin/env tarantool test = require("sqltester") -test:plan(79) +test:plan(95) --!./tcltestrunner.lua -- 2001 September 15 @@ -1472,4 +1472,166 @@ test:do_catchsql_test( -- </table-24.2> }) +-- +-- gh-3503 Check constraint name for duplicate. +-- +test:do_execsql_test( + "table-25.1", + [[ + CREATE TABLE t1 (i INT PRIMARY KEY); + ]], { + -- <table-25.1> + -- </table-25.1> + }) + +test:do_catchsql_test( + "table-25.2", + [[ + CREATE TABLE t2 (i INT, CONSTRAINT c UNIQUE (i), CONSTRAINT c PRIMARY KEY (i)); + ]], { + -- <table-25.2> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.2> + }) + +test:do_catchsql_test( + "table-25.3", + [[ + CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0), CONSTRAINT c PRIMARY KEY (i)); + ]], { + -- <table-25.3> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.3> + }) + +test:do_catchsql_test( + "table-25.4", + [[ + CREATE TABLE t2 (i INT, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c PRIMARY KEY (i)); + ]], { + -- <table-25.4> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.4> + }) + +test:do_catchsql_test( + "table-25.5", + [[ + CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY, CONSTRAINT c UNIQUE (i)); + ]], { + -- <table-25.5> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.5> + }) + +test:do_catchsql_test( + "table-25.6", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c UNIQUE (i), CONSTRAINT c UNIQUE (i)); + ]], { + -- <table-25.6> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.6> + }) + +test:do_catchsql_test( + "table-25.7", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c UNIQUE (i)); + ]], { + -- <table-25.7> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.7> + }) + +test:do_catchsql_test( + "table-25.8", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c UNIQUE (i)); + ]], { + -- <table-25.8> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.8> + }) + +test:do_catchsql_test( + "table-25.9", + [[ + CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY, CONSTRAINT c CHECK (i > 0)); + ]], { + -- <table-25.9> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.9> + }) + +test:do_catchsql_test( + "table-25.10", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c UNIQUE (i), CONSTRAINT c CHECK (i > 0)); + ]], { + -- <table-25.10> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.10> + }) + +test:do_catchsql_test( + "table-25.11", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c CHECK (i < 0)); + ]], { + -- <table-25.11> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.11> + }) + +test:do_catchsql_test( + "table-25.12", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c CHECK (i > 0)); + ]], { + -- <table-25.12> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.12> + }) + +test:do_catchsql_test( + "table-25.13", + [[ + CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i)); + ]], { + -- <table-25.13> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.13> + }) + +test:do_catchsql_test( + "table-25.14", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c UNIQUE (i), CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i)); + ]], { + -- <table-25.14> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.14> + }) + +test:do_catchsql_test( + "table-25.15", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i)); + ]], { + -- <table-25.15> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.15> + }) + +test:do_catchsql_test( + "table-25.16", + [[ + CREATE TABLE t2 (i INT PRIMARY KEY CONSTRAINT c REFERENCES t1(i), CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i)); + ]], { + -- <table-25.16> + 1,"Failed to create check constraint 'T2': Constraints can't have the same name: C" + -- </table-25.16> + }) + test:finish_test() diff --git a/test/sql/checks.result b/test/sql/checks.result index bdcf50731..f155957dd 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -249,10 +249,8 @@ s:drop() ... box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))") --- -- error: Constraint CK1 already exists -... -box.space.T2:drop() ---- +- error: 'Failed to create check constraint ''T2'': Constraints can''t have the same + name: CK1' ... box.space._ck_constraint:select() --- diff --git a/test/sql/checks.test.lua b/test/sql/checks.test.lua index 50cb13c54..dbe185771 100644 --- a/test/sql/checks.test.lua +++ b/test/sql/checks.test.lua @@ -86,7 +86,6 @@ box.execute("INSERT INTO \"test\" VALUES(11, 11);") box.execute("INSERT INTO \"test\" VALUES(12, 11);") s:drop() box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))") -box.space.T2:drop() box.space._ck_constraint:select() -- -- 2.20.1 (Apple Git-117)
next reply other threads:[~2019-07-23 11:28 UTC|newest] Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-23 11:28 Roman Khabibov [this message] 2019-08-08 22:42 ` [tarantool-patches] " n.pettik
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190723112803.37978-1-roman.habibov@tarantool.org \ --to=roman.habibov@tarantool.org \ --cc=korablev@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='Re: [tarantool-patches] [PATCH] sql: check constraint name duplication' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox