[tarantool-patches] [PATCH] sql: check constraint name duplication
Roman Khabibov
roman.habibov at tarantool.org
Tue Jul 23 14:28:03 MSK 2019
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)
More information about the Tarantool-patches
mailing list