[tarantool-patches] Re: [PATCH v4 8/8] sql: remove global sql_trigger hash
n.pettik
korablev at tarantool.org
Wed Jun 27 20:28:00 MSK 2018
> On 26 Jun 2018, at 19:13, Kirill Shcherbatov <kshcherbatov at tarantool.org> wrote:
>
> As new _trigger format contain space_id,
Typo: ‘contains’.
> we can do not
https://www.urbandictionary.com/define.php?term=Can%20Don%27t
It is better to say: ‘we can avoid storing AST pointers in HASH’.
> store AST pointers in HASH. Requested AST could be found
> by name in appropriate space.
Personally I would squash this commit with ’sql: move Triggers to server’,
but it is up to you.
> @@ -3973,10 +3973,14 @@ sqlite3WithDelete(sqlite3 * db, With * pWith)
> #endif /* !defined(SQLITE_OMIT_CTE) */
>
> int
> -vdbe_emit_halt_if_exists(struct Parse *parser, int space_id, int index_id,
> - const char *name_src, int tarantool_error_code,
> - const char *error_src, bool no_error)
> -{
> +vdbe_emit_halt_with_presence_test(struct Parse *parser, int space_id,
> + int index_id,
> + const char *name_src,
You can fit index_id and name_src in one line.
> @@ -4103,16 +4102,24 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
> */
> void sql_trigger_finish(Parse *, TriggerStep *, Token *);
>
> -void sqlite3DropTrigger(Parse *, SrcList *, int);
> +/**
> + * This function is called from parser to generate drop trigger
> + * VDBE code.
> + *
> + * @param parser Parser context.
> + * @param name The name of trigger to drop.
> + * @param no_err Suppress errors if the trigger already exists.
> + */
> +void vdbe_code_drop_trigger(Parse *, SrcList *, int);
void
vdbe_code_drop_trigger...
And add arg names to prototype.
>
> /**
> * Drop a trigger given a pointer to that trigger.
> *
> - * @param parser Parse context.
> - * @param trigger Trigger to drop.
> + * @param parser Parser context.
> + * @param name The name of trigger to drop.
> */
> void
> -vdbe_code_drop_trigger_ptr(struct Parse *parser, struct sql_trigger *trigger);
> +vdbe_code_drop_trigger_ptr(struct Parse *parser, const char *trigger_name);
>
> /**
> * Return a list of all triggers on table pTab if there exists at
> @@ -4784,8 +4791,8 @@ table_column_nullable_action(struct Table *tab, uint32_t column);
>
> /**
> * Generate VDBE code to halt execution with correct error if
> - * the object with specified name is already present in
> - * specified space.
> + * the object with specified name is already present (or doesn't
> + * present is raise_if_not_exists flag is set
I don’t see such flag.
> diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
> index 4c28580..ca01e6e 100644
> --- a/src/box/sql/trigger.c
> +++ b/src/box/sql/trigger.c
> @@ -125,11 +125,11 @@ sql_trigger_begin(struct Parse *parse, struct Token *name, int tr_tm,
> const char *error_msg =
> tt_sprintf(tnt_errcode_desc(ER_TRIGGER_EXISTS),
> trigger_name);
> - if (vdbe_emit_halt_if_exists(parse, BOX_TRIGGER_ID,
> - 0, trigger_name,
> - ER_TRIGGER_EXISTS,
> - error_msg,
> - (no_err != 0)) != 0)
> + if (vdbe_emit_halt_with_presence_test(parse, BOX_TRIGGER_ID, 0,
> + trigger_name,
> + ER_TRIGGER_EXISTS,
> + error_msg, (no_err != 0),
> + OP_NoConflict) != 0)
> goto trigger_cleanup;
> }
>
> @@ -441,21 +441,36 @@ sql_trigger_delete(struct sqlite3 *db, struct sql_trigger *trigger)
> sqlite3DbFree(db, trigger);
> }
>
> -/*
> - * This function is called to drop a trigger from the database
> - * schema.
> - *
> - * This may be called directly from the parser and therefore
> - * identifies the trigger by name. The sql_drop_trigger_ptr()
> - * routine does the same job as this routine except it takes a
> - * pointer to the trigger instead of the trigger name.
> - */
> void
> -sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr)
> +vdbe_code_drop_trigger_ptr(struct Parse *parser, const char *trigger_name)
Lets drop ‘_ptr’ suffix.
> {
> - struct sql_trigger *trigger = NULL;
> - const char *zName;
> - sqlite3 *db = pParse->db;
> + sqlite3 *db = parser->db;
> + assert(db->pSchema != NULL);
> + struct Vdbe *v = sqlite3GetVdbe(parser);
> + if (v == NULL)
> + return;
> + /*
> + * Generate code to delete entry from _trigger and
> + * internal SQL structures.
> + */
> + int trig_name_reg = ++parser->nMem;
> + int record_to_delete = ++parser->nMem;
> + sqlite3VdbeAddOp4(v, OP_String8, 0, trig_name_reg, 0,
> + sqlite3DbStrDup(db, trigger_name), P4_DYNAMIC);
> + sqlite3VdbeAddOp3(v, OP_MakeRecord, trig_name_reg, 1,
> + record_to_delete);
> + sqlite3VdbeAddOp2(v, OP_SDelete, BOX_TRIGGER_ID,
> + record_to_delete);
> + if (parser->nested == 0)
> + sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
> + sqlite3ChangeCookie(parser);
> +}
> +
> +void
> +vdbe_code_drop_trigger(struct Parse *parser, struct SrcList *name, int no_err)
Lets call it with ’sql_’ prefix - this function is invoked from parser
(see for instance sql_drop_table(), sql_drop_index() etc).
> @@ -562,24 +549,19 @@ sql_trigger_replace(struct sqlite3 *db, const char *name,
> trigger->tr_tm = TRIGGER_AFTER;
> }
>
> -
> - *old_trigger = sqlite3HashInsert(hash, name, trigger);
> - if (*old_trigger == trigger) {
> - diag_set(OutOfMemory, sizeof(struct HashElem),
> - "sqlite3HashInsert", "ret");
> - return -1;
> + 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;
> }
>
> - if (*old_trigger != NULL) {
> - struct sql_trigger **pp;
> - for (pp = &space->sql_triggers; *pp != *old_trigger;
> - pp = &((*pp)->next));
> - *pp = (*pp)->next;
> - }
> if (trigger != NULL) {
> trigger->next = space->sql_triggers;
> space->sql_triggers = trigger;
> }
> +
Redundant diff.
More information about the Tarantool-patches
mailing list