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 61B172F2A0 for ; Thu, 16 May 2019 19:08:38 -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 q_S6paXzqJ6P for ; Thu, 16 May 2019 19:08:38 -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 12AAB21B00 for ; Thu, 16 May 2019 19:08:37 -0400 (EDT) From: "n.pettik" Message-Id: Content-Type: multipart/alternative; boundary="Apple-Mail=_A65A6FE5-7281-45CD-9013-62050F4EFD83" Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.8\)) Subject: [tarantool-patches] Re: [PATCH v1 3/3] box: local sql_flags for parser and vdbe Date: Fri, 17 May 2019 02:08:34 +0300 In-Reply-To: <5d4de78a-0ffe-5238-c2c9-8854a15b842f@tarantool.org> References: <5d4de78a-0ffe-5238-c2c9-8854a15b842f@tarantool.org> 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: Kirill Shcherbatov --Apple-Mail=_A65A6FE5-7281-45CD-9013-62050F4EFD83 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=us-ascii > On 15 May 2019, at 21:54, Kirill Shcherbatov = 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 @@ > */ >=20 > #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 =3D sql_lookup_space(parse, tab_list->a); > if (space =3D=3D NULL) > goto delete_from_cleanup; > - trigger_list =3D sql_triggers_exist(space->def, TK_DELETE, NULL, = NULL); > + trigger_list =3D 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>=3D&aOp[-1] && pOp<&aOp[p->nOp-1]); >=20 > #ifdef SQL_DEBUG > - if (user_session->sql_flags & SQL_VdbeTrace) { > + if ((p->sql_flags & SQL_VdbeTrace) !=3D 0) { > u8 opProperty =3D = sqlOpcodeProperty[pOrigOp->opcode]; > if (rc!=3D0) printf("rc=3D%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.= --Apple-Mail=_A65A6FE5-7281-45CD-9013-62050F4EFD83 Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=us-ascii

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 =3D sql_lookup_space(parse, tab_list->a);
if (space =3D=3D= NULL)
= goto = delete_from_cleanup;
- trigger_list =3D sql_triggers_exist(space->def, TK_DELETE, = NULL, NULL);
+ trigger_list = =3D 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>=3D&aOp[-1] && = pOp<&aOp[p->nOp-1]);

#ifdef SQL_DEBUG
- = if = (user_session->sql_flags & SQL_VdbeTrace) {
+ if ((p->sql_flags & SQL_VdbeTrace) !=3D 0) {
= u8 opProperty = =3D sqlOpcodeProperty[pOrigOp->opcode];
= if (rc!=3D0) printf("rc=3D%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.
= --Apple-Mail=_A65A6FE5-7281-45CD-9013-62050F4EFD83--