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 47A9121ECD for ; Tue, 26 Jun 2018 12:13:37 -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 3NzfQCsHHuYj for ; Tue, 26 Jun 2018 12:13:37 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 CA1C621CAA for ; Tue, 26 Jun 2018 12:13:36 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v4 8/8] sql: remove global sql_trigger hash Date: Tue, 26 Jun 2018 19:13:32 +0300 Message-Id: <1d6bcf87b16c4b7c64eb82d3da43adfabede0a7e.1530029141.git.kshcherbatov@tarantool.org> In-Reply-To: <81986d1f9307191bd3d3e37514dc32fadb7e6970.1530029141.git.kshcherbatov@tarantool.org> References: <81986d1f9307191bd3d3e37514dc32fadb7e6970.1530029141.git.kshcherbatov@tarantool.org> In-Reply-To: References: 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: n.pettik@corp.mail.ru, Kirill Shcherbatov As new _trigger format contain space_id, we can do not store AST pointers in HASH. Requested AST could be found by name in appropriate space. Part of #3273. --- src/box/alter.cc | 15 +++-- src/box/errcode.h | 2 +- src/box/sql.h | 3 +- src/box/sql/build.c | 16 +++-- src/box/sql/callback.c | 9 --- src/box/sql/parse.y | 2 +- src/box/sql/sqliteInt.h | 31 ++++++---- src/box/sql/status.c | 8 --- src/box/sql/trigger.c | 132 ++++++++++++++++++----------------------- test/sql-tap/trigger1.test.lua | 4 +- test/sql/persistency.result | 2 +- 11 files changed, 106 insertions(+), 118 deletions(-) diff --git a/src/box/alter.cc b/src/box/alter.cc index 449b4b1..9051d7d 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -3264,6 +3264,7 @@ on_replace_trigger_rollback(struct trigger *trigger, void *event) /* Rollback DELETE trigger. */ if (sql_trigger_replace(sql_get(), sql_trigger_name(old_trigger), + sql_trigger_space_id(old_trigger), old_trigger, &new_trigger) != 0) panic("Out of memory on insertion into trigger hash"); assert(new_trigger == NULL); @@ -3271,6 +3272,7 @@ on_replace_trigger_rollback(struct trigger *trigger, void *event) /* Rollback INSERT trigger. */ int rc = sql_trigger_replace(sql_get(), sql_trigger_name(old_trigger), + sql_trigger_space_id(old_trigger), NULL, &new_trigger); (void)rc; assert(rc == 0); @@ -3280,6 +3282,7 @@ on_replace_trigger_rollback(struct trigger *trigger, void *event) /* Rollback REPLACE trigger. */ if (sql_trigger_replace(sql_get(), sql_trigger_name(old_trigger), + sql_trigger_space_id(old_trigger), old_trigger, &new_trigger) != 0) panic("Out of memory on insertion into trigger hash"); assert(old_trigger != new_trigger); @@ -3322,6 +3325,9 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) const char *trigger_name_src = tuple_field_str_xc(old_tuple, BOX_TRIGGER_FIELD_NAME, &trigger_name_len); + uint32_t space_id = + tuple_field_u32_xc(old_tuple, + BOX_TRIGGER_FIELD_SPACE_ID); char *trigger_name = (char *)region_alloc_xc(&fiber()->gc, trigger_name_len + 1); @@ -3329,8 +3335,8 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) trigger_name[trigger_name_len] = 0; struct sql_trigger *old_trigger; - int rc = sql_trigger_replace(sql_get(), trigger_name, NULL, - &old_trigger); + int rc = sql_trigger_replace(sql_get(), trigger_name, space_id, + NULL, &old_trigger); (void)rc; assert(rc == 0); assert(old_trigger != NULL); @@ -3378,8 +3384,9 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event) } struct sql_trigger *old_trigger; - if (sql_trigger_replace(sql_get(), trigger_name, new_trigger, - &old_trigger) != 0) + if (sql_trigger_replace(sql_get(), trigger_name, + sql_trigger_space_id(new_trigger), + new_trigger, &old_trigger) != 0) diag_raise(); on_commit->data = old_trigger; diff --git a/src/box/errcode.h b/src/box/errcode.h index eb05936..6375764 100644 --- a/src/box/errcode.h +++ b/src/box/errcode.h @@ -86,7 +86,7 @@ struct errcode_record { /* 31 */_(ER_KEY_PART_COUNT, "Invalid key part count (expected [0..%u], got %u)") \ /* 32 */_(ER_PROC_LUA, "%s") \ /* 33 */_(ER_NO_SUCH_PROC, "Procedure '%.*s' is not defined") \ - /* 34 */_(ER_NO_SUCH_TRIGGER, "Trigger is not found") \ + /* 34 */_(ER_NO_SUCH_TRIGGER, "Trigger '%s' doesn't exist") \ /* 35 */_(ER_NO_SUCH_INDEX, "No index #%u is defined in space '%s'") \ /* 36 */_(ER_NO_SUCH_SPACE, "Space '%s' does not exist") \ /* 37 */_(ER_NO_SUCH_FIELD, "Field %d was not found in the tuple") \ diff --git a/src/box/sql.h b/src/box/sql.h index f483921..3ee974e 100644 --- a/src/box/sql.h +++ b/src/box/sql.h @@ -126,6 +126,7 @@ space_trigger_list(uint32_t space_id); * Perform replace trigger in SQL internals with new AST object. * @param db SQL handle. * @param name a name of the trigger. + * @param space_id of the space to insert trigger. * @param trigger AST object to insert. * @param[out] old_trigger Old object if exists. * @@ -133,7 +134,7 @@ space_trigger_list(uint32_t space_id); * @retval -1 on error. */ int -sql_trigger_replace(struct sqlite3 *db, const char *name, +sql_trigger_replace(struct sqlite3 *db, const char *name, uint32_t space_id, struct sql_trigger *trigger, struct sql_trigger **old_trigger); diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 093fea5..59d9656 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -2121,7 +2121,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space, parse_context->nested++; struct sql_trigger *trigger = space->sql_triggers; while (trigger != NULL) { - vdbe_code_drop_trigger_ptr(parse_context, trigger); + vdbe_code_drop_trigger_ptr(parse_context, trigger->zName); trigger = trigger->next; } parse_context->nested--; @@ -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, + int tarantool_error_code, + const char *error_src, bool no_error, + int cond_opcode) +{ + assert(cond_opcode == OP_NoConflict || cond_opcode == OP_Found); struct Vdbe *v = sqlite3GetVdbe(parser); assert(v != NULL); @@ -3996,7 +4000,7 @@ vdbe_emit_halt_if_exists(struct Parse *parser, int space_id, int index_id, int name_reg = parser->nMem++; int label = sqlite3VdbeAddOp4(v, OP_String8, 0, name_reg, 0, name, P4_DYNAMIC); - sqlite3VdbeAddOp4Int(v, OP_NoConflict, cursor, label + 3, name_reg, 1); + sqlite3VdbeAddOp4Int(v, cond_opcode, cursor, label + 3, name_reg, 1); if (no_error) { sqlite3VdbeAddOp0(v, OP_Halt); } else { diff --git a/src/box/sql/callback.c b/src/box/sql/callback.c index c3c38cb..01e8dd8 100644 --- a/src/box/sql/callback.c +++ b/src/box/sql/callback.c @@ -283,18 +283,10 @@ sqlite3SchemaClear(sqlite3 * db) assert(db->pSchema != NULL); Hash temp1; - Hash temp2; HashElem *pElem; Schema *pSchema = db->pSchema; temp1 = pSchema->tblHash; - temp2 = pSchema->trigHash; - sqlite3HashInit(&pSchema->trigHash); - for (pElem = sqliteHashFirst(&temp2); pElem != NULL; - pElem = sqliteHashNext(pElem)) - sql_trigger_delete(NULL, - (struct sql_trigger *)sqliteHashData(pElem)); - sqlite3HashClear(&temp2); sqlite3HashInit(&pSchema->tblHash); for (pElem = sqliteHashFirst(&temp1); pElem; pElem = sqliteHashNext(pElem)) { @@ -317,7 +309,6 @@ sqlite3SchemaCreate(sqlite3 * db) sqlite3OomFault(db); } else { sqlite3HashInit(&p->tblHash); - sqlite3HashInit(&p->trigHash); sqlite3HashInit(&p->fkeyHash); } return p; diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 91bdc6e..2ae0535 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1421,7 +1421,7 @@ raisetype(A) ::= FAIL. {A = ON_CONFLICT_ACTION_FAIL;} //////////////////////// DROP TRIGGER statement ////////////////////////////// cmd ::= DROP TRIGGER ifexists(NOERR) fullname(X). { - sqlite3DropTrigger(pParse,X,NOERR); + vdbe_code_drop_trigger(pParse,X,NOERR); } ////////////////////////// REINDEX collation ////////////////////////////////// diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index d6ae5c2..5f7a0f1 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -1523,7 +1523,6 @@ typedef int VList; struct Schema { int schema_cookie; /* Database schema version number for this file */ Hash tblHash; /* All tables indexed by name */ - Hash trigHash; /* All triggers indexed by name */ Hash fkeyHash; /* All foreign keys by referenced table name */ }; @@ -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); /** * 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) in specified space. * The function allocates error and name resources for VDBE itself. * * @param parser Parsing context. @@ -4795,13 +4802,17 @@ table_column_nullable_action(struct Table *tab, uint32_t column); * @param tarantool_error_code to set on halt. * @param error_src Error message to display on VDBE halt. * @param no_error Do not raise error flag. + * @param cond_opcode Condition to raise - OP_NoConflict or OP_Found. * * @retval -1 on memory allocation error. * @retval 0 on success. */ 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, + int tarantool_error_code, + const char *error_src, bool no_error, + int cond_opcode); #endif /* SQLITEINT_H */ diff --git a/src/box/sql/status.c b/src/box/sql/status.c index 161136c..5bb1f8f 100644 --- a/src/box/sql/status.c +++ b/src/box/sql/status.c @@ -248,18 +248,10 @@ sqlite3_db_status(sqlite3 * db, /* The database connection whose status is desir nByte += ROUND8(sizeof(HashElem)) * (pSchema->tblHash.count + - pSchema->trigHash.count + pSchema->fkeyHash.count); nByte += sqlite3_msize(pSchema->tblHash.ht); - nByte += sqlite3_msize(pSchema->trigHash.ht); nByte += sqlite3_msize(pSchema->fkeyHash.ht); - for (p = sqliteHashFirst(&pSchema->trigHash); p; - p = sqliteHashNext(p)) { - sql_trigger_delete(db, - (struct sql_trigger *) - sqliteHashData(p)); - } for (p = sqliteHashFirst(&pSchema->tblHash); p; p = sqliteHashNext(p)) { sqlite3DeleteTable(db, 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) { - 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) +{ + + sqlite3 *db = parser->db; if (db->mallocFailed) goto drop_trigger_cleanup; @@ -467,68 +482,40 @@ sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr) * here to account DROP TRIGGER IF EXISTS case if the trigger * actually does not exist. */ - if (!pParse->nested) { - Vdbe *v = sqlite3GetVdbe(pParse); + if (!parser->nested) { + Vdbe *v = sqlite3GetVdbe(parser); if (v != NULL) sqlite3VdbeCountChanges(v); } - assert(pName->nSrc == 1); - zName = pName->a[0].zName; - trigger = sqlite3HashFind(&(db->pSchema->trigHash), zName); - if (trigger == NULL) { - if (!noErr) { - sqlite3ErrorMsg(pParse, "no such trigger: %S", pName, - 0); - } - pParse->checkSchema = 1; + assert(name->nSrc == 1); + const char *trigger_name = name->a[0].zName; + const char *error_msg = + tt_sprintf(tnt_errcode_desc(ER_NO_SUCH_TRIGGER), + trigger_name); + if (vdbe_emit_halt_with_presence_test(parser, BOX_TRIGGER_ID, 0, + trigger_name, ER_NO_SUCH_TRIGGER, + error_msg, (no_err != 0), + OP_Found) != 0) goto drop_trigger_cleanup; - } - vdbe_code_drop_trigger_ptr(pParse, trigger); - drop_trigger_cleanup: - sqlite3SrcListDelete(db, pName); -} + vdbe_code_drop_trigger_ptr(parser, trigger_name); -void -vdbe_code_drop_trigger_ptr(struct Parse *parser, struct sql_trigger *trigger) -{ - 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, - trigger->zName, P4_STATIC); - sqlite3VdbeAddOp3(v, OP_MakeRecord, trig_name_reg, 1, - record_to_delete); - sqlite3VdbeAddOp2(v, OP_SDelete, BOX_TRIGGER_ID, - record_to_delete); - if (!parser->nested) - sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE); - - sqlite3ChangeCookie(parser); + drop_trigger_cleanup: + sqlite3SrcListDelete(db, name); } int -sql_trigger_replace(struct sqlite3 *db, const char *name, +sql_trigger_replace(struct sqlite3 *db, const char *name, uint32_t space_id, struct sql_trigger *trigger, struct sql_trigger **old_trigger) { assert(db->pSchema != NULL); assert(trigger == NULL || strcmp(name, trigger->zName) == 0); - struct Hash *hash = &db->pSchema->trigHash; - - struct sql_trigger *src_trigger = - trigger != NULL ? trigger : sqlite3HashFind(hash, name); - assert(src_trigger != NULL); - struct space *space = space_cache_find(src_trigger->space_id); + struct space *space = space_cache_find(space_id); assert(space != NULL); + *old_trigger = NULL; if (trigger != NULL) { /* Do not create a trigger on a system space. */ @@ -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; } + return 0; } diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua index c45fdab..0707c8f 100755 --- a/test/sql-tap/trigger1.test.lua +++ b/test/sql-tap/trigger1.test.lua @@ -159,7 +159,7 @@ test:do_catchsql_test( DROP TRIGGER biggles; ]], { -- - 1, "no such trigger: BIGGLES" + 1, "Trigger 'BIGGLES' doesn't exist" -- }) @@ -170,7 +170,7 @@ test:do_catchsql_test( DROP TRIGGER tr1; ]], { -- - 1, "no such trigger: TR1" + 1, "Trigger 'TR1' doesn't exist" -- }) diff --git a/test/sql/persistency.result b/test/sql/persistency.result index d85d7cc..8f89039 100644 --- a/test/sql/persistency.result +++ b/test/sql/persistency.result @@ -190,7 +190,7 @@ box.sql.execute("DROP TRIGGER tfoobar") -- Should error box.sql.execute("DROP TRIGGER tfoobar") --- -- error: 'no such trigger: TFOOBAR' +- error: Trigger 'TFOOBAR' doesn't exist ... -- Should be empty box.sql.execute("SELECT \"name\", \"opts\" FROM \"_trigger\"") -- 2.7.4