[tarantool-patches] Re: [PATCH v1 4/4] sql: move Triggers to server
Kirill Shcherbatov
kshcherbatov at tarantool.org
Fri Jun 1 23:25:26 MSK 2018
>From c0775801fcdb475ee6e77dff483f4f6a84c1afc3 Mon Sep 17 00:00:00 2001
Message-Id: <c0775801fcdb475ee6e77dff483f4f6a84c1afc3.1527884700.git.kshcherbatov at gmail.com>
From: Kirill Shcherbatov <kshcherbatov at tarantool.org>
Date: Wed, 30 May 2018 16:03:43 +0300
Subject: [PATCH] sql: move Triggers to server
Introduced sql_triggers field in space structure.
Changed parser logic to do not insert built triggers, just only
to do parsing. All triggers insertions and deletions are operated
via on_replace_dd_trigger now.
Resolves #3273.
---
src/box/alter.cc | 37 ++++-
src/box/errcode.h | 2 +-
src/box/space.h | 2 +
src/box/sql.c | 48 ++----
src/box/sql.h | 44 ++++++
src/box/sql/build.c | 8 +-
src/box/sql/fkey.c | 1 -
src/box/sql/insert.c | 6 +-
src/box/sql/sqliteInt.h | 6 +-
src/box/sql/tokenize.c | 38 ++++-
src/box/sql/trigger.c | 287 +++++++++++++++++-----------------
src/box/sql/vdbe.c | 76 ++-------
src/box/sql/vdbe.h | 1 -
src/box/sql/vdbeaux.c | 9 --
test/sql-tap/identifier_case.test.lua | 4 +-
test/sql-tap/trigger1.test.lua | 4 +-
test/sql/triggers.test.lua | 72 +++++++++
17 files changed, 370 insertions(+), 275 deletions(-)
create mode 100644 test/sql/triggers.test.lua
diff --git a/src/box/alter.cc b/src/box/alter.cc
index f2bf85d..bf170a5 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -551,6 +551,9 @@ 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);
rlist_swap(&new_space->on_stmt_begin, &old_space->on_stmt_begin);
+ /** Copy SQL Triggers pointer. */
+ new_space->sql_triggers = old_space->sql_triggers;
+ old_space->sql_triggers = NULL;
}
/**
@@ -732,7 +735,6 @@ alter_space_commit(struct trigger *trigger, void *event)
}
trigger_run_xc(&on_alter_space, alter->new_space);
-
alter->new_space = NULL; /* for alter_space_delete(). */
/*
* Delete the old version of the space, we are not
@@ -3100,6 +3102,39 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
{
struct txn *txn = (struct txn *) event;
txn_check_singlestatement_xc(txn, "Space _trigger");
+
+ struct txn_stmt *stmt = txn_current_stmt(txn);
+ if (stmt->old_tuple != NULL) {
+ uint32_t trigger_name_len;
+ const char *trigger_name_src =
+ tuple_field_str_xc(stmt->old_tuple, 0,
+ &trigger_name_len);
+ if (sql_trigger_delete_by_name(sql_get(), trigger_name_src,
+ trigger_name_len) != 0)
+ diag_raise();
+ }
+ if (stmt->new_tuple != NULL) {
+ uint32_t trigger_name_len;
+ const char *trigger_name_src =
+ tuple_field_str_xc(stmt->new_tuple, 0,
+ &trigger_name_len);
+ const char *space_opts =
+ tuple_field_with_type_xc(stmt->new_tuple, 1, MP_MAP);
+ struct space_opts opts;
+ struct region *region = &fiber()->gc;
+ space_opts_decode(&opts, space_opts, region);
+ struct Trigger *trigger =
+ sql_trigger_compile(sql_get(), opts.sql);
+ if (trigger == NULL)
+ diag_raise();
+ auto trigger_guard = make_scoped_guard([=] {
+ sql_trigger_delete(sql_get(), trigger);
+ });
+ if (sql_trigger_insert(trigger, trigger_name_src,
+ trigger_name_len) != 0)
+ diag_raise();
+ trigger_guard.is_active = false;
+ }
}
struct trigger alter_space_on_replace_space = {
diff --git a/src/box/errcode.h b/src/box/errcode.h
index ba52880..eb05936 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -107,7 +107,7 @@ struct errcode_record {
/* 52 */_(ER_FUNCTION_EXISTS, "Function '%s' already exists") \
/* 53 */_(ER_BEFORE_REPLACE_RET, "Invalid return value of space:before_replace trigger: expected tuple or nil, got %s") \
/* 54 */_(ER_FUNCTION_MAX, "A limit on the total number of functions has been reached: %u") \
- /* 55 */_(ER_UNUSED4, "") \
+ /* 55 */_(ER_TRIGGER_EXISTS, "Trigger '%s' already exists") \
/* 56 */_(ER_USER_MAX, "A limit on the total number of users has been reached: %u") \
/* 57 */_(ER_NO_SUCH_ENGINE, "Space engine '%s' does not exist") \
/* 58 */_(ER_RELOAD_CFG, "Can't set option '%s' dynamically") \
diff --git a/src/box/space.h b/src/box/space.h
index b8d29ca..0413cd0 100644
--- a/src/box/space.h
+++ b/src/box/space.h
@@ -146,6 +146,8 @@ struct space {
struct rlist on_replace;
/** Triggers fired before space statement */
struct rlist on_stmt_begin;
+ /** SQL Trigger list. */
+ struct Trigger *sql_triggers;
/**
* The number of *enabled* indexes in the space.
*
diff --git a/src/box/sql.c b/src/box/sql.c
index 0eded6f..6ecb226 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1223,9 +1223,6 @@ space_foreach_put_cb(struct space *space, void *udata)
/* Load database schema from Tarantool. */
void tarantoolSqlite3LoadSchema(InitData *init)
{
- box_iterator_t *it;
- box_tuple_t *tuple;
-
sql_schema_put(
init, TARANTOOL_SYS_SCHEMA_NAME,
BOX_SCHEMA_ID, 0,
@@ -1294,42 +1291,6 @@ void tarantoolSqlite3LoadSchema(InitData *init)
init->rc = SQL_TARANTOOL_ERROR;
return;
}
-
- /* Read _trigger */
- it = box_index_iterator(BOX_TRIGGER_ID, 0, ITER_GE,
- nil_key, nil_key + sizeof(nil_key));
-
- if (it == NULL) {
- init->rc = SQL_TARANTOOL_ITERATOR_FAIL;
- return;
- }
-
- while (box_iterator_next(it, &tuple) == 0 && tuple != NULL) {
- const char *field, *ptr;
- char *name, *sql;
- unsigned len;
- assert(tuple_field_count(tuple) == 2);
-
- field = tuple_field(tuple, 0);
- assert (field != NULL);
- ptr = mp_decode_str(&field, &len);
- name = strndup(ptr, len);
-
- field = tuple_field(tuple, 1);
- assert (field != NULL);
- mp_decode_array(&field);
- ptr = mp_decode_str(&field, &len);
- assert (strncmp(ptr, "sql", 3) == 0);
-
- ptr = mp_decode_str(&field, &len);
- sql = strndup(ptr, len);
-
- sql_schema_put(init, name, 0, 0, sql);
-
- free(name);
- free(sql);
- }
- box_iterator_free(it);
}
/*********************************************************************
@@ -1737,6 +1698,15 @@ space_column_default_expr(uint32_t space_id, uint32_t fieldno)
return space->def->fields[fieldno].default_value_expr;
}
+struct 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;
+}
+
struct space_def *
sql_ephemeral_space_def_new(Parse *parser, const char *name)
{
diff --git a/src/box/sql.h b/src/box/sql.h
index 35aacc3..f4d909f 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -84,6 +84,17 @@ struct Expr *
sql_expr_compile(struct sqlite3 *db, const char *expr, int expr_len);
/**
+ * Perform parsing of provided SQL request and construct trigger AST.
+ * @param db SQL context handle.
+ * @param sql request to parse.
+ *
+ * @retval NULL on error
+ * @retval not NULL Trigger AST pointer on success.
+ */
+struct Trigger *
+sql_trigger_compile(struct sqlite3 *db, const char *sql);
+
+/**
* Free AST pointed by trigger.
* @param db SQL handle.
* @param trigger AST object.
@@ -92,6 +103,39 @@ void
sql_trigger_delete(struct sqlite3 *db, struct Trigger *trigger);
/**
+ * Remove a trigger from the hash tables of the sqlite* pointer.
+ * @param db SQL handle.
+ * @param name a name of trigger object to delete.
+ * @param name_len length of the name.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_trigger_delete_by_name(struct sqlite3 *db, const char *name, int name_len);
+
+/**
+ * Get server triggers list by space_id.
+ * @param space_id Space ID.
+ *
+ * @retval trigger AST list.
+ */
+struct Trigger *
+space_trigger_list(uint32_t space_id);
+
+/**
+ * Perform insert trigger in appropriate space.
+ * @param trigger object to append.
+ * @param name a name of trigger object to insert.
+ * @param name_len length of the name.
+ *
+ * @retval 0 on success.
+ * @retval -1 on error.
+ */
+int
+sql_trigger_insert(struct Trigger *trigger, const char *name, int name_len);
+
+/**
* Store duplicate of a parsed expression into @a parser.
* @param parser Parser context.
* @param select Select to extract from.
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 28e4d7a..40f693b 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -2289,16 +2289,14 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
/*
* Drop all triggers associated with the table being
* dropped. Code is generated to remove entries from
- * _trigger. OP_DropTrigger will remove it from internal
- * SQL structures.
+ * _trigger. on_replace_dd_trigger will remove it from
+ * internal SQL structures.
*
* Do not account triggers deletion - they will be
* accounted in DELETE from _space below.
*/
parse_context->nested++;
- Table *table = sqlite3HashFind(&parse_context->db->pSchema->tblHash,
- space->def->name);
- struct Trigger *trigger = table->pTrigger;
+ struct Trigger *trigger = space->sql_triggers;
while (trigger != NULL) {
sqlite3DropTriggerPtr(parse_context, trigger);
trigger = trigger->pNext;
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 70ebef8..d5585cf 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -1440,7 +1440,6 @@ fkActionTrigger(Parse * pParse, /* Parse context */
}
pStep->pTrig = pTrigger;
pTrigger->pSchema = pTab->pSchema;
- pTrigger->pTabSchema = pTab->pSchema;
pFKey->apTrigger[iAction] = pTrigger;
pTrigger->op = (pChanges ? TK_UPDATE : TK_DELETE);
}
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 59c61c7..023e6b0 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1766,9 +1766,9 @@ xferOptimization(Parse * pParse, /* Parser context */
*/
return 0;
}
- if (pDest->pTrigger) {
- return 0; /* tab1 must not have triggers */
- }
+ /* The pDest must not have triggers. */
+ if (space_trigger_list(pDest->def->id) != NULL)
+ return 0;
if (onError == ON_CONFLICT_ACTION_DEFAULT) {
if (pDest->iPKey >= 0)
onError = pDest->keyConf;
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ecbd573..4c07480 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1935,7 +1935,6 @@ struct Table {
#ifndef SQLITE_OMIT_ALTERTABLE
int addColOffset; /* Offset in CREATE TABLE stmt to add a new column */
#endif
- Trigger *pTrigger; /* List of triggers stored in pSchema */
Schema *pSchema; /* Schema that contains this table */
Table *pNextZombie; /* Next on the Parse.pZombieTab list */
/** Space definition with Tarantool metadata. */
@@ -3032,14 +3031,14 @@ struct Parse {
*/
struct Trigger {
char *zName; /* The name of the trigger */
- char *table; /* The table or view to which the trigger applies */
+ /** The ID of space the triggers is refer to. */
+ uint32_t space_id;
u8 op; /* One of TK_DELETE, TK_UPDATE, TK_INSERT */
u8 tr_tm; /* One of TRIGGER_BEFORE, TRIGGER_AFTER */
Expr *pWhen; /* The WHEN clause of the expression (may be NULL) */
IdList *pColumns; /* If this is an UPDATE OF <column-list> trigger,
the <column-list> is stored here */
Schema *pSchema; /* Schema containing the trigger */
- Schema *pTabSchema; /* Schema containing the table */
TriggerStep *step_list; /* Link list of trigger program steps */
Trigger *pNext; /* Next trigger associated with the table */
};
@@ -4027,7 +4026,6 @@ TriggerStep *sqlite3TriggerInsertStep(sqlite3 *, Token *, IdList *,
TriggerStep *sqlite3TriggerUpdateStep(sqlite3 *, Token *, ExprList *, Expr *,
u8);
TriggerStep *sqlite3TriggerDeleteStep(sqlite3 *, Token *, Expr *);
-void sqlite3UnlinkAndDeleteTrigger(sqlite3 *, const char *);
u32 sqlite3TriggerColmask(Parse *, Trigger *, ExprList *, int, int, Table *,
int);
#define sqlite3ParseToplevel(p) ((p)->pToplevel ? (p)->pToplevel : (p))
diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
index 9b65064..0c851c9 100644
--- a/src/box/sql/tokenize.c
+++ b/src/box/sql/tokenize.c
@@ -39,6 +39,7 @@
#include <stdlib.h>
#include <unicode/utf8.h>
#include <unicode/uchar.h>
+#include "box/schema.h"
#include "say.h"
#include "sqliteInt.h"
@@ -122,6 +123,7 @@ static const char sql_ascii_class[] = {
* the #include below.
*/
#include "keywordhash.h"
+#include "tarantoolInt.h"
#define maybe_utf8(c) ((sqlite3CtypeMap[c] & 0x40) != 0)
@@ -510,8 +512,13 @@ sqlite3RunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
}
if (pParse->rc != SQLITE_OK && pParse->rc != SQLITE_DONE
&& pParse->zErrMsg == 0) {
- pParse->zErrMsg =
- sqlite3MPrintf(db, "%s", sqlite3ErrStr(pParse->rc));
+ const char *error;
+ if (is_tarantool_error(pParse->rc) && tarantoolErrorMessage()) {
+ error = tarantoolErrorMessage();
+ } else {
+ error = sqlite3ErrStr(pParse->rc);
+ }
+ pParse->zErrMsg = sqlite3MPrintf(db, "%s", error);
}
assert(pzErrMsg != 0);
if (pParse->zErrMsg) {
@@ -528,7 +535,12 @@ sqlite3RunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
if (pParse->pWithToFree)
sqlite3WithDelete(db, pParse->pWithToFree);
- sql_trigger_delete(db, pParse->pNewTrigger);
+ /*
+ * Trigger is exporting with pNewTrigger field when
+ * parse_only flag is set.
+ */
+ if (!pParse->parse_only)
+ sql_trigger_delete(db, pParse->pNewTrigger);
sqlite3DbFree(db, pParse->pVList);
while (pParse->pZombieTab) {
Table *p = pParse->pZombieTab;
@@ -569,3 +581,23 @@ cleanup:
sql_parser_destroy(&parser);
return expression;
}
+
+struct Trigger *
+sql_trigger_compile(struct sqlite3 *db, const char *sql)
+{
+ struct Parse parser;
+ sql_parser_create(&parser, db);
+ parser.parse_only = true;
+ char *sql_error;
+ struct Trigger *trigger = NULL;
+ if (sqlite3RunParser(&parser, sql, &sql_error) != SQLITE_OK) {
+ char *error = tt_static_buf();
+ snprintf(error, TT_STATIC_BUF_LEN, "%s", sql_error);
+ diag_set(ClientError, ER_SQL_EXECUTE, sql);
+ goto cleanup;
+ }
+ trigger = parser.pNewTrigger;
+cleanup:
+ sql_parser_destroy(&parser);
+ return trigger;
+}
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 053dadb..beaad11 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -33,6 +33,8 @@
* This file contains the implementation for TRIGGERs
*/
+#include "box/box.h"
+#include "box/schema.h"
#include "sqliteInt.h"
#include "tarantoolInt.h"
#include "vdbeInt.h"
@@ -81,102 +83,121 @@ sqlite3BeginTrigger(Parse * pParse, /* The parse context of the CREATE TRIGGER s
)
{
Trigger *pTrigger = 0; /* The new trigger */
- Table *pTab; /* Table that the trigger fires off of */
+ /* Table that the trigger fires off of. */
+ struct Table *table = NULL;
char *zName = 0; /* Name of the trigger */
sqlite3 *db = pParse->db; /* The database connection */
DbFixer sFix; /* State vector for the DB fixer */
- /* Do not account nested operations: the count of such
- * operations depends on Tarantool data dictionary internals,
- * such as data layout in system spaces.
+ /*
+ * Do not account nested operations: the count of such
+ * operations depends on Tarantool data dictionary
+ * internals, such as data layout in system spaces.
*/
if (!pParse->nested) {
Vdbe *v = sqlite3GetVdbe(pParse);
if (v != NULL)
sqlite3VdbeCountChanges(v);
}
- assert(pName != 0); /* pName->z might be NULL, but not pName itself */
+ /* pName->z might be NULL, but not pName itself. */
+ assert(pName != 0);
assert(op == TK_INSERT || op == TK_UPDATE || op == TK_DELETE);
assert(op > 0 && op < 0xff);
- if (!pTableName || db->mallocFailed) {
+ if (!pTableName || db->mallocFailed)
goto trigger_cleanup;
- }
- /* Ensure the table name matches database name and that the table exists */
+ /*
+ * Ensure the table name matches database name and that
+ * the table exists.
+ */
if (db->mallocFailed)
goto trigger_cleanup;
assert(pTableName->nSrc == 1);
sqlite3FixInit(&sFix, pParse, "trigger", pName);
- if (sqlite3FixSrcList(&sFix, pTableName)) {
+ if (sqlite3FixSrcList(&sFix, pTableName))
goto trigger_cleanup;
- }
- pTab = sql_list_lookup_table(pParse, pTableName);
- if (!pTab) {
- goto trigger_cleanup;
- }
- /* Check that the trigger name is not reserved and that no trigger of the
- * specified name exists
- */
zName = sqlite3NameFromToken(db, pName);
- if (!zName || SQLITE_OK != sqlite3CheckIdentifierName(pParse, zName)) {
- goto trigger_cleanup;
- }
- if (sqlite3HashFind(&(db->pSchema->trigHash), zName)) {
- if (!noErr) {
- sqlite3ErrorMsg(pParse, "trigger %s already exists",
- zName);
- } else {
- assert(!db->init.busy);
- }
+ if (zName == NULL)
goto trigger_cleanup;
- }
- /* Do not create a trigger on a system table */
- if (sqlite3StrNICmp(pTab->def->name, "sqlite_", 7) == 0) {
- sqlite3ErrorMsg(pParse,
- "cannot create trigger on system table");
+ if (sqlite3CheckIdentifierName(pParse, zName) != SQLITE_OK)
goto trigger_cleanup;
- }
- /* INSTEAD of triggers are only for views and views only support INSTEAD
- * of triggers.
- */
- if (space_is_view(pTab) && tr_tm != TK_INSTEAD) {
- sqlite3ErrorMsg(pParse, "cannot create %s trigger on view: %S",
- (tr_tm == TK_BEFORE) ? "BEFORE" : "AFTER",
- pTableName, 0);
- goto trigger_cleanup;
- }
- if (!space_is_view(pTab) && tr_tm == TK_INSTEAD) {
- sqlite3ErrorMsg(pParse, "cannot create INSTEAD OF"
- " trigger on table: %S", pTableName, 0);
- goto trigger_cleanup;
+ /* FIXME: Move all checks in VDBE #3435. */
+ if (!pParse->parse_only) {
+ table = sql_list_lookup_table(pParse, pTableName);
+ if (table == NULL)
+ goto trigger_cleanup;
+
+ if (sqlite3HashFind(&db->pSchema->trigHash, zName)) {
+ if (!noErr) {
+ diag_set(ClientError, ER_TRIGGER_EXISTS, zName);
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ } else {
+ assert(!db->init.busy);
+ }
+ goto trigger_cleanup;
+ }
+
+ /* Do not create a trigger on a system table. */
+ if (sqlite3StrNICmp(table->def->name, "sqlite_", 7) == 0) {
+ sqlite3ErrorMsg(pParse,
+ "cannot create trigger on system table");
+ goto trigger_cleanup;
+ }
+
+ /*
+ * INSTEAD of triggers are only for views and
+ * views only support INSTEAD of triggers.
+ */
+ if (table->def->opts.is_view && tr_tm != TK_INSTEAD) {
+ sqlite3ErrorMsg(pParse,
+ "cannot create %s trigger on view: %S",
+ (tr_tm == TK_BEFORE) ?
+ "BEFORE" : "AFTER",
+ pTableName, 0);
+ goto trigger_cleanup;
+ }
+ if (!table->def->opts.is_view && tr_tm == TK_INSTEAD) {
+ sqlite3ErrorMsg(pParse,
+ "cannot create INSTEAD OF trigger "
+ "on table: %S", pTableName, 0);
+ goto trigger_cleanup;
+ }
}
- /* INSTEAD OF triggers can only appear on views and BEFORE triggers
+ /*
+ * INSTEAD OF triggers can only appear on views and BEFORE triggers
* cannot appear on views. So we might as well translate every
* INSTEAD OF trigger into a BEFORE trigger. It simplifies code
* elsewhere.
*/
- if (tr_tm == TK_INSTEAD) {
+ if (tr_tm == TK_INSTEAD)
tr_tm = TK_BEFORE;
- }
- /* Build the Trigger object */
+ /* Build the Trigger object. */
pTrigger = (Trigger *) sqlite3DbMallocZero(db, sizeof(Trigger));
if (pTrigger == 0)
goto trigger_cleanup;
pTrigger->zName = zName;
zName = 0;
- pTrigger->table = sqlite3DbStrDup(db, pTableName->a[0].zName);
+ /* Initialize space_id on trigger insert. */
+ if (schema_find_id(BOX_SPACE_ID, 2, pTableName->a[0].zName,
+ strlen(pTableName->a[0].zName),
+ &pTrigger->space_id) != 0) {
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ goto trigger_cleanup;
+ }
pTrigger->pSchema = db->pSchema;
- pTrigger->pTabSchema = pTab->pSchema;
pTrigger->op = (u8) op;
pTrigger->tr_tm = tr_tm == TK_BEFORE ? TRIGGER_BEFORE : TRIGGER_AFTER;
pTrigger->pWhen = sqlite3ExprDup(db, pWhen, EXPRDUP_REDUCE);
pTrigger->pColumns = sqlite3IdListDup(db, pColumns);
+ if ((pWhen != NULL && pTrigger->pWhen == NULL) ||
+ (pColumns != NULL && pTrigger->pColumns == NULL))
+ goto trigger_cleanup;
assert(pParse->pNewTrigger == 0);
pParse->pNewTrigger = pTrigger;
@@ -210,7 +231,7 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
DbFixer sFix; /* Fixer object */
Token nameToken; /* Trigger name for error reporting */
- pParse->pNewTrigger = 0;
+ pParse->pNewTrigger = NULL;
if (NEVER(pParse->nErr) || !pTrig)
goto triggerfinish_cleanup;
zName = pTrig->zName;
@@ -227,10 +248,11 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
goto triggerfinish_cleanup;
}
- /* if we are not initializing,
- * generate byte code to insert a new trigger into Tarantool.
+ /*
+ * Generate byte code to insert a new trigger into
+ * Tarantool for non-parsig mode or export trigger.
*/
- if (!db->init.busy) {
+ if (!pParse->parse_only) {
Vdbe *v;
int zOptsSz;
Table *pSysTrigger;
@@ -288,37 +310,11 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
sqlite3VdbeAddOp1(v, OP_Close, iCursor);
- /* parseschema3(reg(iFirstCol), ref(iFirstCol)+1) */
- iFirstCol = pParse->nMem + 1;
- pParse->nMem += 2;
-
sql_set_multi_write(pParse, false);
- sqlite3VdbeAddOp4(v,
- OP_String8, 0, iFirstCol, 0,
- zName, P4_STATIC);
-
- sqlite3VdbeAddOp4(v,
- OP_String8, 0, iFirstCol + 1, 0,
- zSql, P4_DYNAMIC);
sqlite3ChangeCookie(pParse);
- sqlite3VdbeAddParseSchema3Op(v, iFirstCol);
- }
-
- if (db->init.busy) {
- Trigger *pLink = pTrig;
- Hash *pHash = &db->pSchema->trigHash;
- pTrig = sqlite3HashInsert(pHash, zName, pTrig);
- if (pTrig) {
- sqlite3OomFault(db);
- } else if (pLink->pSchema == pLink->pTabSchema) {
- Table *pTab;
- pTab =
- sqlite3HashFind(&pLink->pTabSchema->tblHash,
- pLink->table);
- assert(pTab != 0);
- pLink->pNext = pTab->pTrigger;
- pTab->pTrigger = pLink;
- }
+ } else {
+ pParse->pNewTrigger = pTrig;
+ pTrig = NULL;
}
triggerfinish_cleanup:
@@ -329,7 +325,7 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
alloc for it either wasn't called at all or failed. */
}
sql_trigger_delete(db, pTrig);
- assert(!pParse->pNewTrigger);
+ assert(!pParse->pNewTrigger || pParse->parse_only);
sqlite3DeleteTriggerStep(db, pStepList);
}
@@ -478,7 +474,6 @@ sql_trigger_delete(struct sqlite3 *db, struct Trigger *trigger)
return;
sqlite3DeleteTriggerStep(db, trigger->step_list);
sqlite3DbFree(db, trigger->zName);
- sqlite3DbFree(db, trigger->table);
sql_expr_delete(db, trigger->pWhen, false);
sqlite3IdListDelete(db, trigger->pColumns);
sqlite3DbFree(db, trigger);
@@ -533,16 +528,6 @@ sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr)
}
/*
- * Return a pointer to the Table structure for the table that a trigger
- * is set on.
- */
-static Table *
-tableOfTrigger(Trigger * pTrigger)
-{
- return sqlite3HashFind(&pTrigger->pTabSchema->tblHash, pTrigger->table);
-}
-
-/*
* Drop a trigger given a pointer to that trigger.
*/
void
@@ -565,34 +550,68 @@ sqlite3DropTriggerPtr(Parse * pParse, Trigger * pTrigger)
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
sqlite3ChangeCookie(pParse);
- sqlite3VdbeAddOp4(v, OP_DropTrigger, 0, 0, 0, pTrigger->zName,
- 0);
}
}
-/*
- * Remove a trigger from the hash tables of the sqlite* pointer.
- */
-void
-sqlite3UnlinkAndDeleteTrigger(sqlite3 * db, const char *zName)
+int
+sql_trigger_delete_by_name(struct sqlite3 *db, const char *name, int name_len)
{
- Trigger *pTrigger;
- Hash *pHash;
- struct session *user_session = current_session();
+ /* Name from Tarantool tuple may be not normalized. */
+ uint32_t used = region_used(&fiber()->gc);
+ char *trigger_name = region_alloc(&fiber()->gc, name_len + 3);
+ if (trigger_name == NULL) {
+ diag_set(OutOfMemory, name_len + 3, "region_alloc",
+ "trigger_name");
+ return -1;
+ }
+ memcpy(trigger_name, name, name_len);
+ trigger_name[name_len] = 0;
+
+ struct Hash *hash = &(db->pSchema->trigHash);
+ struct Trigger *trigger = sqlite3HashInsert(hash, trigger_name, NULL);
+ assert(trigger != NULL);
+
+ struct space *space = space_by_id(trigger->space_id);
+ /* Space could be already deleted. */
+ if (space != NULL) {
+ struct Trigger **pp;
+ for (pp = &space->sql_triggers;
+ *pp != trigger;
+ pp = &((*pp)->pNext));
+ *pp = (*pp)->pNext;
+ }
- pHash = &(db->pSchema->trigHash);
- pTrigger = sqlite3HashInsert(pHash, zName, 0);
- if (ALWAYS(pTrigger)) {
- if (pTrigger->pSchema == pTrigger->pTabSchema) {
- Table *pTab = tableOfTrigger(pTrigger);
- Trigger **pp;
- for (pp = &pTab->pTrigger; *pp != pTrigger;
- pp = &((*pp)->pNext)) ;
- *pp = (*pp)->pNext;
- }
- sql_trigger_delete(db, pTrigger);
- user_session->sql_flags |= SQLITE_InternChanges;
+ sql_trigger_delete(db, trigger);
+ region_truncate(&fiber()->gc, used);
+ return 0;
+}
+
+int
+sql_trigger_insert(struct Trigger *trigger, const char *name, int name_len)
+{
+ if (strncmp(name, trigger->zName, name_len) != 0) {
+ diag_set(ClientError, ER_SQL,
+ "tuple trigger name does not match extracted from SQL");
+ return -1;
+ }
+
+ struct space *space = space_cache_find(trigger->space_id);
+ assert(space != NULL);
+ struct Hash *hash = &trigger->pSchema->trigHash;
+ if (sqlite3HashFind(hash, trigger->zName) != NULL) {
+ diag_set(ClientError, ER_TRIGGER_EXISTS, trigger->zName);
+ return -1;
+ }
+
+ void *ret = sqlite3HashInsert(hash, trigger->zName, trigger);
+ if (ret == trigger) {
+ diag_set(OutOfMemory, 0, "sqlite3HashInsert", "ret");
+ return -1;
}
+ assert(ret == NULL);
+ trigger->pNext = space->sql_triggers;
+ space->sql_triggers = trigger;
+ return 0;
}
/*
@@ -631,22 +650,17 @@ sqlite3TriggersExist(Table * pTab, /* The table the contains the triggers */
)
{
int mask = 0;
- Trigger *pList = 0;
- Trigger *p;
+ struct Trigger *trigger_list = NULL;
struct session *user_session = current_session();
-
- if ((user_session->sql_flags & SQLITE_EnableTrigger) != 0) {
- pList = pTab->pTrigger;
- }
- for (p = pList; p; p = p->pNext) {
- if (p->op == op && checkColumnOverlap(p->pColumns, pChanges)) {
+ if ((user_session->sql_flags & SQLITE_EnableTrigger) != 0)
+ trigger_list = space_trigger_list(pTab->def->id);
+ for (struct Trigger *p = trigger_list; p; p = p->pNext) {
+ if (p->op == op && checkColumnOverlap(p->pColumns, pChanges))
mask |= p->tr_tm;
- }
}
- if (pMask) {
+ if (pMask)
*pMask = mask;
- }
- return (mask ? pList : 0);
+ return mask ? trigger_list : 0;
}
/*
@@ -835,7 +849,7 @@ codeRowTrigger(Parse * pParse, /* Current parse context */
Parse *pSubParse; /* Parse context for sub-vdbe */
int iEndTrigger = 0; /* Label to jump to if WHEN is false */
- assert(pTrigger->zName == 0 || pTab == tableOfTrigger(pTrigger));
+ assert(pTrigger->zName == 0 || pTab->def->id == pTrigger->space_id);
assert(pTop->pVdbe);
/* Allocate the TriggerPrg and SubProgram objects. To ensure that they
@@ -952,7 +966,7 @@ getRowTrigger(Parse * pParse, /* Current parse context */
Parse *pRoot = sqlite3ParseToplevel(pParse);
TriggerPrg *pPrg;
- assert(pTrigger->zName == 0 || pTab == tableOfTrigger(pTrigger));
+ assert(pTrigger->zName == 0 || pTab->def->id == pTrigger->space_id);
/* It may be that this trigger has already been coded (or is in the
* process of being coded). If this is the case, then an entry with
@@ -1077,14 +1091,7 @@ sqlite3CodeRowTrigger(Parse * pParse, /* Parse context */
assert((op == TK_UPDATE) == (pChanges != 0));
for (p = pTrigger; p; p = p->pNext) {
-
- /* Sanity checking: The schema for the trigger and for the table are
- * always defined. The trigger must be in the same schema as the table
- * or else it must be a TEMP trigger.
- */
assert(p->pSchema != 0);
- assert(p->pTabSchema != 0);
- assert(p->pSchema == p->pTabSchema);
/* Determine whether we should code this trigger */
if (p->op == op
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 3fe5875..bba9bf1 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4686,46 +4686,6 @@ case OP_ParseSchema2: {
break;
}
-/* Opcode: ParseSchema3 P1 * * * *
- * Synopsis: name=r[P1] sql=r[P1+1]
- *
- * Create trigger named r[P1] w/ DDL SQL stored in r[P1+1]
- * in database P2
- */
-case OP_ParseSchema3: {
- InitData initData;
- Mem *pRec;
- char zPgnoBuf[16];
- char *argv[4] = {NULL, zPgnoBuf, NULL, NULL};
- assert(db->pSchema != NULL);
-
- initData.db = db;
- initData.pzErrMsg = &p->zErrMsg;
-
- assert(db->init.busy==0);
- db->init.busy = 1;
- initData.rc = SQLITE_OK;
- assert(!db->mallocFailed);
-
- pRec = &aMem[pOp->p1];
- argv[0] = pRec[0].z;
- argv[1] = "0";
- argv[2] = pRec[1].z;
- sqlite3InitCallback(&initData, 3, argv, NULL);
-
- rc = initData.rc;
- db->init.busy = 0;
-
- if (rc) {
- sqlite3ResetAllSchemasOfConnection(db);
- if (rc==SQLITE_NOMEM) {
- goto no_mem;
- }
- goto abort_due_to_error;
- }
- break;
-}
-
/* Opcode: RenameTable P1 * * P4 *
* Synopsis: P1 = root, P4 = name
*
@@ -4745,7 +4705,6 @@ case OP_RenameTable: {
const char *zNewTableName;
Table *pTab;
FKey *pFKey;
- Trigger *pTrig;
int iRootPage;
InitData initData;
char *argv[4] = {NULL, NULL, NULL, NULL};
@@ -4758,7 +4717,6 @@ case OP_RenameTable: {
assert(zOldTableName);
pTab = sqlite3HashFind(&db->pSchema->tblHash, zOldTableName);
assert(pTab);
- pTrig = pTab->pTrigger;
iRootPage = pTab->tnum;
zNewTableName = pOp->p4.z;
zOldTableName = sqlite3DbStrNDup(db, zOldTableName,
@@ -4799,19 +4757,21 @@ case OP_RenameTable: {
goto abort_due_to_error;
}
- pTab = sqlite3HashFind(&db->pSchema->tblHash, zNewTableName);
- pTab->pTrigger = pTrig;
+ space = space_by_id(space_id);
+ assert(space != NULL);
- /* Rename all trigger created on this table.*/
- for (; pTrig; pTrig = pTrig->pNext) {
- sqlite3DbFree(db, pTrig->table);
- pTrig->table = sqlite3DbStrNDup(db, zNewTableName,
- sqlite3Strlen30(zNewTableName));
- pTrig->pTabSchema = pTab->pSchema;
- rc = tarantoolSqlite3RenameTrigger(pTrig->zName,
+ /* Rename all trigger created on this table. */
+ for (struct Trigger *trigger = space->sql_triggers; trigger != NULL; ) {
+ struct Trigger *next_trigger = trigger->pNext;
+ rc = tarantoolSqlite3RenameTrigger(trigger->zName,
zOldTableName, zNewTableName);
- if (rc) goto abort_due_to_error;
+ if (rc != SQLITE_OK) {
+ sqlite3ResetAllSchemasOfConnection(db);
+ goto abort_due_to_error;
+ }
+ trigger = next_trigger;
}
+
sqlite3DbFree(db, (void*)zOldTableName);
sqlite3DbFree(db, (void*)zSqlStmt);
break;
@@ -4857,18 +4817,6 @@ case OP_DropIndex: {
break;
}
-/* Opcode: DropTrigger P1 * * P4 *
- *
- * Remove the internal (in-memory) data structures that describe
- * the trigger named P4 in database P1. This is called after a trigger
- * is dropped from disk (using the Destroy opcode) in order to keep
- * the internal representation of the
- * schema consistent with what is on disk.
- */
-case OP_DropTrigger: {
- sqlite3UnlinkAndDeleteTrigger(db, pOp->p4.z);
- break;
-}
#ifndef SQLITE_OMIT_TRIGGER
/* Opcode: Program P1 P2 P3 P4 P5
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 68d542c..77fa41a 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -238,7 +238,6 @@ void sqlite3VdbeVerifyNoResultRow(Vdbe * p);
VdbeOp *sqlite3VdbeAddOpList(Vdbe *, int nOp, VdbeOpList const *aOp,
int iLineno);
void sqlite3VdbeAddParseSchema2Op(Vdbe * p, int, int);
-void sqlite3VdbeAddParseSchema3Op(Vdbe * p, int);
void sqlite3VdbeAddRenameTableOp(Vdbe * p, int, char *);
void sqlite3VdbeChangeOpcode(Vdbe *, u32 addr, u8);
void sqlite3VdbeChangeP1(Vdbe *, u32 addr, int P1);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index 679bd0b..7b3c13d 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -410,15 +410,6 @@ sqlite3VdbeAddParseSchema2Op(Vdbe * p, int iRec, int n)
sqlite3VdbeAddOp3(p, OP_ParseSchema2, iRec, n, 0);
}
-/*
- * Add an OP_ParseSchema3 opcode which in turn will create a trigger
- */
-void
-sqlite3VdbeAddParseSchema3Op(Vdbe * p, int iRec)
-{
- sqlite3VdbeAddOp2(p, OP_ParseSchema3, iRec, 0);
-}
-
void
sqlite3VdbeAddRenameTableOp(Vdbe * p, int iTab, char* zNewName)
{
diff --git a/test/sql-tap/identifier_case.test.lua b/test/sql-tap/identifier_case.test.lua
index 3f6dc07..5e7573a 100755
--- a/test/sql-tap/identifier_case.test.lua
+++ b/test/sql-tap/identifier_case.test.lua
@@ -170,8 +170,8 @@ test:do_execsql_test(
data = {
{ 1, [[ trigger1 ]], {0}},
- { 2, [[ Trigger1 ]], {1, "trigger TRIGGER1 already exists"}},
- { 3, [["TRIGGER1"]], {1, "trigger TRIGGER1 already exists"}},
+ { 2, [[ Trigger1 ]], {1, "Trigger 'TRIGGER1' already exists"}},
+ { 3, [["TRIGGER1"]], {1, "Trigger 'TRIGGER1' already exists"}},
{ 4, [["trigger1" ]], {0}}
}
diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
index 40daba4..ed6ee36 100755
--- a/test/sql-tap/trigger1.test.lua
+++ b/test/sql-tap/trigger1.test.lua
@@ -101,7 +101,7 @@ test:do_catchsql_test(
END
]], {
-- <trigger1-1.2.1>
- 1, "trigger TR1 already exists"
+ 1, "Trigger 'TR1' already exists"
-- </trigger1-1.2.1>
})
@@ -113,7 +113,7 @@ test:do_catchsql_test(
END
]], {
-- <trigger1-1.2.2>
- 1, [[trigger TR1 already exists]]
+ 1, [[Trigger 'TR1' already exists]]
-- </trigger1-1.2.2>
})
diff --git a/test/sql/triggers.test.lua b/test/sql/triggers.test.lua
new file mode 100644
index 0000000..23bdcfd
--- /dev/null
+++ b/test/sql/triggers.test.lua
@@ -0,0 +1,72 @@
+env = require('test_run')
+test_run = env.new()
+test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua...\"]:<line>: '")
+
+--
+-- gh-3273: Move Triggers to server
+--
+
+box.sql.execute("CREATE TABLE t1(x INTEGER PRIMARY KEY);")
+box.sql.execute("CREATE TABLE t2(x INTEGER PRIMARY KEY);")
+box.sql.execute([[CREATE TRIGGER t1t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(1); END; ]])
+box.space._trigger:select()
+
+-- checks for LUA tuples
+tuple = {"T1T", {sql = "CREATE TRIGGER t1t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(1); END;"}}
+box.space._trigger:insert(tuple)
+
+tuple = {"T1t", {sql = "CREATE TRIGGER t1t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(1); END;"}}
+box.space._trigger:insert(tuple)
+
+tuple = {"T1T", {sql = "CREATE TRIGGER t12t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(1); END;"}}
+box.space._trigger:insert(tuple)
+
+tuple = {"T1t", {sql = "CREATE TRIGGER t12t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(1); END;"}}
+box.space._trigger:insert(tuple)
+
+box.space._trigger:select()
+
+
+-- test, didn't trigger t1t degrade
+box.sql.execute("INSERT INTO t1 VALUES(1);")
+-- test duplicate index error
+box.sql.execute("INSERT INTO t1 VALUES(1);")
+box.sql.execute("SELECT * FROM t2;")
+box.sql.execute("DELETE FROM t2;")
+
+
+-- test triggers
+tuple = {"T2T", {sql = "CREATE TRIGGER t2t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(2); END;"}}
+box.space._trigger:insert(tuple)
+tuple = {"T3T", {sql = "CREATE TRIGGER t3t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(3); END;"}}
+box.space._trigger:insert(tuple)
+box.space._trigger:select()
+box.sql.execute("INSERT INTO t1 VALUES(2);")
+box.sql.execute("SELECT * FROM t2;")
+box.sql.execute("DELETE FROM t2;")
+
+-- test t1t after t2t and t3t drop
+box.sql.execute("DROP TRIGGER T2T;")
+box.space._trigger:delete("T2T")
+box.space._trigger:select()
+box.sql.execute("INSERT INTO t1 VALUES(3);")
+box.sql.execute("SELECT * FROM t2;")
+box.sql.execute("DELETE FROM t2;")
+
+-- insert new SQL t2t and t3t
+box.sql.execute([[CREATE TRIGGER t2t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(2); END; ]])
+box.sql.execute([[CREATE TRIGGER t3t AFTER INSERT ON t1 BEGIN INSERT INTO t2 VALUES(3); END; ]])
+box.space._trigger:select()
+box.sql.execute("INSERT INTO t1 VALUES(4);")
+box.sql.execute("SELECT * FROM t2;")
+
+-- clean up
+box.space._trigger:delete("t1t")
+box.space._trigger:delete("t2t")
+box.space._trigger:delete("t3t")
+box.sql.execute("DROP TABLE t1;")
+box.sql.execute("DROP TABLE t2;")
+box.space._trigger:select()
+
+
+test_run:cmd("clear filter")
--
2.7.4
More information about the Tarantool-patches
mailing list