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 0405226366 for ; Thu, 14 Jun 2018 13:32:33 -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 Uph___YGTihW for ; Thu, 14 Jun 2018 13:32:32 -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 92C632635D for ; Thu, 14 Jun 2018 13:32:32 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v3 10/10] sql: VDBE tests for trigger existence Date: Thu, 14 Jun 2018 20:32:19 +0300 Message-Id: <2dc4c354dc72123c3447831a6ac48038eaf95f48.1528997527.git.kshcherbatov@tarantool.org> In-Reply-To: References: 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: v.shpilevoy@tarantool.org, Kirill Shcherbatov 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 | 37 +++++++++++++++++++++++++++++++++++++ src/box/sql/main.c | 10 +++++++--- src/box/sql/sqliteInt.h | 20 ++++++++++++++++++++ src/box/sql/trigger.c | 23 +++++++++++------------ src/box/sql/vdbe.c | 10 +++++++--- src/box/sql/vdbe.h | 2 ++ src/box/sql/vdbeapi.c | 5 +++-- src/box/sql/vdbeaux.c | 4 ++++ src/diag.h | 3 +++ 9 files changed, 94 insertions(+), 20 deletions(-) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index 67cdf11..0a731df 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -4171,4 +4171,41 @@ sqlite3WithDelete(sqlite3 * db, With * pWith) sqlite3DbFree(db, pWith); } } + +int +parser_emit_execution_halt_on_exists(struct Parse *parser, int space_id, + int index_id, const char *name, + struct error *error, bool no_error) +{ + error_ref(error); + struct Vdbe *v = sqlite3GetVdbe(parser); + assert(v != NULL); + + struct 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) { + error_unref(error); + sqlite3VdbeAddOp0(v, OP_Halt); + } else { + sqlite3VdbeAddOp4(v, OP_Halt, SQL_TARANTOOL_ERROR, + ON_CONFLICT_ACTION_FAIL, 0, (void *)error, + P4_ERROROBJ); + } + 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..e1de815 100644 --- a/src/box/sql/main.c +++ b/src/box/sql/main.c @@ -1454,10 +1454,14 @@ 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; } } return z; diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 332986f..d86ace5 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 existence. + * @param error object to raise on target exists. + * @param no_error Do not raise error flag. + * + * @retval -1 on memory allocation error. + * @retval 0 on success. + */ +int +parser_emit_execution_halt_on_exists(struct Parse *parser, int space_id, + int index_id, const char *name, + struct error *error, bool no_error); + #endif /* SQLITEINT_H */ diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c index 9ca4262..4cb89fe 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), @@ -184,6 +172,17 @@ sqlite3BeginTrigger(Parse * pParse, /* The parse context of the CREATE TRIGGER s pParse->nErr++; goto trigger_cleanup; } + if (!pParse->parse_only) { + struct error *error = + error_object(ClientError, ER_TRIGGER_EXISTS, zName); + if (parser_emit_execution_halt_on_exists(pParse, BOX_TRIGGER_ID, + 0, zName, error, + (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 b5f1f1c..c3109ab 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1005,9 +1005,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 (p->rc == SQL_TARANTOOL_ERROR) { + diag_add_error(diag_get(), (struct error *)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 +1030,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/vdbe.h b/src/box/sql/vdbe.h index 77fa41a..66e6954 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -149,6 +149,8 @@ typedef struct VdbeOpList VdbeOpList; #define P4_PTR (-18) /* P4 is a generic pointer */ #define P4_KEYDEF (-19) /* P4 is a pointer to key_def structure. */ #define P4_SPACEPTR (-20) /* P4 is a space pointer */ +/** P4 is an error object. */ +#define P4_ERROROBJ (-21) /* Error message codes for OP_Halt */ #define P5_ConstraintNotNull 1 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 diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 7b3c13d..8d89894 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -981,6 +981,10 @@ freeP4(sqlite3 * db, int p4type, void *p4) } break; } + case P4_ERROROBJ: { + error_unref(p4); + break; + } } } diff --git a/src/diag.h b/src/diag.h index 0ccf86d..502b294 100644 --- a/src/diag.h +++ b/src/diag.h @@ -261,6 +261,9 @@ struct error * BuildSocketError(const char *file, unsigned line, const char *socketname, const char *format, ...); +#define error_object(class, ...) \ + Build##class(__FILE__, __LINE__, ##__VA_ARGS__); + #define diag_set(class, ...) do { \ /* Preserve the original errno. */ \ int save_errno = errno; \ -- 2.7.4