[tarantool-patches] [PATCH v4 6/6] sql: VDBE tests for trigger existence
Kirill Shcherbatov
kshcherbatov at tarantool.org
Wed Jun 20 13:46:16 MSK 2018
Trigger presence in system should be tested on each VDBE
execution attempt, not on Parser iteration.
Part of #3435, #3273
---
src/box/sql/build.c | 42 ++++++++++++++++++++++++++++++++++++++++++
src/box/sql/main.c | 11 ++++++++---
src/box/sql/sqliteInt.h | 24 ++++++++++++++++++++++++
src/box/sql/trigger.c | 25 +++++++++++++------------
src/box/sql/vdbe.c | 18 +++++++++++-------
test/sql-tap/view.test.lua | 8 ++++----
test/sql/view.result | 4 ++--
7 files changed, 104 insertions(+), 28 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index c8bfad7..fff7c19 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -3971,4 +3971,46 @@ sqlite3WithDelete(sqlite3 * db, With * pWith)
sqlite3DbFree(db, pWith);
}
}
+
+int
+vdbe_emit_execution_halt_on_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)
+{
+ struct Vdbe *v = sqlite3GetVdbe(parser);
+ assert(v != NULL);
+
+ struct sqlite3 *db = parser->db;
+ char *name = sqlite3DbStrDup(db, name_src);
+ if (name == NULL) {
+ size_t size = strlen(name_src) + 1;
+ diag_set(OutOfMemory, size, "sqlite3DbStrDup", "name");
+ return -1;
+ }
+ char *error = sqlite3DbStrDup(db, error_src);
+ if (error == NULL) {
+ sqlite3DbFree(db, name);
+ size_t size = strlen(error_src) + 1;
+ diag_set(OutOfMemory, size, "sqlite3DbStrDup", "error");
+ return -1;
+ }
+
+ int cursor = parser->nTab++;
+ vdbe_emit_open_cursor(parser, cursor, index_id, space_by_id(space_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);
+ if (no_error) {
+ sqlite3VdbeAddOp0(v, OP_Halt);
+ } else {
+ sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR,
+ ON_CONFLICT_ACTION_FAIL, 0, error, P4_DYNAMIC);
+ sqlite3VdbeChangeP5(v, tarantool_error_code);
+ }
+ sqlite3VdbeAddOp1(v, OP_Close, cursor);
+ return 0;
+}
#endif /* !defined(SQLITE_OMIT_CTE) */
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 0acf7bc..e435c01 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -1454,11 +1454,16 @@ sqlite3_errmsg(sqlite3 * db)
z = sqlite3ErrStr(SQLITE_NOMEM_BKPT);
} else {
testcase(db->pErr == 0);
- z = (char *)sqlite3_value_text(db->pErr);
assert(!db->mallocFailed);
- if (z == 0) {
- z = sqlite3ErrStr(db->errCode);
+ if (db->errCode != SQL_TARANTOOL_ERROR) {
+ assert(!db->mallocFailed);
+ z = (char *)sqlite3_value_text(db->pErr);
+ if (z == NULL)
+ z = sqlite3ErrStr(db->errCode);
+ } else {
+ z = diag_last_error(diag_get())->errmsg;
}
+ assert(z != NULL);
}
return z;
}
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 72803fa..acda23d 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4627,4 +4627,28 @@ extern int sqlite3InitDatabase(sqlite3 * db);
enum on_conflict_action
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 function does not begin to hold the passed error pointer
+ * to memory.
+ *
+ * @param parser Parsing context.
+ * @param space_id Space to lookup identifier.
+ * @param index_id Index identifier containing string primary key.
+ * @param name_src Of object to test existence.
+ * @param tarantool_error_code to set on halt.
+ * @param error_src string to notify on halt.
+ * @param no_error Do not raise error flag.
+ *
+ * @retval -1 on memory allocation error.
+ * @retval 0 on success.
+ */
+int
+vdbe_emit_execution_halt_on_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);
+
#endif /* SQLITEINT_H */
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index e4c936d..8ccb646 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -122,18 +122,6 @@ sqlite3BeginTrigger(Parse * pParse, /* The parse context of the CREATE TRIGGER s
if (sqlite3CheckIdentifierName(pParse, zName) != SQLITE_OK)
goto trigger_cleanup;
- if (!pParse->parse_only &&
- sqlite3HashFind(&db->pSchema->trigHash, zName) != NULL) {
- if (!noErr) {
- diag_set(ClientError, ER_TRIGGER_EXISTS, zName);
- pParse->rc = SQL_TARANTOOL_ERROR;
- pParse->nErr++;
- } else {
- assert(!db->init.busy);
- }
- goto trigger_cleanup;
- }
-
const char *table_name = pTableName->a[0].zName;
uint32_t space_id;
if (schema_find_id(BOX_SPACE_ID, 2, table_name, strlen(table_name),
@@ -179,6 +167,19 @@ sqlite3BeginTrigger(Parse * pParse, /* The parse context of the CREATE TRIGGER s
pParse->nErr++;
goto trigger_cleanup;
}
+ if (!pParse->parse_only) {
+ const char *error_msg =
+ tt_sprintf(tnt_errcode_desc(ER_TRIGGER_EXISTS), zName);
+ if (vdbe_emit_execution_halt_on_exists(pParse, BOX_TRIGGER_ID,
+ 0, zName,
+ ER_TRIGGER_EXISTS,
+ error_msg,
+ (noErr != 0)) != 0) {
+ pParse->rc = SQL_TARANTOOL_ERROR;
+ pParse->nErr++;
+ goto trigger_cleanup;
+ }
+ }
/*
* INSTEAD OF triggers can only appear on views and BEFORE triggers
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c1df880..487b026 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -960,7 +960,9 @@ case OP_HaltIfNull: { /* in3 */
*
* If P4 is not null then it is an error message string.
*
- * P5 is a value between 0 and 4, inclusive, that modifies the P4 string.
+ * If P1 is SQL_TARANTOOL_ERROR then P5 is a ClientError code and
+ * P4 is error message to set. Else P5 is a value between 0 and 4,
+ * inclusive, that modifies the P4 string.
*
* 0: (no change)
* 1: NOT NULL contraint failed: P4
@@ -968,8 +970,8 @@ case OP_HaltIfNull: { /* in3 */
* 3: CHECK constraint failed: P4
* 4: FOREIGN KEY constraint failed: P4
*
- * If P5 is not zero and P4 is NULL, then everything after the ":" is
- * omitted.
+ * If P5 is not zero and P4 is NULL, then everything after the
+ * ":" is omitted.
*
* There is an implied "Halt 0 0 0" instruction inserted at the very end of
* every program. So a jump past the last instruction of the program
@@ -1005,9 +1007,11 @@ case OP_Halt: {
p->rc = pOp->p1;
p->errorAction = (u8)pOp->p2;
p->pc = pcx;
- assert(pOp->p5<=4);
if (p->rc) {
- if (pOp->p5) {
+ if (p->rc == SQL_TARANTOOL_ERROR) {
+ assert(pOp->p4.z != NULL);
+ box_error_set(__FILE__, __LINE__, pOp->p5, pOp->p4.z);
+ } else if (pOp->p5 != 0) {
static const char * const azType[] = { "NOT NULL", "UNIQUE", "CHECK",
"FOREIGN KEY" };
testcase( pOp->p5==1);
@@ -2999,8 +3003,8 @@ case OP_CheckViewReferences: {
struct space *space = space_by_id(space_id);
assert(space != NULL);
if (space->def->view_ref_count > 0) {
- sqlite3VdbeError(p,"Can't drop table %s: other views depend "
- "on this space", space->def->name);
+ diag_set(ClientError, ER_DROP_SPACE, space->def->name,
+ "other views depend on this space");
rc = SQL_TARANTOOL_ERROR;
goto abort_due_to_error;
}
diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua
index 5dc9503..ac1a27d 100755
--- a/test/sql-tap/view.test.lua
+++ b/test/sql-tap/view.test.lua
@@ -127,7 +127,7 @@ test:do_catchsql_test(
DROP TABLE t1;
]], {
-- <view-1.6>
- 1, "Can't drop table T1: other views depend on this space"
+ 1, "Can't drop space 'T1': other views depend on this space"
-- </view-1.6>
})
@@ -1185,7 +1185,7 @@ test:do_catchsql_test(
DROP TABLE t11;
]], {
-- <view-23.2>
- 1, "Can't drop table T11: other views depend on this space"
+ 1, "Can't drop space 'T11': other views depend on this space"
-- </view-23.2>
})
@@ -1195,7 +1195,7 @@ test:do_catchsql_test(
DROP TABLE t12;
]], {
-- <view-23.3>
- 1, "Can't drop table T12: other views depend on this space"
+ 1, "Can't drop space 'T12': other views depend on this space"
-- </view-23.3>
})
@@ -1205,7 +1205,7 @@ test:do_catchsql_test(
DROP TABLE t13;
]], {
-- <view-23.4>
- 1, "Can't drop table T13: other views depend on this space"
+ 1, "Can't drop space 'T13': other views depend on this space"
-- </view-23.4>
})
diff --git a/test/sql/view.result b/test/sql/view.result
index 0b9dc55..b033f19 100644
--- a/test/sql/view.result
+++ b/test/sql/view.result
@@ -124,7 +124,7 @@ box.sql.execute("CREATE VIEW v2 AS SELECT * FROM t2;");
...
box.sql.execute("DROP TABLE t2;");
---
-- error: 'Can''t drop table T2: other views depend on this space'
+- error: 'Can''t drop space ''T2'': other views depend on this space'
...
sp = box.space._space:get{box.space.T2.id};
---
@@ -134,7 +134,7 @@ sp = box.space._space:replace(sp);
...
box.sql.execute("DROP TABLE t2;");
---
-- error: 'Can''t drop table T2: other views depend on this space'
+- error: 'Can''t drop space ''T2'': other views depend on this space'
...
box.sql.execute("DROP VIEW v2;");
---
--
2.7.4
More information about the Tarantool-patches
mailing list