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 28DC042F041 for ; Mon, 14 Oct 2019 19:03:29 +0300 (MSK) From: Kirill Shcherbatov Date: Mon, 14 Oct 2019 19:03:20 +0300 Message-Id: <60d9fb2635567c5be665bf01a8beae1abe9a1905.1571068485.git.kshcherbatov@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v1 5/9] sql: use rlist to organize triggers in a list 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 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 #include #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 trigger, + * the 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 trigger, - * the 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 =. 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