[Tarantool-patches] [PATCH v1 5/9] sql: use rlist to organize triggers in a list

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Using a rlist structure member to organize structures in a list
is typical practice in the Tarantool core; and reduces costs for
supporting and extending of an existent code.

With this refactoring using an universal trigger structure in
further patches would be simpler.

Needed for #4343
---
 src/box/alter.cc            |  4 +--
 src/box/space.c             |  3 +-
 src/box/space.h             |  2 +-
 src/box/sql.h               | 47 +++++++++++++++++++++++++++++
 src/box/sql/build.c         |  6 ++--
 src/box/sql/delete.c        |  7 +++--
 src/box/sql/fk_constraint.c |  1 +
 src/box/sql/insert.c        | 31 +++++++++----------
 src/box/sql/sqlInt.h        | 59 +++++--------------------------------
 src/box/sql/trigger.c       | 54 ++++++++++++++++-----------------
 src/box/sql/update.c        | 24 +++++++--------
 src/box/sql/vdbe.c          | 16 ++++++----
 12 files changed, 130 insertions(+), 124 deletions(-)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 4bc2e6e6b..84f95abc1 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -606,9 +606,7 @@ space_swap_triggers(struct space *new_space, struct space *old_space)
 	rlist_swap(&new_space->before_replace, &old_space->before_replace);
 	rlist_swap(&new_space->on_replace, &old_space->on_replace);
 	/** Swap SQL Triggers pointer. */
-	struct sql_trigger *new_value = new_space->sql_triggers;
-	new_space->sql_triggers = old_space->sql_triggers;
-	old_space->sql_triggers = new_value;
+	rlist_swap(&new_space->trigger_list, &old_space->trigger_list);
 }
 
 /** The same as for triggers - swap lists of FK constraints. */
