[tarantool-patches] [PATCH v2 11/11] sql: VDBE tests for trigger existence

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Trigger presence in system should be tested on each VDBE
execution attempt, not on Parser iteration.

Part of #3435, #3273
---
 src/box/sql.c           | 35 +++++++++++++++++++++++++++++++++++
 src/box/sql/main.c      | 13 +++++++++++--
 src/box/sql/sqliteInt.h | 20 ++++++++++++++++++++
 src/box/sql/trigger.c   | 20 +++++++++-----------
 src/box/sql/vdbe.c      | 14 ++++++++++----
 src/box/sql/vdbeapi.c   |  5 +++--
 6 files changed, 88 insertions(+), 19 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index 57211fd..f539085 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1836,3 +1836,38 @@ sql_checks_resolve_space_def_reference(ExprList *expr_list,
 	}
 	return 0;
 }
+
+int
+vdbe_abort_execution_on_exists(Parse *parser, int space_id, int index_id,
+			       const char *name, int tarantool_error_code,
+			       bool no_error)
+{
+	Vdbe *v = sqlite3GetVdbe(parser);
+	assert(v != NULL);
+
+	sqlite3 *db = parser->db;
+	name = sqlite3DbStrDup(db, name);
+	if (name == NULL) {
+		diag_set(OutOfMemory, strlen(name) + 1, "sqlite3DbStrDup",
+			 "name");
+		return -1;
+	}
+	int cursor = parser->nTab++;
+	int entity_id =
+		SQLITE_PAGENO_FROM_SPACEID_AND_INDEXID(space_id, index_id);
+	emit_open_cursor(parser, cursor, entity_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, name, P4_STATIC);
+		sqlite3VdbeChangeP5(v, tarantool_error_code);
+	}
+	sqlite3VdbeAddOp1(v, OP_Close, cursor);
+	return 0;
+}
diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 0acf7bc..f2334cb 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -1456,8 +1456,17 @@ sqlite3_errmsg(sqlite3 * db)
 		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) {
+			testcase(db->pErr == 0);
+			z = (char *) sqlite3_value_text(db->pErr);
+			assert(!db->mallocFailed);
+			if (z == NULL)
+				z = sqlite3ErrStr(db->errCode);
+		} else {
+			struct diag *diag = diag_get();
+			assert(diag != NULL);
+			struct error *error = diag_last_error(diag);
+			z = error->errmsg;
 		}
 	}
 	return z;
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 276f881..4f7dcbe 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4537,4 +4537,24 @@ 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.
+ *
+ * @param parser Parsing context.
+ * @param space_id Space to lookup identifier.
+ * @param index_id Index identifier containing string primary key.
+ * @param name Of object to test existency.
+ * @param tarantool_error_code Tarantool error to raise on object exists.
+ * @param no_error Do not raise error flag.
+ *
+ * @retval -1 on memory allocation error.
+ * @retval 0 on success.
+ */
+int
+vdbe_abort_execution_on_exists(Parse *parser, int space_id, int index_id,
+			       const char *name, int tarantool_error_code,
+			       bool no_error);
+
 #endif				/* SQLITEINT_H */
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 1b569bc..92747f0 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -122,17 +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;
-		} 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),
@@ -178,6 +167,15 @@ sqlite3BeginTrigger(Parse * pParse,	/* The parse context of the CREATE TRIGGER s
 		pParse->rc = SQL_TARANTOOL_ERROR;
 		goto trigger_cleanup;
 	}
+	if (!pParse->parse_only) {
+		if (vdbe_abort_execution_on_exists(pParse, BOX_TRIGGER_ID, 0,
+						   zName,
+						   ER_TRIGGER_EXISTS,
+						   (noErr != 0)) != 0) {
+			pParse->rc = SQL_TARANTOOL_ERROR;
+			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 2d1538e..0696cad 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -959,7 +959,7 @@ case OP_HaltIfNull: {      /* in3 */
  * VDBE, but do not rollback the transaction.
  *
  * If P4 is not null then it is an error message string.
- *
+ * If P1 is not SQL_TARANTOOL_ERROR,
  * P5 is a value between 0 and 4, inclusive, that modifies the P4 string.
  *
  *    0:  (no change)
@@ -968,6 +968,8 @@ case OP_HaltIfNull: {      /* in3 */
  *    3:  CHECK constraint failed: P4
  *    4:  FOREIGN KEY constraint failed: P4
  *
+ * If P1 is SQL_TARANTOOL_ERROR, P5 contain ClientError code.
+ *
  * If P5 is not zero and P4 is NULL, then everything after the ":" is
  * omitted.
  *
@@ -1005,9 +1007,10 @@ 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 (pOp->p5 != 0 && p->rc == SQL_TARANTOOL_ERROR) {
+			diag_set(ClientError, 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);
@@ -1029,7 +1032,10 @@ case OP_Halt: {
 		p->rc = SQLITE_BUSY;
 	} else {
 		assert(rc==SQLITE_OK || (p->rc&0xff)==SQLITE_CONSTRAINT);
-		rc = p->rc ? SQLITE_ERROR : SQLITE_DONE;
+		if (p->rc != SQL_TARANTOOL_ERROR)
+			rc = (p->rc != SQLITE_OK) ? SQLITE_ERROR : SQLITE_DONE;
+		else
+			rc = SQL_TARANTOOL_ERROR;
 	}
 	goto vdbe_return;
 }
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index d35338a..0aa5c4a 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -598,8 +598,9 @@ sqlite3Step(Vdbe * p)
 	 * contains the value that would be returned if sqlite3_finalize()
 	 * were called on statement p.
 	 */
-	assert(rc == SQLITE_ROW || rc == SQLITE_DONE || rc == SQLITE_ERROR
-	       || (rc & 0xff) == SQLITE_BUSY || rc == SQLITE_MISUSE);
+	assert(rc == SQLITE_ROW || rc == SQLITE_DONE || rc == SQLITE_ERROR ||
+	       (rc & 0xff) == SQLITE_BUSY || rc == SQLITE_MISUSE ||
+	       rc == SQL_TARANTOOL_ERROR);
 	if (p->isPrepareV2 && rc != SQLITE_ROW && rc != SQLITE_DONE) {
 		/* If this statement was prepared using sqlite3_prepare_v2(), and an
 		 * error has occurred, then return the error code in p->rc to the
-- 
2.7.4