[tarantool-patches] Re: [PATCH v1 6/6] sql: make sql checks on server side

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Working with Functional Indexes patch I've found this problem here.
Believe that space trigger should not be a part of DDL logic

diff --git a/src/box/alter.cc b/src/box/alter.cc
index aa75457ac..8226d7624 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1408,33 +1408,6 @@ on_drop_space_rollback(struct trigger *trigger, void *event)
 	space_cache_replace(NULL, space);
 }
 
-/**
- * SQL-specific actions for space.
- */
-static void
-on_space_before_replace(struct trigger *trigger, void *event)
-{
-	uint32_t space_id = (uint32_t)(uintptr_t)trigger->data;
-	struct space *space = space_cache_find(space_id);
-	assert(space != NULL);
-	struct txn *txn = (struct txn *) event;
-	struct txn_stmt *stmt = txn_current_stmt(txn);
-	if (stmt == NULL)
-		return;
-	struct tuple *new_tuple = stmt->new_tuple;
-	struct tuple *old_tuple = stmt->old_tuple;
-	if (new_tuple != NULL && space->sql_checks != NULL) {
-		const char *new_tuple_raw = tuple_data(new_tuple);
-		const char *old_tuple_raw = old_tuple != NULL ?
-					    tuple_data(old_tuple) : NULL;
-		if (sql_checks_run(space->sql_checks,
-				   space->def->opts.checks_ast, space->def,
-				   old_tuple_raw, new_tuple_raw) != 0) {
-			diag_raise();
-		}
-	}
-}
-
 /**
  * Run the triggers registered on commit of a change in _space.
  */
@@ -1753,21 +1726,6 @@ on_replace_dd_space(struct trigger * /* trigger */, void *event)
 			txn_on_rollback(txn, on_rollback_view);
 			select_guard.is_active = false;
 		}
-		/* Setup SQL-actions trigger if required.  */
-		if (def->opts.checks_ast != NULL) {
-			struct trigger *sql_actions_trigger =
-				(struct trigger *)malloc(sizeof(struct trigger));
-			if (sql_actions_trigger == NULL) {
-				tnt_raise(OutOfMemory, sizeof(struct trigger),
-					  "calloc",
-					  "on_space_before_replace");
-			}
-			trigger_create(sql_actions_trigger,
-				       on_space_before_replace,
-				       (void *)(uintptr_t)space->def->id,
-				       (trigger_f0)free);
-			trigger_add(&space->before_replace, sql_actions_trigger);
-		}
 	} else if (new_tuple == NULL) { /* DELETE */
 		access_check_ddl(old_space->def->name, old_space->def->id,
 				 old_space->def->uid, SC_SPACE, PRIV_D);
diff --git a/src/box/space.c b/src/box/space.c
index 6da0e6e9a..b139a0905 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -109,6 +109,33 @@ space_fill_index_map(struct space *space)
 	}
 }
 
+/**
+ * SQL-specific actions for space.
+ */
+static void
+on_space_before_replace(struct trigger *trigger, void *event)
+{
+	uint32_t space_id = (uint32_t)(uintptr_t)trigger->data;
+	struct space *space = space_cache_find(space_id);
+	assert(space != NULL);
+	struct txn *txn = (struct txn *) event;
+	struct txn_stmt *stmt = txn_current_stmt(txn);
+	if (stmt == NULL)
+		return;
+	struct tuple *new_tuple = stmt->new_tuple;
+	struct tuple *old_tuple = stmt->old_tuple;
+	if (new_tuple != NULL && space->sql_checks != NULL) {
+		const char *new_tuple_raw = tuple_data(new_tuple);
+		const char *old_tuple_raw = old_tuple != NULL ?
+					    tuple_data(old_tuple) : NULL;
+		if (sql_checks_run(space->sql_checks,
+				   space->def->opts.checks_ast, space->def,
+				   old_tuple_raw, new_tuple_raw) != 0) {
+			diag_raise();
+		}
+	}
+}
+
 int
 space_create(struct space *space, struct engine *engine,
 	     const struct space_vtab *vtab, struct space_def *def,
@@ -172,6 +199,18 @@ space_create(struct space *space, struct engine *engine,
 		if (unlikely(checks == NULL))
 			goto fail_free_indexes;
 		space->sql_checks = checks;
+
+		struct trigger *sql_actions_trigger =
+			(struct trigger *)malloc(sizeof(struct trigger));
+		if (sql_actions_trigger == NULL) {
+			diag_set(OutOfMemory, sizeof(struct trigger),
+				"calloc", "on_space_before_replace");
+			goto fail_free_indexes;
+		}
+		trigger_create(sql_actions_trigger, on_space_before_replace,
+			       (void *)(uintptr_t)space->def->id,
+			       (trigger_f0)free);
+		trigger_add(&space->before_replace, sql_actions_trigger);
 	}
 	return 0;