[tarantool-patches] Re: [PATCH v3 10/10] sql: VDBE tests for trigger existence

[Vladislav Shpilevoy]

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;
> +		}
> +	}
>