[tarantool-patches] [PATCH v4 8/8] sql: remove global sql_trigger hash
Kirill Shcherbatov
kshcherbatov at tarantool.org
Tue Jun 26 19:13:32 MSK 2018
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;
]], {
-- <trigger1-1.6.2>
- 1, "no such trigger: BIGGLES"
+ 1, "Trigger 'BIGGLES' doesn't exist"
-- </trigger1-1.6.2>
})
@@ -170,7 +170,7 @@ test:do_catchsql_test(
DROP TRIGGER tr1;
]], {
-- <trigger1-1.7>
- 1, "no such trigger: TR1"
+ 1, "Trigger 'TR1' doesn't exist"
-- </trigger1-1.7>
})
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
More information about the Tarantool-patches
mailing list