diff --git a/src/box/space.c b/src/box/space.c
index f8a184c4e..97335e284 100644
--- a/src/box/space.c
+++ b/src/box/space.c
@@ -177,6 +177,7 @@ space_create(struct space *space, struct engine *engine,
 	rlist_create(&space->parent_fk_constraint);
 	rlist_create(&space->child_fk_constraint);
 	rlist_create(&space->ck_constraint);
+	rlist_create(&space->trigger_list);
 
 	/*
 	 * Check if there are unique indexes that are contained
@@ -261,7 +262,7 @@ space_delete(struct space *space)
 	 * SQL Triggers should be deleted with
 	 * on_replace_dd_trigger on deletion from _trigger.
 	 */
-	assert(space->sql_triggers == NULL);
+	assert(rlist_empty(&space->trigger_list));
 	assert(rlist_empty(&space->parent_fk_constraint));
 	assert(rlist_empty(&space->child_fk_constraint));
 	assert(rlist_empty(&space->ck_constraint));
diff --git a/src/box/space.h b/src/box/space.h
index 8064a046d..cc5ccbd3c 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -163,7 +163,7 @@ struct space {
 	/** Triggers fired after space_replace() -- see txn_commit_stmt(). */
 	struct rlist on_replace;
 	/** SQL Trigger list. */
-	struct sql_trigger *sql_triggers;
+	struct rlist trigger_list;
 	/**
 	 * The number of *enabled* indexes in the space.
 	 *
diff --git a/src/box/sql.h b/src/box/sql.h
index 1f289ad97..09d2b7d7c 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -34,6 +34,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include "box/trigger_def.h"
+#include "small/rlist.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -421,6 +422,52 @@ void
 vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref,
 			     struct tuple *tuple);
 
+/**
+ * Each trigger present in the database schema is stored as an
+ * instance of struct sql_trigger.
+ * Pointers to instances of struct sql_trigger are stored in a
+ * linked list, using the next member of struct sql_trigger. A
+ * pointer to the first element of the linked list is stored as
+ * trigger_list member of the associated space.
+ *
+ * The "step_list" member points to the first element of a linked
+ * list containing the SQL statements specified as the trigger
+ * program.
+ */
+struct sql_trigger {
+	/** The name of the trigger. */
+	char *zName;
+	/** The ID of space the trigger refers to. */
+	uint32_t space_id;
+	/**
+	 * The trigger event. This is the type of operation
+	 * on the associated space for which the trigger
+	 * activates. The value is `INSERT` (a row was inserted),
+	 * `DELETE` (a row was deleted), or `UPDATE` (a row was
+	 * modified).
+	 */
+	enum trigger_event_manipulation event_manipulation;
+	/**
+	 * Whether the trigger activates before or after the
+	 * triggering event. The value is `BEFORE` or `AFTER`.
+	 */
+	enum trigger_action_timing action_timing;
+	/** The WHEN clause of the expression (may be NULL). */
+	struct Expr *pWhen;
+	/**
+	 * If this is an UPDATE OF <column-list> trigger,
+	 * the <column-list> is stored here
+	 */
+	struct IdList *pColumns;
+	/** Link list of trigger program steps. */
+	struct TriggerStep *step_list;
+	/**
+	 * Organize sql_trigger structs into linked list
+	 * with space::trigger_list.
+	 */
+	struct rlist link;
+};
+
 /**
  * Convert a given OP_INSERT/OP_UPDATE/OP_DELETE operation
  * to trigger_event_manipulation value.
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 233f56734..9f9f80b70 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1546,11 +1546,9 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
 	 * Do not account triggers deletion - they will be
 	 * accounted in DELETE from _space below.
 	 */
-	struct sql_trigger *trigger = space->sql_triggers;
-	while (trigger != NULL) {
+	struct sql_trigger *trigger;
+	rlist_foreach_entry(trigger, &space->trigger_list, link)
 		vdbe_code_drop_trigger(parse_context, trigger->zName, false);
-		trigger = trigger->next;
-	}
 	/*
 	 * Remove any entries from the _sequence_data, _sequence
 	 * and _space_sequence spaces associated with the table
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 3995e15dd..9083736ae 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -33,6 +33,7 @@
 #include "box/schema.h"
 #include "sqlInt.h"
 #include "tarantoolInt.h"
+#include "small/rlist.h"
 
 struct space *
 sql_lookup_space(struct Parse *parse, struct SrcList_item *space_name)
@@ -143,14 +144,14 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 	/* Figure out if we have any triggers and if the table
 	 * being deleted from is a view.
 	 */
-	struct sql_trigger *trigger_list = NULL;
+	struct rlist *trigger_list = NULL;
 	/* True if there are triggers or FKs or subqueries in the
 	 * WHERE clause.
 	 */
 	struct space *space = sql_lookup_space(parse, tab_list->a);
 	if (space == NULL)
 		goto delete_from_cleanup;
-	trigger_list = sql_triggers_exist(space->def,
+	trigger_list = sql_triggers_exist(space,
 					  TRIGGER_EVENT_MANIPULATION_DELETE,
 					  NULL, parse->sql_flags, NULL);
 	bool is_complex = trigger_list != NULL || fk_constraint_is_required(space, NULL);
@@ -432,7 +433,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
 
 void
 sql_generate_row_delete(struct Parse *parse, struct space *space,
-			struct sql_trigger *trigger_list, int cursor,
+			struct rlist *trigger_list, int cursor,
 			int reg_pk, short npk, bool need_update_count,
 			enum on_conflict_action onconf, u8 mode,
 			int idx_noseek)
diff --git a/src/box/sql/fk_constraint.c b/src/box/sql/fk_constraint.c
index 2a2d52de6..539a16560 100644
--- a/src/box/sql/fk_constraint.c
+++ b/src/box/sql/fk_constraint.c
@@ -857,6 +857,7 @@ fk_constraint_action_trigger(struct Parse *pParse, struct space_def *def,
 	if (trigger != NULL) {
 		size_t step_size = sizeof(TriggerStep) + name_len + 1;
 		trigger->step_list = sqlDbMallocZero(db, step_size);
+		rlist_create(&trigger->link);
 		step = trigger->step_list;
 		step->zTarget = (char *) &step[1];
 		memcpy((char *) step->zTarget, space_name, name_len);
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 0c8a3bc75..185f936b4 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -257,7 +257,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	int regData;		/* register holding first column to insert */
 	int *aRegIdx = 0;	/* One register allocated to each index */
 	/* List of triggers on pTab, if required. */
-	struct sql_trigger *trigger;
+	struct rlist *trigger_list;
 	int tmask;		/* Mask of trigger times */
 
 	db = pParse->db;
@@ -293,13 +293,13 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	 * inserted into is a view
 	 */
 	struct space_def *space_def = space->def;
-	trigger = sql_triggers_exist(space_def,
-				     TRIGGER_EVENT_MANIPULATION_INSERT, NULL,
-				     pParse->sql_flags, &tmask);
+	trigger_list = sql_triggers_exist(space,
+					  TRIGGER_EVENT_MANIPULATION_INSERT,
+					  NULL, pParse->sql_flags, &tmask);
 
 	bool is_view = space_def->opts.is_view;
-	assert((trigger != NULL && tmask != 0) ||
-	       (trigger == NULL && tmask == 0));
+	assert((trigger_list != NULL && tmask != 0) ||
+	       (trigger_list == NULL && tmask == 0));
 
 	/* If pTab is really a view, make sure it has been initialized.
 	 * ViewGetColumnNames() is a no-op if pTab is not a view.
@@ -320,7 +320,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	if (v == NULL)
 		goto insert_cleanup;
 	sqlVdbeCountChanges(v);
-	sql_set_multi_write(pParse, pSelect != NULL || trigger != NULL);
+	sql_set_multi_write(pParse, pSelect != NULL || trigger_list != NULL);
 
 	/* If the statement is of the form
 	 *
@@ -333,7 +333,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	 */
 	if (pColumn == NULL &&
 	    xferOptimization(pParse, space, pSelect, on_error)) {
-		assert(trigger == NULL);
+		assert(trigger_list == NULL);
 		assert(pList == 0);
 		goto insert_end;
 	}
@@ -435,7 +435,8 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		 * the SELECT statement. Also use a temp table in
 		 * the case of row triggers.
 		 */
-		if (trigger != NULL || vdbe_has_space_read(pParse, space_def))
+		if (trigger_list != NULL ||
+		    vdbe_has_space_read(pParse, space_def))
 			useTempTable = 1;
 
 		if (useTempTable) {
@@ -600,7 +601,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 			sql_emit_table_types(v, space_def, regCols + 1);
 
 		/* Fire BEFORE or INSTEAD OF triggers */
-		vdbe_code_row_trigger(pParse, trigger,
+		vdbe_code_row_trigger(pParse, trigger_list,
 				      TRIGGER_EVENT_MANIPULATION_INSERT, 0,
 				      TRIGGER_ACTION_TIMING_BEFORE, space,
 				      regCols - space_def->field_count - 1, on_error,
@@ -753,9 +754,9 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		sqlVdbeAddOp2(v, OP_AddImm, regRowCount, 1);
 	}
 
-	if (trigger != NULL) {
+	if (trigger_list != NULL) {
 		/* Code AFTER triggers */
-		vdbe_code_row_trigger(pParse, trigger,
+		vdbe_code_row_trigger(pParse, trigger_list,
 				      TRIGGER_EVENT_MANIPULATION_INSERT, 0,
 				      TRIGGER_ACTION_TIMING_AFTER, space,
 				      regData - 2 - space_def->field_count, on_error,
@@ -963,8 +964,8 @@ process_index:  ;
 					     skip_index, idx_key_reg,
 					     part_count);
 			sql_set_multi_write(parse_context, true);
-			struct sql_trigger *trigger =
-				sql_triggers_exist(space->def,
+			struct rlist *trigger =
+				sql_triggers_exist(space,
 					TRIGGER_EVENT_MANIPULATION_DELETE, NULL,
 					parse_context->sql_flags, NULL);
 			sql_generate_row_delete(parse_context, space, trigger,
@@ -1069,7 +1070,7 @@ xferOptimization(Parse * pParse,	/* Parser context */
 		return 0;
 	}
 	/* The pDest must not have triggers. */
-	if (dest->sql_triggers != NULL)
+	if (!rlist_empty(&dest->trigger_list))
 		return 0;
 	if (onError == ON_CONFLICT_ACTION_DEFAULT) {
 		onError = ON_CONFLICT_ACTION_ABORT;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index bb08ff8d8..8c9a69a00 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2282,49 +2282,6 @@ struct Parse {
 #define OPFLAG_XFER_OPT      0x01
 #endif
 
-/*
- * Each trigger present in the database schema is stored as an
- * instance of struct sql_trigger.
- * Pointers to instances of struct sql_trigger are stored in a
- * linked list, using the next member of struct sql_trigger. A
- * pointer to the first element of the linked list is stored as
- * sql_triggers member of the associated space.
- *
- * The "step_list" member points to the first element of a linked
- * list containing the SQL statements specified as the trigger
- * program.
- */
-struct sql_trigger {
-	/** The name of the trigger. */
-	char *zName;
-	/** The ID of space the trigger refers to. */
-	uint32_t space_id;
-	/**
-	 * The trigger event. This is the type of operation
-	 * on the associated space for which the trigger
-	 * activates. The value is `INSERT` (a row was inserted),
-	 * `DELETE` (a row was deleted), or `UPDATE` (a row was
-	 * modified).
-	 */
-	enum trigger_event_manipulation event_manipulation;
-	/**
-	 * Whether the trigger activates before or after the
-	 * triggering event. The value is `BEFORE` or `AFTER`.
-	 */
-	enum trigger_action_timing action_timing;
-	/** The WHEN clause of the expression (may be NULL). */
-	Expr *pWhen;
-	/**
-	 * If this is an UPDATE OF <column-list> trigger,
-	 * the <column-list> is stored here
-	 */
-	IdList *pColumns;
-	/** Link list of trigger program steps. */
-	TriggerStep *step_list;
-	/** Next trigger associated with the table. */
-	struct sql_trigger *next;
-};
-
 /*
  * An instance of struct TriggerStep is used to store a single SQL statement
  * that is a part of a trigger-program.
@@ -3229,7 +3186,7 @@ sql_expr_needs_no_type_change(const struct Expr *expr, enum field_type type);
  */
 void
 sql_generate_row_delete(struct Parse *parse, struct space *space,
-			struct sql_trigger *trigger_list, int cursor,
+			struct rlist *trigger_list, int cursor,
 			int reg_pk, short npk, bool need_update_count,
 			enum on_conflict_action onconf, u8 mode,
 			int idx_noseek);
@@ -3462,8 +3419,8 @@ vdbe_code_drop_trigger(struct Parse *parser, const char *trigger_name,
  * @param sql_flags SQL flags which describe how to parse request.
  * @param[out] pMask Mask of TRIGGER_BEFORE|TRIGGER_AFTER
  */
-struct sql_trigger *
-sql_triggers_exist(struct space_def *space_def,
+struct rlist *
+sql_triggers_exist(struct space *space,
 		   enum trigger_event_manipulation event_manipulation,
 		   struct ExprList *changes_list, uint32_t sql_flags,
 		   int *mask_ptr);
@@ -3512,7 +3469,7 @@ sql_triggers_exist(struct space_def *space_def,
  * jump to if a trigger program raises an IGNORE exception.
  *
  * @param parser Parse context.
- * @param trigger List of triggers on table.
+ * @param trigger_list List of triggers on table.
  * @param event_manipulation Trigger operation.
  * @param changes_list Changes list for any UPDATE OF triggers.
  * @param action_timing Whether the trigger activates before or
@@ -3523,7 +3480,7 @@ sql_triggers_exist(struct space_def *space_def,
  * @param ignore_jump Instruction to jump to for RAISE(IGNORE).
  */
 void
-vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
+vdbe_code_row_trigger(struct Parse *parser, struct rlist *trigger_list,
 		      enum trigger_event_manipulation event_manipulation,
 		      struct ExprList *changes_list,
 		      enum trigger_action_timing action_timing,
@@ -3537,7 +3494,7 @@ vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
  * header function for sql_code_row_trigger().
  *
  * @param parser Parse context.
- * @param trigger Trigger to code.
+ * @param trigger_list Trigger to code.
  * @param space The space to code triggers from.
  * @param reg Reg array containing OLD.* and NEW.* values.
  * @param orconf ON CONFLICT policy.
@@ -3650,7 +3607,7 @@ sql_trigger_delete_step(struct sql *db, struct Token *table_name,
  * returned mask if the TRIGGER_AFTER bit is set in tr_tm.
  *
  * @param parser  Parse context.
- * @param trigger List of triggers on table.
+ * @param trigger_list List of triggers on table.
  * @param changes_list Changes list for any UPDATE OF triggers.
  * @param new  1 for new.* ref mask, 0 for old.* ref mask.
  * @param action_timing_mask Mask of action timings referenced in
@@ -3661,7 +3618,7 @@ sql_trigger_delete_step(struct sql *db, struct Token *table_name,
  * @retval mask value.
  */
 uint64_t
-sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
+sql_trigger_colmask(struct Parse *parser, struct rlist *trigger_list,
 		    struct ExprList *changes_list, int new,
 		    uint8_t action_timing_mask, struct space *space,
 		    int orconf);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index f30c68b09..269c09a3d 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -130,6 +130,7 @@ sql_trigger_begin(struct Parse *parse)
 	trigger->action_timing = trigger_def->action_timing;
 	trigger->pWhen = sqlExprDup(db, trigger_def->when, EXPRDUP_REDUCE);
 	trigger->pColumns = sqlIdListDup(db, trigger_def->cols);
+	rlist_create(&trigger->link);
 	if ((trigger->pWhen != NULL && trigger->pWhen == NULL) ||
 	    (trigger->pColumns != NULL && trigger->pColumns == NULL))
 		goto trigger_cleanup;
@@ -472,18 +473,17 @@ sql_trigger_replace(const char *name, uint32_t space_id,
 			trigger->action_timing = TRIGGER_ACTION_TIMING_BEFORE;
 	}
 
-	struct sql_trigger **ptr = &space->sql_triggers;
-	while (*ptr != NULL && strcmp((*ptr)->zName, name) != 0)
-		ptr = &((*ptr)->next);
-	if (*ptr != NULL) {
-		*old_trigger = *ptr;
-		*ptr = (*ptr)->next;
+	struct sql_trigger *p;
+	rlist_foreach_entry(p, &space->trigger_list, link) {
+		if (strcmp(p->zName, name) != 0)
+			continue;
+		*old_trigger = p;
+		rlist_del(&p->link);
+		break;
 	}
 
-	if (trigger != NULL) {
-		trigger->next = space->sql_triggers;
-		space->sql_triggers = trigger;
-	}
+	if (trigger != NULL)
+		rlist_add(&space->trigger_list, &trigger->link);
 	return 0;
 }
 
@@ -499,15 +499,6 @@ sql_trigger_space_id(struct sql_trigger *trigger)
 	return trigger->space_id;
 }
 
-struct sql_trigger *
-space_trigger_list(uint32_t space_id)
-{
-	struct space *space = space_cache_find(space_id);
-	assert(space != NULL);
-	assert(space->def != NULL);
-	return space->sql_triggers;
-}
-
 /*
  * pEList is the SET clause of an UPDATE statement.  Each entry
  * in pEList is of the format <id>=<expr>.  If any of the entries
@@ -530,17 +521,18 @@ checkColumnOverlap(IdList * pIdList, ExprList * pEList)
 	return 0;
 }
 
-struct sql_trigger *
-sql_triggers_exist(struct space_def *space_def,
+struct rlist *
+sql_triggers_exist(struct space *space,
 		   enum trigger_event_manipulation event_manipulation,
 		   struct ExprList *changes_list, uint32_t sql_flags,
 		   int *mask_ptr)
 {
 	int mask = 0;
-	struct sql_trigger *trigger_list = NULL;
+	struct rlist *trigger_list = NULL;
 	if ((sql_flags & SQL_EnableTrigger) != 0)
-		trigger_list = space_trigger_list(space_def->id);
-	for (struct sql_trigger *p = trigger_list; p != NULL; p = p->next) {
+		trigger_list = &space->trigger_list;
+	struct sql_trigger *p;
+	rlist_foreach_entry(p, trigger_list, link) {
 		if (p->event_manipulation == event_manipulation &&
 		    checkColumnOverlap(p->pColumns, changes_list) != 0)
 			mask |= (1 << p->action_timing);
@@ -911,7 +903,7 @@ vdbe_code_row_trigger_direct(struct Parse *parser, struct sql_trigger *trigger,
 }
 
 void
-vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
+vdbe_code_row_trigger(struct Parse *parser, struct rlist *trigger_list,
 		      enum trigger_event_manipulation event_manipulation,
 		      struct ExprList *changes_list,
 		      enum trigger_action_timing action_timing,
@@ -921,8 +913,11 @@ vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
 	       action_timing == TRIGGER_ACTION_TIMING_AFTER);
 	assert((event_manipulation == TRIGGER_EVENT_MANIPULATION_UPDATE) ==
 	       (changes_list != NULL));
+	if (trigger_list == NULL)
+		return;
 
-	for (struct sql_trigger *p = trigger; p != NULL; p = p->next) {
+	struct sql_trigger *p;
+	rlist_foreach_entry(p, trigger_list, link) {
 		/* Determine whether we should code trigger. */
 		if (p->event_manipulation == event_manipulation &&
 		    p->action_timing == action_timing &&
@@ -934,7 +929,7 @@ vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
 }
 
 uint64_t
-sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
+sql_trigger_colmask(Parse *parser, struct rlist *trigger_list,
 		    ExprList *changes_list, int new, uint8_t action_timing_mask,
 		    struct space *space, int orconf)
 {
@@ -942,9 +937,12 @@ sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
 		changes_list != NULL ? TRIGGER_EVENT_MANIPULATION_UPDATE :
 				       TRIGGER_EVENT_MANIPULATION_DELETE;
 	uint64_t mask = 0;
+	if (trigger_list == NULL)
+		return mask;
 
 	assert(new == 1 || new == 0);
-	for (struct sql_trigger *p = trigger; p != NULL; p = p->next) {
+	struct sql_trigger *p;
+	rlist_foreach_entry(p, trigger_list, link) {
 		if (p->event_manipulation == event_manipulation &&
 		    ((action_timing_mask & (1 << p->action_timing)) != 0) &&
 		    checkColumnOverlap(p->pColumns, changes_list)) {
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 75bc487b8..0e1fef855 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -69,7 +69,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 
 	bool is_view;		/* True when updating a view (INSTEAD OF trigger) */
 	/* List of triggers on pTab, if required. */
-	struct sql_trigger *trigger;
+	struct rlist *trigger_list;
 	int tmask;		/* Mask of TRIGGER_BEFORE|TRIGGER_AFTER */
 	int iEph = 0;		/* Ephemeral table holding all primary key values */
 	int nKey = 0;		/* Number of elements in regKey */
@@ -100,11 +100,11 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	/* Figure out if we have any triggers and if the table being
 	 * updated is a view.
 	 */
-	trigger = sql_triggers_exist(space->def,
-				     TRIGGER_EVENT_MANIPULATION_UPDATE,
-				     pChanges, pParse->sql_flags, &tmask);
+	trigger_list = sql_triggers_exist(space,
+					  TRIGGER_EVENT_MANIPULATION_UPDATE,
+					  pChanges, pParse->sql_flags, &tmask);
 	is_view = space->def->opts.is_view;
-	assert(trigger != NULL || tmask == 0);
+	assert(trigger_list != NULL || tmask == 0);
 
 	if (is_view &&
 	    sql_view_assign_cursors(pParse, space->def->opts.sql) != 0) {
@@ -192,7 +192,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	/* Allocate required registers. */
 	regOldPk = regNewPk = ++pParse->nMem;
 
-	if (is_pk_modified || trigger != NULL || hasFK != 0) {
+	if (is_pk_modified || trigger_list != NULL || hasFK != 0) {
 		regOld = pParse->nMem + 1;
 		pParse->nMem += def->field_count;
 		regNewPk = ++pParse->nMem;
@@ -301,16 +301,16 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	 * then regNewPk is the same register as regOldPk, which is
 	 * already populated.
 	 */
-	assert(is_pk_modified || trigger != NULL || hasFK != 0 ||
+	assert(is_pk_modified || trigger_list != NULL || hasFK != 0 ||
 	       regOldPk == regNewPk);
 
 	/* Compute the old pre-UPDATE content of the row being changed, if that
 	 * information is needed
 	 */
-	if (is_pk_modified || hasFK != 0 || trigger != NULL) {
+	if (is_pk_modified || hasFK != 0 || trigger_list != NULL) {
 		assert(space != NULL);
 		uint64_t oldmask = hasFK ? space->fk_constraint_mask : 0;
-		oldmask |= sql_trigger_colmask(pParse, trigger, pChanges, 0,
+		oldmask |= sql_trigger_colmask(pParse, trigger_list, pChanges, 0,
 					(1 << TRIGGER_ACTION_TIMING_BEFORE) |
 					(1 << TRIGGER_ACTION_TIMING_AFTER),
 					space, on_error);
@@ -339,7 +339,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	 * may have modified them). So not loading those that are not going to
 	 * be used eliminates some redundant opcodes.
 	 */
-	uint64_t newmask = sql_trigger_colmask(pParse, trigger, pChanges, 1,
+	uint64_t newmask = sql_trigger_colmask(pParse, trigger_list, pChanges, 1,
 					       1 << TRIGGER_ACTION_TIMING_BEFORE,
 					       space, on_error);
 	for (i = 0; i < (int)def->field_count; i++) {
@@ -366,7 +366,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	 */
 	if ((tmask & (1 << TRIGGER_ACTION_TIMING_BEFORE)) != 0) {
 		sql_emit_table_types(v, space->def, regNew);
-		vdbe_code_row_trigger(pParse, trigger,
+		vdbe_code_row_trigger(pParse, trigger_list,
 				      TRIGGER_EVENT_MANIPULATION_UPDATE,
 				      pChanges, TRIGGER_ACTION_TIMING_BEFORE,
 				      space, regOldPk, on_error, labelContinue);
@@ -481,7 +481,7 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 		sqlVdbeAddOp2(v, OP_AddImm, regRowCount, 1);
 	}
 
-	vdbe_code_row_trigger(pParse, trigger,
+	vdbe_code_row_trigger(pParse, trigger_list,
 			      TRIGGER_EVENT_MANIPULATION_UPDATE, pChanges,
 			      TRIGGER_ACTION_TIMING_AFTER, space, regOldPk,
 			      on_error, labelContinue);
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index f50198281..7af1bfa60 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4710,23 +4710,28 @@ case OP_RenameTable: {
 	space = space_by_id(space_id);
 	assert(space);
 	/* Rename space op doesn't change triggers. */
-	struct sql_trigger *triggers = space->sql_triggers;
+	RLIST_HEAD(trigger_list);
+	rlist_swap(&trigger_list, &space->trigger_list);
 	zOldTableName = space_name(space);
 	assert(zOldTableName);
 	zNewTableName = pOp->p4.z;
 	zOldTableName = sqlDbStrNDup(db, zOldTableName,
 					 sqlStrlen30(zOldTableName));
-	if (sql_rename_table(space_id, zNewTableName) != 0)
+	if (sql_rename_table(space_id, zNewTableName) != 0) {
+		rlist_swap(&trigger_list, &space->trigger_list);
 		goto abort_due_to_error;
+	}
+	space = space_by_id(space_id);
+	assert(space != NULL);
+	rlist_swap(&trigger_list, &space->trigger_list);
 	/*
 	 * Rebuild 'CREATE TRIGGER' expressions of all triggers
 	 * created on this table. Sure, this action is not atomic
 	 * due to lack of transactional DDL, but just do the best
 	 * effort.
 	 */
-	for (struct sql_trigger *trigger = triggers; trigger != NULL; ) {
-		/* Store pointer as trigger will be destructed. */
-		struct sql_trigger *next_trigger = trigger->next;
+	struct sql_trigger *trigger, *tmp;
+	rlist_foreach_entry_safe(trigger, &space->trigger_list, link, tmp) {
 		/*
 		 * FIXME: In the case of error, part of triggers
 		 * would have invalid space name in tuple so can
@@ -4736,7 +4741,6 @@ case OP_RenameTable: {
 		if (tarantoolsqlRenameTrigger(trigger->zName, zOldTableName,
 					      zNewTableName) != 0)
 			goto abort_due_to_error;
-		trigger = next_trigger;
 	}
 	sqlDbFree(db, (void*)zOldTableName);
 	break;
-- 
2.23.0