[Tarantool-patches] [PATCH v2 3/3] sql: support constraint drop

Roman Khabibov roman.habibov at tarantool.org
Tue Mar 3 13:12:04 MSK 2020


Extend <ALTER TABLE> statement to drop table constraints by their
names.

Closes #4120

@TarantoolBot document
Title: Drop table constraints in SQL
Now, it is possible to drop table constraints (PRIMARY KEY,
UNIQUE, FOREIGN KEY, CHECK) using
<ALTER TABLE table_name DROP CONSTRAINT constraint_name> statement
by their names.

For example:

tarantool> box.execute([[CREATE TABLE test (
                             a INTEGER PRIMARY KEY,
                             b INTEGER,
                             CONSTRAINT cnstr CHECK (a >= 0)
                        );]])
---
- row_count: 1
...

tarantool> box.execute('ALTER TABLE test DROP CONSTRAINT cnstr;')
---
- row_count: 1
...

The same for all the other constraints.
---
 src/box/constraint_id.h      |  1 +
 src/box/sql/build.c          | 66 +++++++++++++++++++----------
 src/box/sql/parse.y          |  4 +-
 src/box/sql/parse_def.h      | 11 ++---
 src/box/sql/sqlInt.h         |  7 +++-
 test/sql/constraint.result   | 81 ++++++++++++++++++++++++++++++++++++
 test/sql/constraint.test.lua | 28 +++++++++++++
 7 files changed, 167 insertions(+), 31 deletions(-)

diff --git a/src/box/constraint_id.h b/src/box/constraint_id.h
index 21f067cdc..ed4f37426 100755
--- a/src/box/constraint_id.h
+++ b/src/box/constraint_id.h
@@ -40,6 +40,7 @@ enum constraint_type {
 	CONSTRAINT_TYPE_UNIQUE,
 	CONSTRAINT_TYPE_FK,
 	CONSTRAINT_TYPE_CK,
+	constraint_type_MAX,
 };
 
 extern const char *constraint_type_strs[];
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index e0f690fc2..6fe8fbcdf 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1507,18 +1507,18 @@ vdbe_emit_index_drop(struct Parse *parse_context, const char *name,
  *
  * @param parse_context Parsing context.
  * @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_def Def of table which constraint belongs to.
  */
 static void
-vdbe_emit_fk_constraint_drop(struct Parse *parse_context, char *constraint_name,
+vdbe_emit_fk_constraint_drop(struct Parse *parse_context,
+			     const char *constraint_name,
 			     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,
+	const char *name_copy = sqlDbStrDup(parse_context->db, constraint_name);
+	sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg, 0, name_copy,
 			  P4_DYNAMIC);
 	sqlVdbeAddOp2(vdbe, OP_Integer, child_def->id, key_reg + 1);
 	const char *error_msg =
@@ -1528,10 +1528,8 @@ vdbe_emit_fk_constraint_drop(struct Parse *parse_context, char *constraint_name,
 					      BOX_FK_CONSTRAINT_ID, 0,
 					      key_reg, 2, ER_NO_SUCH_CONSTRAINT,
 					      error_msg, false,
-					      OP_Found) != 0) {
-		sqlDbFree(parse_context->db, constraint_name);
+					      OP_Found) != 0)
 		return;
-	}
 	sqlVdbeAddOp3(vdbe, OP_MakeRecord, key_reg, 2, key_reg + 2);
 	sqlVdbeAddOp2(vdbe, OP_SDelete, BOX_FK_CONSTRAINT_ID, key_reg + 2);
 	VdbeComment((vdbe, "Delete FK constraint %s", constraint_name));
@@ -1688,11 +1686,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 	struct fk_constraint *child_fk;
 	rlist_foreach_entry(child_fk, &space->child_fk_constraint,
 			    in_child_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,
+		vdbe_emit_fk_constraint_drop(parse_context, child_fk->def->name,
 					     space->def);
 	}
 	/* Delete all CK constraints. */
