[tarantool-patches] Re: [PATCH v1 3/3] box: local sql_flags for parser and vdbe

["n.pettik" <korablev@tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]



> On 15 May 2019, at 21:54, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> The sql_flags is a parser parameter that describe

-> describes

Nit: not only how to parse, but it determines general behaviour:
like whether foreign keys are handled as deferred or not etc.

> how to parse the
> SQL request, but now this information is taken from the global
> user session object.
> When we need to run the parser with some other parameters, it is
> necessary to change global session object, which may lead to
> unpredictable consequences in general case.
> Introduced a new parser and vdbe field sql_flags is responsible

-> which is responsible

> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index a95b07155..bde70a4d3 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -30,7 +30,6 @@
>  */
> 
> #include "box/box.h"
> -#include "box/session.h"
> #include "box/schema.h"
> #include "sqlInt.h"
> #include "tarantoolInt.h"
> @@ -151,7 +150,8 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
> 	struct space *space = sql_lookup_space(parse, tab_list->a);
> 	if (space == NULL)
> 		goto delete_from_cleanup;
> -	trigger_list = sql_triggers_exist(space->def, TK_DELETE, NULL, NULL);
> +	trigger_list = sql_triggers_exist(parse, space->def, TK_DELETE,
> +					  NULL, NULL);

Why not pass only flags from parsing context?

> @@ -5444,7 +5439,7 @@ default: {          /* This is really OP_Noop and OP_Explain */
> 		assert(pOp>=&aOp[-1] && pOp<&aOp[p->nOp-1]);
> 
> #ifdef SQL_DEBUG
> -		if (user_session->sql_flags & SQL_VdbeTrace) {
> +		if ((p->sql_flags & SQL_VdbeTrace) != 0) {
> 			u8 opProperty = sqlOpcodeProperty[pOrigOp->opcode];
> 			if (rc!=0) printf("rc=%d\n",rc);
> 			if (opProperty & (OPFLG_OUT2)) {
> diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
> index a3100e513..86fd92da1 100644
> --- a/src/box/sql/vdbeInt.h
> +++ b/src/box/sql/vdbeInt.h
> @@ -446,6 +446,8 @@ struct Vdbe {
> 	u32 expmask;		/* Binding to these vars invalidates VM */
> 	SubProgram *pProgram;	/* Linked list of all sub-programs used by VM */
> 	AuxData *pAuxData;	/* Linked list of auxdata allocations */
> +	/** Parser flags with which this object was built. */

Strictly speaking, they these flags are not only parsing flags.

The rest is OK.

[-- Attachment #2: Type: text/html, Size: 49991 bytes --]