[Tarantool-patches] [PATCH v1 4/9] box: introduce trigger_action_timing enum

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

This patch introduces a new trigger_action_timing enum
that describes a moment when the trigger activates: before or
after the triggering event.

Using the enum instead of sql-specific TK_ token identified is
an important step to introduce a trigger_def structure in
further patches.

Needed for #4343
---
 src/box/sql.c           | 15 ++++++++++++++
 src/box/sql.h           |  7 +++++++
 src/box/sql/delete.c    | 13 +++++++------
 src/box/sql/insert.c    |  6 +++---
 src/box/sql/parse_def.h |  9 ++++++---
 src/box/sql/sqlInt.h    | 35 ++++++++++++++++-----------------
 src/box/sql/trigger.c   | 43 ++++++++++++++++++++++-------------------
 src/box/sql/update.c    | 20 ++++++++++---------
 src/box/trigger_def.c   |  2 ++
 src/box/trigger_def.h   | 17 ++++++++++++++++
 10 files changed, 108 insertions(+), 59 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 670e4fb3e..134225dcc 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1280,3 +1280,18 @@ trigger_event_manipulation_by_op(int op)
 		unreachable();
 	}
 }
+
+enum trigger_action_timing
+trigger_action_timing_by_op(int op)
+{
+	switch (op) {
+	case TK_BEFORE:
+		return TRIGGER_ACTION_TIMING_BEFORE;
+	case TK_AFTER:
+		return TRIGGER_ACTION_TIMING_AFTER;
+	case TK_INSTEAD:
+		return TRIGGER_ACTION_TIMING_INSTEAD;
+	default:
+		unreachable();
+	}
+}
diff --git a/src/box/sql.h b/src/box/sql.h
index ac6efeec9..1f289ad97 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -428,6 +428,13 @@ vdbe_field_ref_prepare_tuple(struct vdbe_field_ref *field_ref,
 enum trigger_event_manipulation
 trigger_event_manipulation_by_op(int op);
 
+/**
+ * Convert a given TK_BEFORE/TK_AFTER/TK_INSTEAD operation
+ * to trigger_event_manipulation value.
+ */
+enum trigger_action_timing
+trigger_action_timing_by_op(int op);
+
 #if defined(__cplusplus)
 } /* extern "C" { */
 #endif
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 94a1f7488..3995e15dd 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -466,8 +466,9 @@ sql_generate_row_delete(struct Parse *parse, struct space *space,
 		/* TODO: Could use temporary registers here. */
 		uint64_t mask =
 			sql_trigger_colmask(parse, trigger_list, 0, 0,
-					    TRIGGER_BEFORE | TRIGGER_AFTER,
-					    space, onconf);
+					(1 << TRIGGER_ACTION_TIMING_BEFORE) |
+					(1 << TRIGGER_ACTION_TIMING_AFTER),
+					space, onconf);
 		assert(space != NULL);
 		mask |= space->fk_constraint_mask;
 		first_old_reg = parse->nMem + 1;
@@ -489,8 +490,8 @@ sql_generate_row_delete(struct Parse *parse, struct space *space,
 		int addr_start = sqlVdbeCurrentAddr(v);
 		vdbe_code_row_trigger(parse, trigger_list,
 				      TRIGGER_EVENT_MANIPULATION_DELETE, NULL,
-				      TRIGGER_BEFORE, space, first_old_reg,
-				      onconf, label);
+				      TRIGGER_ACTION_TIMING_BEFORE, space,
+				      first_old_reg, onconf, label);
 
 		/* If any BEFORE triggers were coded, then seek
 		 * the cursor to the row to be deleted again. It
@@ -544,8 +545,8 @@ sql_generate_row_delete(struct Parse *parse, struct space *space,
 		/* Invoke AFTER DELETE trigger programs. */
 		vdbe_code_row_trigger(parse, trigger_list,
 				      TRIGGER_EVENT_MANIPULATION_DELETE, 0,
-				      TRIGGER_AFTER, space, first_old_reg,
-				      onconf, label);
+				      TRIGGER_ACTION_TIMING_AFTER, space,
+				      first_old_reg, onconf, label);
 	}
 
 	/* Jump here if the row had already been deleted before
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 9c612dc28..0c8a3bc75 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -552,7 +552,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 	/* Run the BEFORE and INSTEAD OF triggers, if there are any
 	 */
 	endOfLoop = sqlVdbeMakeLabel(v);