@@ -2085,28 +2079,38 @@ fk_constraint_change_defer_mode(struct Parse *parse_context, bool is_deferred)
 		is_deferred;
 }
 
+/**
+ * Emit code to drop the entry from _index or _ck_contstraint or
+ * _fk_constraint space corresponding with the constraint type.
+ */
 void
-sql_drop_foreign_key(struct Parse *parse_context)
+sql_drop_constraint(struct Parse *parse_context)
 {
-	struct drop_entity_def *drop_def = &parse_context->drop_fk_def.base;
-	assert(drop_def->base.entity_type == ENTITY_TYPE_FK);
+	struct drop_entity_def *drop_def =
+		&parse_context->drop_constraint_def.base;
+	assert(drop_def->base.entity_type == ENTITY_TYPE_CONSTRAINT);
 	assert(drop_def->base.alter_action == ALTER_ACTION_DROP);
 	const char *table_name = drop_def->base.entity_name->a[0].zName;
 	assert(table_name != NULL);
-	struct space *child = space_by_name(table_name);
-	if (child == NULL) {
+	struct space *space = space_by_name(table_name);
+	if (space == NULL) {
 		diag_set(ClientError, ER_NO_SUCH_SPACE, table_name);
 		parse_context->is_aborted = true;
 		return;
 	}
-	char *constraint_name =
-		sql_name_from_token(parse_context->db, &drop_def->name);
-	if (constraint_name == NULL) {
+	char *name = sql_normalized_name_region_new(&parse_context->region,
+						    drop_def->name.z,
+						    drop_def->name.n);
+	if (name == NULL) {
+		parse_context->is_aborted = true;
+		return;
+	}
+	struct constraint_id *id = space_find_constraint_id(space, name);
+	if (id == NULL) {
+		diag_set(ClientError, ER_NO_SUCH_CONSTRAINT, name, table_name);
 		parse_context->is_aborted = true;
 		return;
 	}
-	vdbe_emit_fk_constraint_drop(parse_context, constraint_name,
-				     child->def);
 	/*
 	 * We account changes to row count only if drop of
 	 * foreign keys take place in a separate
@@ -2114,6 +2118,24 @@ sql_drop_foreign_key(struct Parse *parse_context)
 	 * DROP TABLE always returns 1 (one) as a row count.
 	 */
 	struct Vdbe *v = sqlGetVdbe(parse_context);
+	assert(v != NULL);
+	assert(id->type < constraint_type_MAX);
+	switch (id->type) {
+	case CONSTRAINT_TYPE_PK:
+	case CONSTRAINT_TYPE_UNIQUE: {
+		vdbe_emit_index_drop(parse_context, name, space->def,
+				     ER_NO_SUCH_CONSTRAINT, false);
+		break;
+	}
+	case CONSTRAINT_TYPE_FK:
+		vdbe_emit_fk_constraint_drop(parse_context, name, space->def);
+		break;
+	case CONSTRAINT_TYPE_CK:
+		vdbe_emit_ck_constraint_drop(parse_context, name, space->def);
+		break;
+	default:
+		unreachable();
+	}
 	sqlVdbeCountChanges(v);
 	sqlVdbeChangeP5(v, OPFLAG_NCHANGE);
 }
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index cfe1c0012..1a0e89703 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -1763,9 +1763,9 @@ cmd ::= alter_table_start(A) RENAME TO nm(N). {
 }
 
 cmd ::= ALTER TABLE fullname(X) DROP CONSTRAINT nm(Z). {
-  drop_fk_def_init(&pParse->drop_fk_def, X, &Z, false);
+  drop_constraint_def_init(&pParse->drop_constraint_def, X, &Z, false);
   pParse->initiateTTrans = true;
-  sql_drop_foreign_key(pParse);
+  sql_drop_constraint(pParse);
 }
 
 cmd ::= alter_table_start(A) enable(E) CHECK CONSTRAINT nm(Z). {
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index 2f433e4c0..cb0ecd2fc 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -265,7 +265,7 @@ struct drop_trigger_def {
 	struct drop_entity_def base;
 };
 
-struct drop_fk_def {
+struct drop_constraint_def {
 	struct drop_entity_def base;
 };
 
@@ -408,11 +408,12 @@ drop_trigger_def_init(struct drop_trigger_def *drop_trigger_def,
 }
 
 static inline void
-drop_fk_def_init(struct drop_fk_def *drop_fk_def, struct SrcList *parent_name,
-		 struct Token *name, bool if_exist)
+drop_constraint_def_init(struct drop_constraint_def *drop_constraint_def,
+			 struct SrcList *parent_name, struct Token *name,
+			 bool if_exist)
 {
-	drop_entity_def_init(&drop_fk_def->base, parent_name, name, if_exist,
-			     ENTITY_TYPE_FK);
+	drop_entity_def_init(&drop_constraint_def->base, parent_name, name,
+			     if_exist, ENTITY_TYPE_CONSTRAINT);
 }
 
 static inline void
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index d1fcf4761..1579cc92e 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2237,7 +2237,7 @@ struct Parse {
 		struct create_trigger_def create_trigger_def;
 		struct create_view_def create_view_def;
 		struct rename_entity_def rename_entity_def;
-		struct drop_fk_def drop_fk_def;
+		struct drop_constraint_def drop_constraint_def;
 		struct drop_index_def drop_index_def;
 		struct drop_table_def drop_table_def;
 		struct drop_trigger_def drop_trigger_def;
@@ -3741,13 +3741,16 @@ void
 sql_create_foreign_key(struct Parse *parse_context);
 
 /**
+ * Emit code to drop the entry from _index or _ck_contstraint or
+ * _fk_constraint space corresponding with the constraint type.
+ *
  * Function called from parser to handle
  * <ALTER TABLE table DROP CONSTRAINT constraint> SQL statement.
  *
  * @param parse_context Parsing context.
  */
 void
-sql_drop_foreign_key(struct Parse *parse_context);
+sql_drop_constraint(struct Parse *parse_context);
 
 /**
  * Now our SQL implementation can't operate on spaces which
diff --git a/test/sql/constraint.result b/test/sql/constraint.result
index 1585c2327..bcdc46a1c 100644
--- a/test/sql/constraint.result
+++ b/test/sql/constraint.result
@@ -291,6 +291,87 @@ box.execute('CREATE UNIQUE INDEX d ON t2(i);')
  | - Index 'D' already exists in space 'T2'
  | ...
 
+--
+-- gh-4120: Ensure that <ALTER TABLE DROP CONSTRAINT> works
+-- correctly for each type of constraint.
+--
+-- Drop non-constraint object (non-unique index).
+box.execute('CREATE INDEX non_constraint ON t2(i);')
+ | ---
+ | - row_count: 1
+ | ...
+box.execute('ALTER TABLE t2 DROP CONSTRAINT non_constraint;')
+ | ---
+ | - null
+ | - Constraint 'NON_CONSTRAINT' does not exist in space 'T2'
+ | ...
+-- Drop UNIQUE constraint.
+box.space.T2.index.E ~= nil
+ | ---
+ | - true
+ | ...
+box.execute('ALTER TABLE t2 DROP CONSTRAINT e;')
+ | ---
+ | - row_count: 1
+ | ...
+box.space.T2.index.E == nil
+ | ---
+ | - true
+ | ...
+-- Drop PRIMARY KEY constraint named "C".
+box.execute('DROP INDEX non_constraint ON t2;')
+ | ---
+ | - row_count: 1
+ | ...
+box.execute('DROP INDEX d ON t2;')
+ | ---
+ | - row_count: 1
+ | ...
+box.space.T2.index.C ~= nil
+ | ---
+ | - true
+ | ...
+box.execute('ALTER TABLE t2 DROP CONSTRAINT c;')
+ | ---
+ | - row_count: 1
+ | ...
+box.space.T2.index.C == nil
+ | ---
+ | - true
+ | ...
+-- Drop CHECK constraint.
+box.execute('ALTER TABLE t2 ADD CONSTRAINT ck_constraint CHECK(i > 0);')
+ | ---
+ | - row_count: 1
+ | ...
+box.space.T2.ck_constraint.CK_CONSTRAINT ~= nil
+ | ---
+ | - true
+ | ...
+box.execute('ALTER TABLE t2 DROP CONSTRAINT ck_constraint;')
+ | ---
+ | - row_count: 1
+ | ...
+box.space.T2.ck_constraint.CK_CONSTRAINT == nil
+ | ---
+ | - true
+ | ...
+-- Drop FOREIGN KEY constraint.
+box.execute('ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY(i) REFERENCES t1(i);')
+ | ---
+ | - row_count: 1
+ | ...
+box.execute('ALTER TABLE t2 DROP CONSTRAINT fk;')
+ | ---
+ | - row_count: 1
+ | ...
+-- Drop non-existing constraint.
+box.execute('ALTER TABLE t2 DROP CONSTRAINT non_existing_constraint;')
+ | ---
+ | - null
+ | - Constraint 'NON_EXISTING_CONSTRAINT' does not exist in space 'T2'
+ | ...
+
 --
 -- Cleanup.
 --
diff --git a/test/sql/constraint.test.lua b/test/sql/constraint.test.lua
index 163f4309c..a9fa4ccc9 100755
--- a/test/sql/constraint.test.lua
+++ b/test/sql/constraint.test.lua
@@ -131,6 +131,34 @@ box.execute('CREATE UNIQUE INDEX e ON t2(i);')
 -- uniqueness, index names should be unique in a space.
 box.execute('CREATE UNIQUE INDEX d ON t2(i);')
 
+--
+-- gh-4120: Ensure that <ALTER TABLE DROP CONSTRAINT> works
+-- correctly for each type of constraint.
+--
+-- Drop non-constraint object (non-unique index).
+box.execute('CREATE INDEX non_constraint ON t2(i);')
+box.execute('ALTER TABLE t2 DROP CONSTRAINT non_constraint;')
+-- Drop UNIQUE constraint.
+box.space.T2.index.E ~= nil
+box.execute('ALTER TABLE t2 DROP CONSTRAINT e;')
+box.space.T2.index.E == nil
+-- Drop PRIMARY KEY constraint named "C".
+box.execute('DROP INDEX non_constraint ON t2;')
+box.execute('DROP INDEX d ON t2;')
+box.space.T2.index.C ~= nil
+box.execute('ALTER TABLE t2 DROP CONSTRAINT c;')
+box.space.T2.index.C == nil
+-- Drop CHECK constraint.
+box.execute('ALTER TABLE t2 ADD CONSTRAINT ck_constraint CHECK(i > 0);')
+box.space.T2.ck_constraint.CK_CONSTRAINT ~= nil
+box.execute('ALTER TABLE t2 DROP CONSTRAINT ck_constraint;')
+box.space.T2.ck_constraint.CK_CONSTRAINT == nil
+-- Drop FOREIGN KEY constraint.
+box.execute('ALTER TABLE t2 ADD CONSTRAINT fk FOREIGN KEY(i) REFERENCES t1(i);')
+box.execute('ALTER TABLE t2 DROP CONSTRAINT fk;')
+-- Drop non-existing constraint.
+box.execute('ALTER TABLE t2 DROP CONSTRAINT non_existing_constraint;')
+
 --
 -- Cleanup.
 --
-- 
2.21.0 (Apple Git-122)



More information about the Tarantool-patches mailing list