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 1F20426212 for ; Thu, 14 Jun 2018 15:27:30 -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 EQtJiL_Dpoja for ; Thu, 14 Jun 2018 15:27:30 -0400 (EDT) Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 CF7BA25F69 for ; Thu, 14 Jun 2018 15:27:29 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v3 10/10] sql: VDBE tests for trigger existence References: <2dc4c354dc72123c3447831a6ac48038eaf95f48.1528997527.git.kshcherbatov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <05ff2cb5-d224-7b04-5aa8-d7d57b7c0031@tarantool.org> Date: Thu, 14 Jun 2018 22:27:27 +0300 MIME-Version: 1.0 In-Reply-To: <2dc4c354dc72123c3447831a6ac48038eaf95f48.1528997527.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Kirill Shcherbatov , tarantool-patches@freelists.org Thanks for the fixes! See 1 comment below. On 14/06/2018 20:32, Kirill Shcherbatov wrote: > 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/trigger.c b/src/box/sql/trigger.c > index 9ca4262..4cb89fe 100644 > --- a/src/box/sql/trigger.c > +++ b/src/box/sql/trigger.c > @@ -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); Unfortunately we have the problem with error objects creating. ClientError in the constructor increments error counter in statistics visible to user. So this still not happened error affects the public API. We need to find another way. I think, we may assume that this way of error setting will be used for ClientErrors only. Indeed, OOM is set explicitly in the place where emerged. Other errors are not used in VDBE. The only hindrance is that ClientErrors have different argument count. But I have found function box_error_set() in error.h. It is a public API to set ClientError with any message. So lets return to the previous implementation, but instead of passing (error code; code arguments) lets pass (error code; the full error message) and use box_error_set() instead of direct diag_set(ClientError, ...). To repeat the same error message format as from the code you can use box_error_set(..., code, tnt_errcode_desc(code), parameters). > + 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; > + } > + } >