From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6248446971A for ; Wed, 4 Dec 2019 19:23:14 +0300 (MSK) From: Roman Khabibov Date: Wed, 4 Dec 2019 19:23:11 +0300 Message-Id: <0218889f41f954a10bbe36c6d0c43f32fa7dd4b4.1575468493.git.roman.habibov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v2 3/3] box: let only box handle constraint dup errors List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org Improve error message about constraint name duplicate and remove it from sql. Follow up #3503 --- src/box/alter.cc | 59 +++++++++++++++++++++++++++--------- src/box/errcode.h | 2 +- src/box/sql/build.c | 20 ------------ test/sql-tap/alter2.test.lua | 4 +-- test/sql/checks.result | 2 +- test/sql/clear.result | 6 ++-- test/sql/constraint.result | 24 +++++++-------- 7 files changed, 64 insertions(+), 53 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index c12279e65..e60c2827b 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -2449,6 +2449,47 @@ index_is_used_by_fk_constraint(struct rlist *fk_list, uint32_t iid) return false; } +/** + * Just return string with constraint type to print it in error + * message. + */ +static inline const char * +cosntraint_type_str(struct constraint_def *def) { + switch (def->type) { + case CONSTRAINT_TYPE_PK: + return "PRIMARY KEY"; + case CONSTRAINT_TYPE_UNIQUE: + return "UNIQUE"; + case CONSTRAINT_TYPE_FK: + return "FOREIGN KEY"; + case CONSTRAINT_TYPE_CK: + return "CHECK"; + } +} + +/** + * Check if constraint with @a name exists within @a space. Call + * diag_set() if it is so. + * + * @param space Space to find constraint within. + * @param name Constraint name. + * + * @retval true Constraint exists. + * @retval false Doesn't exist. + */ +static inline bool +space_constraint_exists(struct space *space, const char *name) +{ + struct constraint_def *def = + space_constraint_def_by_name(space, name); + if (def != NULL) { + diag_set(ClientError, ER_CONSTRAINT_EXISTS, + cosntraint_type_str(def), name, space_name(space)); + return true; + } + return false; +} + /** * Put the node of an unique index to the constraint hash table of * @a space. @@ -2462,10 +2503,8 @@ create_index_as_constraint(struct alter_space *alter, { assert(def->opts.is_unique); struct space *space = alter->old_space; - if (space_constraint_def_by_name(space, def->name) != NULL) { - diag_set(ClientError, ER_CONSTRAINT_EXISTS, def->name); + if (space_constraint_exists(space, def->name)) return -1; - } struct constraint_def *constr_def = constraint_def_new(space->def->id, def->iid == 0 ? CONSTRAINT_TYPE_PK : CONSTRAINT_TYPE_UNIQUE, @@ -5348,13 +5387,8 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event) fk->def = fk_def; fk->index_id = fk_index->def->iid; if (old_tuple == NULL) { - if (space_constraint_def_by_name(child_space, - fk_def->name) - != NULL) { - diag_set(ClientError, ER_CONSTRAINT_EXISTS, - fk_def->name); + if (space_constraint_exists(child_space, fk_def->name)) return -1; - } rlist_add_entry(&child_space->child_fk_constraint, fk, in_child_space); rlist_add_entry(&parent_space->parent_fk_constraint, @@ -5663,13 +5697,10 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event) auto ck_guard = make_scoped_guard([=] { ck_constraint_delete(new_ck_constraint); }); - if (old_ck_constraint != NULL) { + if (old_ck_constraint != NULL) rlist_del_entry(old_ck_constraint, link); - } else if (space_constraint_def_by_name(space, name) != NULL) { - diag_set(ClientError, ER_CONSTRAINT_EXISTS, - name); + else if (space_constraint_exists(space, name)) return -1; - } if (space_add_ck_constraint(space, new_ck_constraint) != 0) return -1; ck_guard.is_active = false; diff --git a/src/box/errcode.h b/src/box/errcode.h index c660b1c70..2b922a363 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -222,7 +222,7 @@ struct errcode_record { /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \ /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \ /*169 */_(ER_NO_SUCH_CONSTRAINT, "Constraint %s does not exist") \ - /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s already exists") \ + /*170 */_(ER_CONSTRAINT_EXISTS, "%s constraint %s already exists within space %s") \ /*171 */_(ER_SQL_TYPE_MISMATCH, "Type mismatch: can not convert %s to %s") \ /*172 */_(ER_ROWID_OVERFLOW, "Rowid is overflowed: too many entries in ephemeral space") \ /*173 */_(ER_DROP_COLLATION, "Can't drop collation %s : %s") \ diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 51cd7ce63..ce4629aed 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -994,14 +994,6 @@ vdbe_emit_ck_constraint_create(struct Parse *parser, sqlVdbeAddOp2(v, OP_Bool, true, ck_constraint_reg + 5); sqlVdbeAddOp3(v, OP_MakeRecord, ck_constraint_reg, 6, ck_constraint_reg + 6); - const char *error_msg = - tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS), - ck_def->name); - if (vdbe_emit_halt_with_presence_test(parser, BOX_CK_CONSTRAINT_ID, 0, - ck_constraint_reg, 2, - ER_CONSTRAINT_EXISTS, error_msg, - false, OP_NoConflict) != 0) - return; sqlVdbeAddOp2(v, OP_SInsert, BOX_CK_CONSTRAINT_ID, ck_constraint_reg + 6); VdbeComment((v, "Create CK constraint %s", ck_def->name)); @@ -1053,18 +1045,6 @@ vdbe_emit_fk_constraint_create(struct Parse *parse_context, sqlVdbeAddOp2(vdbe, OP_Integer, fk->parent_id, constr_tuple_reg + 2); } - /* - * Lets check that constraint with this name hasn't - * been created before. - */ - const char *error_msg = - tt_sprintf(tnt_errcode_desc(ER_CONSTRAINT_EXISTS), name_copy); - if (vdbe_emit_halt_with_presence_test(parse_context, - BOX_FK_CONSTRAINT_ID, 0, - constr_tuple_reg, 2, - ER_CONSTRAINT_EXISTS, error_msg, - false, OP_NoConflict) != 0) - return; sqlVdbeAddOp2(vdbe, OP_Bool, fk->is_deferred, constr_tuple_reg + 3); sqlVdbeAddOp4(vdbe, OP_String8, 0, constr_tuple_reg + 4, 0, fk_constraint_match_strs[fk->match], P4_STATIC); diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua index e0bd60727..e4603e579 100755 --- a/test/sql-tap/alter2.test.lua +++ b/test/sql-tap/alter2.test.lua @@ -246,7 +246,7 @@ test:do_catchsql_test( ALTER TABLE child ADD CONSTRAINT fk FOREIGN KEY (a) REFERENCES child; ]], { -- - 1, "Constraint FK already exists" + 1, "Duplicate key exists in unique index 'primary' in space '_fk_constraint'" -- }) @@ -278,7 +278,7 @@ test:do_catchsql_test( "alter2-6.2", [[ ALTER TABLE t1 ADD CONSTRAINT ck CHECK(id > 0); - ]], { 1, "Constraint CK already exists" }) + ]], { 1, "Duplicate key exists in unique index 'primary' in space '_ck_constraint'" }) -- Make sure that CHECK constraint can be created only on empty space. -- diff --git a/test/sql/checks.result b/test/sql/checks.result index a952b2b70..b9e9dfbcd 100644 --- a/test/sql/checks.result +++ b/test/sql/checks.result @@ -260,7 +260,7 @@ s:drop() box.execute("CREATE TABLE T2(ID INT PRIMARY KEY, CONSTRAINT CK1 CHECK(ID > 0), CONSTRAINT CK1 CHECK(ID < 0))") --- - null -- Constraint CK1 already exists +- Duplicate key exists in unique index 'primary' in space '_ck_constraint' ... box.space.T2 --- diff --git a/test/sql/clear.result b/test/sql/clear.result index afa6520e8..816612da6 100644 --- a/test/sql/clear.result +++ b/test/sql/clear.result @@ -177,7 +177,7 @@ box.execute("DROP TABLE zoobar") box.execute("CREATE TABLE t1(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 CHECK(id < 0));") --- - null -- Constraint CK1 already exists +- Duplicate key exists in unique index 'primary' in space '_ck_constraint' ... box.space.t1 --- @@ -190,7 +190,7 @@ box.space._ck_constraint:select() box.execute("CREATE TABLE t2(id INT PRIMARY KEY, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t2);") --- - null -- Constraint FK1 already exists +- Duplicate key exists in unique index 'primary' in space '_fk_constraint' ... box.space.t2 --- @@ -207,7 +207,7 @@ box.space._fk_constraint:select() box.execute("CREATE TABLE t3(id INT PRIMARY KEY, CONSTRAINT ck1 CHECK(id > 0), CONSTRAINT ck1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT fk1 FOREIGN KEY(id) REFERENCES t3, CONSTRAINT ck1 CHECK(id < 0));") --- - null -- Constraint CK1 already exists +- FOREIGN KEY constraint CK1 already exists within space T3 ... box.space.t1 --- diff --git a/test/sql/constraint.result b/test/sql/constraint.result index ba262182b..ae2d5b915 100644 --- a/test/sql/constraint.result +++ b/test/sql/constraint.result @@ -26,48 +26,48 @@ box.execute([[CREATE TABLE t2 (i INT, CONSTRAINT c CHECK (i > 0), CONSTRAINT c PRIMARY KEY (i));]]); | --- | - null - | - Constraint C already exists + | - PRIMARY KEY constraint C already exists within space T2 | ... box.execute([[CREATE TABLE t2 (i INT, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c PRIMARY KEY (i));]]); | --- | - null - | - Constraint C already exists + | - PRIMARY KEY constraint C already exists within space T2 | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c UNIQUE (i));]]); | --- | - null - | - Constraint C already exists + | - UNIQUE constraint C already exists within space T2 | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c UNIQUE (i));]]); | --- | - null - | - Constraint C already exists + | - UNIQUE constraint C already exists within space T2 | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c CHECK (i > 0), CONSTRAINT c CHECK (i < 0));]]); | --- | - null - | - Constraint C already exists + | - Duplicate key exists in unique index 'primary' in space '_ck_constraint' | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY, CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i), CONSTRAINT c CHECK (i > 0));]]); | --- | - null - | - Constraint C already exists + | - FOREIGN KEY constraint C already exists within space T2 | ... box.execute([[CREATE TABLE t2 (i INT PRIMARY KEY CONSTRAINT c REFERENCES t1(i), CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i))]]); | --- | - null - | - Constraint C already exists + | - Duplicate key exists in unique index 'primary' in space '_fk_constraint' | ... -- @@ -81,7 +81,7 @@ box.execute('CREATE TABLE t2 (i INT CONSTRAINT c PRIMARY KEY);'); box.execute('ALTER TABLE t2 ADD CONSTRAINT c CHECK(i > 0);'); | --- | - null - | - Constraint C already exists + | - PRIMARY KEY constraint C already exists within space T2 | ... box.execute('ALTER TABLE t2 ADD CONSTRAINT c UNIQUE(i);'); | --- @@ -91,7 +91,7 @@ box.execute('ALTER TABLE t2 ADD CONSTRAINT c UNIQUE(i);'); box.execute('ALTER TABLE t2 ADD CONSTRAINT c FOREIGN KEY(i) REFERENCES t1(i);'); | --- | - null - | - Constraint C already exists + | - PRIMARY KEY constraint C already exists within space T2 | ... -- @@ -150,7 +150,7 @@ box.space.T2.index.D:alter({name = 'E'}); box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);'); | --- | - null - | - Constraint E already exists + | - UNIQUE constraint E already exists within space T2 | ... -- @@ -166,7 +166,7 @@ box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);'); | ... box.space.T2.index.E:alter({unique = true}); | --- - | - error: Constraint E already exists + | - error: CHECK constraint E already exists within space T2 | ... box.space.T2.ck_constraint.E:drop(); | --- @@ -177,7 +177,7 @@ box.space.T2.index.E:alter({unique = true}); box.execute('ALTER TABLE t2 ADD CONSTRAINT e CHECK(i > 0);'); | --- | - null - | - Constraint E already exists + | - UNIQUE constraint E already exists within space T2 | ... -- Alter name and uniqueness of an unique index simultaneously. -- 2.21.0 (Apple Git-122)