From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 20FFC42F040 for ; Mon, 14 Oct 2019 19:03:28 +0300 (MSK) From: Kirill Shcherbatov Date: Mon, 14 Oct 2019 19:03:19 +0300 Message-Id: <5be4e412cc767e3c0626670493da4f9c325f59e9.1571068485.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v1 4/9] box: introduce trigger_action_timing enum List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org, kostja.osipov@gmail.com, korablev@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