From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 537DB2276B for ; Wed, 27 Jun 2018 13:28:05 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id NyQbLj1LSA0P for ; Wed, 27 Jun 2018 13:28:05 -0400 (EDT) Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 1390B21015 for ; Wed, 27 Jun 2018 13:28:04 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 10.3 \(3273\)) Subject: [tarantool-patches] Re: [PATCH v4 8/8] sql: remove global sql_trigger hash From: "n.pettik" In-Reply-To: <1d6bcf87b16c4b7c64eb82d3da43adfabede0a7e.1530029141.git.kshcherbatov@tarantool.org> Date: Wed, 27 Jun 2018 20:28:00 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <44395AB5-7067-46A9-A785-A0707686DCD9@tarantool.org> References: <81986d1f9307191bd3d3e37514dc32fadb7e6970.1530029141.git.kshcherbatov@tarantool.org> <1d6bcf87b16c4b7c64eb82d3da43adfabede0a7e.1530029141.git.kshcherbatov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: Kirill Shcherbatov > On 26 Jun 2018, at 19:13, Kirill Shcherbatov = wrote: >=20 > As new _trigger format contain space_id, Typo: =E2=80=98contains=E2=80=99. > we can do not https://www.urbandictionary.com/define.php?term=3DCan%20Don%27t It is better to say: =E2=80=98we can avoid storing AST pointers in = HASH=E2=80=99. > store AST pointers in HASH. Requested AST could be found > by name in appropriate space. Personally I would squash this commit with =E2=80=99sql: move Triggers = to server=E2=80=99, but it is up to you. > @@ -3973,10 +3973,14 @@ sqlite3WithDelete(sqlite3 * db, With * pWith) > #endif /* !defined(SQLITE_OMIT_CTE) */ >=20 > 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 *); >=20 > -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. >=20 > /** > * 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); >=20 > /** > * 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); >=20 > /** > * 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=E2=80=99t 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 =3D > 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 !=3D 0)) !=3D 0) > + if (vdbe_emit_halt_with_presence_test(parse, = BOX_TRIGGER_ID, 0, > + trigger_name, > + ER_TRIGGER_EXISTS, > + error_msg, (no_err = !=3D 0), > + OP_NoConflict) !=3D = 0) > goto trigger_cleanup; > } >=20 > @@ -441,21 +441,36 @@ sql_trigger_delete(struct sqlite3 *db, struct = sql_trigger *trigger) > sqlite3DbFree(db, trigger); > } >=20 > -/* > - * 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 =E2=80=98_ptr=E2=80=99 suffix. > { > - struct sql_trigger *trigger =3D NULL; > - const char *zName; > - sqlite3 *db =3D pParse->db; > + sqlite3 *db =3D parser->db; > + assert(db->pSchema !=3D NULL); > + struct Vdbe *v =3D sqlite3GetVdbe(parser); > + if (v =3D=3D NULL) > + return; > + /* > + * Generate code to delete entry from _trigger and > + * internal SQL structures. > + */ > + int trig_name_reg =3D ++parser->nMem; > + int record_to_delete =3D ++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 =3D=3D 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 =E2=80=99sql_=E2=80=99 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 =3D TRIGGER_AFTER; > } >=20 > - > - *old_trigger =3D sqlite3HashInsert(hash, name, trigger); > - if (*old_trigger =3D=3D trigger) { > - diag_set(OutOfMemory, sizeof(struct HashElem), > - "sqlite3HashInsert", "ret"); > - return -1; > + struct sql_trigger **ptr =3D &space->sql_triggers; > + while (*ptr !=3D NULL && strcmp((*ptr)->zName, name) !=3D 0) > + ptr =3D &((*ptr)->next); > + if (*ptr !=3D NULL) { > + *old_trigger =3D *ptr; > + *ptr =3D (*ptr)->next; > } >=20 > - if (*old_trigger !=3D NULL) { > - struct sql_trigger **pp; > - for (pp =3D &space->sql_triggers; *pp !=3D *old_trigger; > - pp =3D &((*pp)->next)); > - *pp =3D (*pp)->next; > - } > if (trigger !=3D NULL) { > trigger->next =3D space->sql_triggers; > space->sql_triggers =3D trigger; > } > + Redundant diff.