-	if (tmask & TRIGGER_BEFORE) {
+	if ((tmask & (1 << TRIGGER_ACTION_TIMING_BEFORE)) != 0) {
 		int regCols =
 			sqlGetTempRange(pParse, space_def->field_count + 1);
 
@@ -602,7 +602,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		/* Fire BEFORE or INSTEAD OF triggers */
 		vdbe_code_row_trigger(pParse, trigger,
 				      TRIGGER_EVENT_MANIPULATION_INSERT, 0,
-				      TRIGGER_BEFORE, space,
+				      TRIGGER_ACTION_TIMING_BEFORE, space,
 				      regCols - space_def->field_count - 1, on_error,
 				      endOfLoop);
 
@@ -757,7 +757,7 @@ sqlInsert(Parse * pParse,	/* Parser context */
 		/* Code AFTER triggers */
 		vdbe_code_row_trigger(pParse, trigger,
 				      TRIGGER_EVENT_MANIPULATION_INSERT, 0,
-				      TRIGGER_AFTER, space,
+				      TRIGGER_ACTION_TIMING_AFTER, space,
 				      regData - 2 - space_def->field_count, on_error,
 				      endOfLoop);
 	}
diff --git a/src/box/sql/parse_def.h b/src/box/sql/parse_def.h
index cf233fc01..2502fc46c 100644
--- a/src/box/sql/parse_def.h
+++ b/src/box/sql/parse_def.h
@@ -267,8 +267,11 @@ struct drop_index_def {
 
 struct create_trigger_def {
 	struct create_entity_def base;
-	/** One of TK_BEFORE, TK_AFTER, TK_INSTEAD. */
-	int tr_tm;
+	/**
+	 * Whether the trigger activates before or after the
+	 * triggering event.
+	 */
+	enum trigger_action_timing action_timing;
 	/** The trigger event: INSERT, UPDATE or DELETE. */
 	enum trigger_event_manipulation event_manipulation;
 	/** Column list if this is an UPDATE trigger. */
@@ -413,7 +416,7 @@ create_trigger_def_init(struct create_trigger_def *trigger_def,
 {
 	create_entity_def_init(&trigger_def->base, ENTITY_TYPE_TRIGGER,
 			       table_name, name, if_not_exists);
-	trigger_def->tr_tm = tr_tm;
+	trigger_def->action_timing = trigger_action_timing_by_op(tr_tm);
 	trigger_def->event_manipulation = trigger_event_manipulation_by_op(op);
 	trigger_def->cols = cols;
 	trigger_def->when = when;
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 7deae7087..bb08ff8d8 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -2307,8 +2307,11 @@ struct sql_trigger {
 	 * modified).
 	 */
 	enum trigger_event_manipulation event_manipulation;
-	/** One of TRIGGER_BEFORE, TRIGGER_AFTER. */
-	u8 tr_tm;
+	/**
+	 * 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;
 	/**
@@ -2322,16 +2325,6 @@ struct sql_trigger {
 	struct sql_trigger *next;
 };
 
-/*
- * A trigger is either a BEFORE or an AFTER trigger.  The following constants
- * determine which.
- *
- * If there are multiple triggers, you might of some BEFORE and some AFTER.
- * In that cases, the constants below can be ORed together.
- */
-#define TRIGGER_BEFORE  1
-#define TRIGGER_AFTER   2
-
 /*
  * An instance of struct TriggerStep is used to store a single SQL statement
  * that is a part of a trigger-program.
@@ -3522,7 +3515,8 @@ sql_triggers_exist(struct space_def *space_def,
  * @param trigger List of triggers on table.
  * @param event_manipulation Trigger operation.
  * @param changes_list Changes list for any UPDATE OF triggers.
- * @param tr_tm One of TRIGGER_BEFORE, TRIGGER_AFTER.
+ * @param action_timing Whether the trigger activates before or
+ *                      after the triggering event. .
  * @param space The space to code triggers from.
  * @param reg The first in an array of registers.
  * @param orconf ON CONFLICT policy.
@@ -3531,8 +3525,10 @@ sql_triggers_exist(struct space_def *space_def,
 void
 vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
 		      enum trigger_event_manipulation event_manipulation,
-		      struct ExprList *changes_list, int tr_tm,
-		      struct space *space, int reg, int orconf, int ignore_jump);
+		      struct ExprList *changes_list,
+		      enum trigger_action_timing action_timing,
+		      struct space *space, int reg, int orconf,
+		      int ignore_jump);
 
 /**
  * Generate code for the trigger program associated with trigger
@@ -3657,7 +3653,8 @@ sql_trigger_delete_step(struct sql *db, struct Token *table_name,
  * @param trigger 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 tr_tm Mask of TRIGGER_BEFORE|TRIGGER_AFTER.
+ * @param action_timing_mask Mask of action timings referenced in
+ *                           the triggers list.
  * @param space The space to code triggers from.
  * @param orconf Default ON CONFLICT policy for trigger steps.
  *
@@ -3665,8 +3662,10 @@ sql_trigger_delete_step(struct sql *db, struct Token *table_name,
  */
 uint64_t
 sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
-		    ExprList *changes_list, int new, int tr_tm,
-		    struct space *space, int orconf);
+		    struct ExprList *changes_list, int new,
+		    uint8_t action_timing_mask, struct space *space,
+		    int orconf);
+
 #define sqlParseToplevel(p) ((p)->pToplevel ? (p)->pToplevel : (p))
 #define sqlIsToplevel(p) ((p)->pToplevel==0)
 
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index cdf51c053..f30c68b09 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -127,7 +127,7 @@ sql_trigger_begin(struct Parse *parse)
 	trigger->zName = trigger_name;
 	trigger_name = NULL;
 	trigger->event_manipulation = trigger_def->event_manipulation;
-	trigger->tr_tm = trigger_def->tr_tm;
+	trigger->action_timing = trigger_def->action_timing;
 	trigger->pWhen = sqlExprDup(db, trigger_def->when, EXPRDUP_REDUCE);
 	trigger->pColumns = sqlIdListDup(db, trigger_def->cols);
 	if ((trigger->pWhen != NULL && trigger->pWhen == NULL) ||
@@ -449,25 +449,27 @@ sql_trigger_replace(const char *name, uint32_t space_id,
 		 * INSTEAD of triggers are only for views and
 		 * views only support INSTEAD of triggers.
 		 */
-		if (space->def->opts.is_view && trigger->tr_tm != TK_INSTEAD) {
-			diag_set(ClientError, ER_SQL_EXECUTE,
-				 tt_sprintf("cannot create %s "\
-                         "trigger on view: %s", trigger->tr_tm == TK_BEFORE ?
-						"BEFORE" : "AFTER",
-					    space->def->name));
+		if (space->def->opts.is_view &&
+		    trigger->action_timing != TRIGGER_ACTION_TIMING_INSTEAD) {
+			const char *err_msg =
+				tt_sprintf("cannot create %s trigger on "
+					   "view: %s",
+					    trigger_action_timing_strs[
+						trigger->action_timing],
+					    space->def->name);
+			diag_set(ClientError, ER_SQL_EXECUTE, err_msg);
 			return -1;
 		}
-		if (!space->def->opts.is_view && trigger->tr_tm == TK_INSTEAD) {
+		if (!space->def->opts.is_view &&
+		    trigger->action_timing == TRIGGER_ACTION_TIMING_INSTEAD) {
 			diag_set(ClientError, ER_SQL_EXECUTE,
 				 tt_sprintf("cannot create "\
                          "INSTEAD OF trigger on space: %s", space->def->name));
 			return -1;
 		}
 
-		if (trigger->tr_tm == TK_BEFORE || trigger->tr_tm == TK_INSTEAD)
-			trigger->tr_tm = TRIGGER_BEFORE;
-		else if (trigger->tr_tm == TK_AFTER)
-			trigger->tr_tm = TRIGGER_AFTER;
+		if (trigger->action_timing == TRIGGER_ACTION_TIMING_INSTEAD)
+			trigger->action_timing = TRIGGER_ACTION_TIMING_BEFORE;
 	}
 
 	struct sql_trigger **ptr = &space->sql_triggers;
@@ -541,7 +543,7 @@ sql_triggers_exist(struct space_def *space_def,
 	for (struct sql_trigger *p = trigger_list; p != NULL; p = p->next) {
 		if (p->event_manipulation == event_manipulation &&
 		    checkColumnOverlap(p->pColumns, changes_list) != 0)
-			mask |= p->tr_tm;
+			mask |= (1 << p->action_timing);
 	}
 	if (mask_ptr != NULL)
 		*mask_ptr = mask;
@@ -763,8 +765,7 @@ sql_row_trigger_program(struct Parse *parser, struct sql_trigger *trigger,
 	if (v != NULL) {
 		VdbeComment((v, "Start: %s.%s (%s %s ON %s)",
 			     trigger->zName, onErrorText(orconf),
-			     (trigger->tr_tm ==
-			      TRIGGER_BEFORE ? "BEFORE" : "AFTER"),
+			     trigger_action_timing_strs[trigger->action_timing],
 			      trigger_event_manipulation_strs[
 						trigger->event_manipulation],
 			      space->def->name));
@@ -912,17 +913,19 @@ vdbe_code_row_trigger_direct(struct Parse *parser, struct sql_trigger *trigger,
 void
 vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
 		      enum trigger_event_manipulation event_manipulation,
-		      struct ExprList *changes_list, int tr_tm,
+		      struct ExprList *changes_list,
+		      enum trigger_action_timing action_timing,
 		      struct space *space, int reg, int orconf, int ignore_jump)
 {
-	assert(tr_tm == TRIGGER_BEFORE || tr_tm == TRIGGER_AFTER);
+	assert(action_timing == TRIGGER_ACTION_TIMING_BEFORE ||
+	       action_timing == TRIGGER_ACTION_TIMING_AFTER);
 	assert((event_manipulation == TRIGGER_EVENT_MANIPULATION_UPDATE) ==
 	       (changes_list != NULL));
 
 	for (struct sql_trigger *p = trigger; p != NULL; p = p->next) {
 		/* Determine whether we should code trigger. */
 		if (p->event_manipulation == event_manipulation &&
-		    p->tr_tm == tr_tm &&
+		    p->action_timing == action_timing &&
 		    checkColumnOverlap(p->pColumns, changes_list)) {
 			vdbe_code_row_trigger_direct(parser, p, space, reg,
 						     orconf, ignore_jump);
@@ -932,7 +935,7 @@ vdbe_code_row_trigger(struct Parse *parser, struct sql_trigger *trigger,
 
 uint64_t
 sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
-		    ExprList *changes_list, int new, int tr_tm,
+		    ExprList *changes_list, int new, uint8_t action_timing_mask,
 		    struct space *space, int orconf)
 {
 	enum trigger_event_manipulation event_manipulation =
@@ -943,7 +946,7 @@ sql_trigger_colmask(Parse *parser, struct sql_trigger *trigger,
 	assert(new == 1 || new == 0);
 	for (struct sql_trigger *p = trigger; p != NULL; p = p->next) {
 		if (p->event_manipulation == event_manipulation &&
-		    (tr_tm & p->tr_tm) &&
+		    ((action_timing_mask & (1 << p->action_timing)) != 0) &&
 		    checkColumnOverlap(p->pColumns, changes_list)) {
 			TriggerPrg *prg =
 				sql_row_trigger(parser, p, space, orconf);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 400375bcf..75bc487b8 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -311,8 +311,9 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 		assert(space != NULL);
 		uint64_t oldmask = hasFK ? space->fk_constraint_mask : 0;
 		oldmask |= sql_trigger_colmask(pParse, trigger, pChanges, 0,
-					       TRIGGER_BEFORE | TRIGGER_AFTER,
-					       space, on_error);
+					(1 << TRIGGER_ACTION_TIMING_BEFORE) |
+					(1 << TRIGGER_ACTION_TIMING_AFTER),
+					space, on_error);
 		for (i = 0; i < (int)def->field_count; i++) {
 			if (column_mask_fieldno_is_set(oldmask, i) ||
 			    sql_space_column_is_in_pk(space, i)) {
@@ -339,13 +340,14 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	 * be used eliminates some redundant opcodes.
 	 */
 	uint64_t newmask = sql_trigger_colmask(pParse, trigger, pChanges, 1,
-					       TRIGGER_BEFORE, space, on_error);
+					       1 << TRIGGER_ACTION_TIMING_BEFORE,
+					       space, on_error);
 	for (i = 0; i < (int)def->field_count; i++) {
 		j = aXRef[i];
 		if (j >= 0) {
 			sqlExprCode(pParse, pChanges->a[j].pExpr,
 					regNew + i);
-		} else if ((tmask & TRIGGER_BEFORE) == 0 ||
+		} else if ((tmask & (1 << TRIGGER_ACTION_TIMING_BEFORE)) == 0 ||
 			   column_mask_fieldno_is_set(newmask, i) != 0) {
 			/* This branch loads the value of a column that will not be changed
 			 * into a register. This is done if there are no BEFORE triggers, or
@@ -362,12 +364,12 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 	/* Fire any BEFORE UPDATE triggers. This happens before constraints are
 	 * verified. One could argue that this is wrong.
 	 */
-	if (tmask & TRIGGER_BEFORE) {
+	if ((tmask & (1 << TRIGGER_ACTION_TIMING_BEFORE)) != 0) {
 		sql_emit_table_types(v, space->def, regNew);
 		vdbe_code_row_trigger(pParse, trigger,
 				      TRIGGER_EVENT_MANIPULATION_UPDATE,
-				      pChanges, TRIGGER_BEFORE, space, regOldPk,
-				      on_error, labelContinue);
+				      pChanges, TRIGGER_ACTION_TIMING_BEFORE,
+				      space, regOldPk, on_error, labelContinue);
 
 		/* The row-trigger may have deleted the row being updated. In this
 		 * case, jump to the next row. No updates or AFTER triggers are
@@ -481,8 +483,8 @@ sqlUpdate(Parse * pParse,		/* The parser context */
 
 	vdbe_code_row_trigger(pParse, trigger,
 			      TRIGGER_EVENT_MANIPULATION_UPDATE, pChanges,
-			      TRIGGER_AFTER, space, regOldPk, on_error,
-			      labelContinue);
+			      TRIGGER_ACTION_TIMING_AFTER, space, regOldPk,
+			      on_error, labelContinue);
 
 	/* Repeat the above with the next record to be updated, until
 	 * all record selected by the WHERE clause have been updated.
diff --git a/src/box/trigger_def.c b/src/box/trigger_def.c
index 4c34f5089..ff7938f8d 100644
--- a/src/box/trigger_def.c
+++ b/src/box/trigger_def.c
@@ -31,3 +31,5 @@
 #include "trigger_def.h"
 
 const char *trigger_event_manipulation_strs[] = {"DELETE", "UPDATE", "INSERT"};
+
+const char *trigger_action_timing_strs[] = {"BEFORE", "AFTER", "INSTEAD"};
diff --git a/src/box/trigger_def.h b/src/box/trigger_def.h
index c1fed4f82..dd34f6859 100644
--- a/src/box/trigger_def.h
+++ b/src/box/trigger_def.h
@@ -50,6 +50,23 @@ enum trigger_event_manipulation {
 
 extern const char *trigger_event_manipulation_strs[];
 
+/**
+ * Whether the trigger activates before or after the triggering
+ * event. The value is `BEFORE` or `AFTER`.
+ */
+enum trigger_action_timing {
+	TRIGGER_ACTION_TIMING_BEFORE,
+	TRIGGER_ACTION_TIMING_AFTER,
+	/*
+	 * INSTEAD of triggers are only for views and
+	 * views only support INSTEAD of triggers.
+	 */
+	TRIGGER_ACTION_TIMING_INSTEAD,
+	trigger_action_timing_MAX
+};
+
+extern const char *trigger_action_timing_strs[];
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
-- 
2.23.0