[Tarantool-patches] [PATCH v2 1/3] sql: improve "no such constraint" error message

[Roman Khabibov <roman.habibov@tarantool.org>]

Clarify the error message for better user handling. Add the name
of space where the constraint under dropping wasn't founded.

Part of #4120
---
 src/box/errcode.h            |  2 +-
 src/box/sql/alter.c          |  2 +-
 src/box/sql/build.c          | 24 +++++++++++++-----------
 test/sql-tap/alter2.test.lua |  2 +-
 test/sql/checks.result       |  2 +-
 5 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index cac8447e2..d7ec97e8c 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -221,7 +221,7 @@ struct errcode_record {
 	/*166 */_(ER_NO_SUCH_COLLATION,		"Collation '%s' does not exist") \
 	/*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") \
+	/*169 */_(ER_NO_SUCH_CONSTRAINT,	"Constraint '%s' does not exist in space '%s'") \
 	/*170 */_(ER_CONSTRAINT_EXISTS,		"%s constraint '%s' already exists in 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") \
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 973b420cf..14f6c1a97 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -121,7 +121,7 @@ sql_alter_ck_constraint_enable(struct Parse *parse)
 	int addr = sqlVdbeAddOp4Int(v, OP_Found, cursor, 0, key_reg, 2);
 	sqlVdbeAddOp4(v, OP_SetDiag, ER_NO_SUCH_CONSTRAINT, 0, 0,
 		      sqlMPrintf(db, tnt_errcode_desc(ER_NO_SUCH_CONSTRAINT),
-				 constraint_name), P4_DYNAMIC);
+				 constraint_name, tbl_name), P4_DYNAMIC);
 	sqlVdbeAddOp2(v, OP_Halt, -1, ON_CONFLICT_ACTION_ABORT);
 	sqlVdbeJumpHere(v, addr);
 
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index bc50ecbfa..d9bf8de91 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1476,21 +1476,21 @@ vdbe_emit_stat_space_clear(struct Parse *parse, const char *stat_table_name,
  * @param constraint_name Name of FK constraint to be dropped.
  *        Must be allocated on head by sqlDbMalloc().
  *        It will be freed in VDBE.
- * @param child_id Id of table which constraint belongs to.
+ * @param child_def Def of table which constraint belongs to.
  */
 static void
 vdbe_emit_fk_constraint_drop(struct Parse *parse_context, char *constraint_name,
-		    uint32_t child_id)
+			     struct space_def *child_def)
 {
 	struct Vdbe *vdbe = sqlGetVdbe(parse_context);
 	assert(vdbe != NULL);
 	int key_reg = sqlGetTempRange(parse_context, 3);
 	sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg, 0, constraint_name,
 			  P4_DYNAMIC);
-	sqlVdbeAddOp2(vdbe, OP_Integer, child_id,  key_reg + 1);
+	sqlVdbeAddOp2(vdbe, OP_Integer, child_def->id, key_reg + 1);
 	const char *error_msg =
 		tt_sprintf(tnt_errcode_desc(ER_NO_SUCH_CONSTRAINT),
-			   constraint_name);
+			   constraint_name, child_def->name);
 	if (vdbe_emit_halt_with_presence_test(parse_context,
 					      BOX_FK_CONSTRAINT_ID, 0,
 					      key_reg, 2, ER_NO_SUCH_CONSTRAINT,
@@ -1510,21 +1510,22 @@ vdbe_emit_fk_constraint_drop(struct Parse *parse_context, char *constraint_name,
  *
  * @param parser Parsing context.
  * @param ck_name Name of CK constraint to be dropped.
- * @param space_id Id of table which constraint belongs to.
+ * @param space_def Def of table which constraint belongs to.
  */
 static void
 vdbe_emit_ck_constraint_drop(struct Parse *parser, const char *ck_name,
-			     uint32_t space_id)
+			     struct space_def *space_def)
 {
 	struct Vdbe *v = sqlGetVdbe(parser);
 	struct sql *db = v->db;
 	assert(v != NULL);
 	int key_reg = sqlGetTempRange(parser, 3);
-	sqlVdbeAddOp2(v, OP_Integer, space_id,  key_reg);
+	sqlVdbeAddOp2(v, OP_Integer, space_def->id, key_reg);
 	sqlVdbeAddOp4(v, OP_String8, 0, key_reg + 1, 0,
 		      sqlDbStrDup(db, ck_name), P4_DYNAMIC);
 	const char *error_msg =
-		tt_sprintf(tnt_errcode_desc(ER_NO_SUCH_CONSTRAINT), ck_name);
+		tt_sprintf(tnt_errcode_desc(ER_NO_SUCH_CONSTRAINT), ck_name,
+			   space_def->name);
 	if (vdbe_emit_halt_with_presence_test(parser, BOX_CK_CONSTRAINT_ID, 0,
 					      key_reg, 2, ER_NO_SUCH_CONSTRAINT,
 					      error_msg, false, OP_Found) != 0)
@@ -1658,14 +1659,15 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 		char *fk_name_dup = sqlDbStrDup(v->db, child_fk->def->name);
 		if (fk_name_dup == NULL)
 			return;
-		vdbe_emit_fk_constraint_drop(parse_context, fk_name_dup, space_id);
+		vdbe_emit_fk_constraint_drop(parse_context, fk_name_dup,
+					     space->def);
 	}
 	/* Delete all CK constraints. */
 	struct ck_constraint *ck_constraint;
 	rlist_foreach_entry(ck_constraint, &space->ck_constraint, link) {
 		vdbe_emit_ck_constraint_drop(parse_context,
 					     ck_constraint->def->name,
-					     space_id);
+					     space->def);
 	}
 	/*
 	 * Drop all _space and _index entries that refer to the
@@ -2071,7 +2073,7 @@ sql_drop_foreign_key(struct Parse *parse_context)
 		return;
 	}
 	vdbe_emit_fk_constraint_drop(parse_context, constraint_name,
-				     child->def->id);
+				     child->def);
 	/*
 	 * We account changes to row count only if drop of
 	 * foreign keys take place in a separate
diff --git a/test/sql-tap/alter2.test.lua b/test/sql-tap/alter2.test.lua
index 3e21a5f40..759acc9c3 100755
--- a/test/sql-tap/alter2.test.lua
+++ b/test/sql-tap/alter2.test.lua
@@ -256,7 +256,7 @@ test:do_catchsql_test(
         ALTER TABLE child DROP CONSTRAINT fake;
     ]], {
         -- <alter2-5.2>
-        1, "Constraint FAKE does not exist"
+        1, "Constraint 'FAKE' does not exist in space 'CHILD'"
         -- </alter2-5.2>
     })
 
diff --git a/test/sql/checks.result b/test/sql/checks.result
index 4ae0b4c10..7b18e5d6b 100644
--- a/test/sql/checks.result
+++ b/test/sql/checks.result
@@ -801,7 +801,7 @@ box.execute('ALTER TABLE test ADD CONSTRAINT \"some_ck\" CHECK(a < 10);')
 box.execute("ALTER TABLE test DISABLE CHECK CONSTRAINT \"falsch\"")
 ---
 - null
-- Constraint falsch does not exist
+- Constraint 'falsch' does not exist in space 'TEST'
 ...
 box.execute("ALTER TABLE test DISABLE CHECK CONSTRAINT \"some_ck\"")
 ---
-- 
2.21.0 (Apple Git-122)