From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E0C12469719 for ; Tue, 3 Mar 2020 13:15:48 +0300 (MSK) Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <6ff10f65-40f4-373c-e881-cc2d00e72235@tarantool.org> Date: Tue, 3 Mar 2020 13:15:44 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: <54533CD7-87C6-4C2E-B6E0-9972544F5D77@tarantool.org> References: <20200229124645.67971-1-roman.habibov@tarantool.org> <20200229124645.67971-3-roman.habibov@tarantool.org> <6ff10f65-40f4-373c-e881-cc2d00e72235@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 2/3] sql: don't select from _index during parsing List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org > On Feb 29, 2020, at 18:31, Vladislav Shpilevoy = wrote: >=20 > Thanks for the patch! >=20 > See 2 comments below. >=20 >> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >> index d9bf8de91..00877b7d8 100644 >> --- a/src/box/sql/build.c >> +++ b/src/box/sql/build.c >> @@ -47,7 +47,6 @@ >> #include "sqlInt.h" >> #include "vdbeInt.h" >> #include "tarantoolInt.h" >> -#include "box/box.h" >> #include "box/ck_constraint.h" >> #include "box/fk_constraint.h" >> #include "box/sequence.h" >> @@ -1469,6 +1468,41 @@ vdbe_emit_stat_space_clear(struct Parse = *parse, const char *stat_table_name, >> sql_table_delete_from(parse, src_list, where); >> } >>=20 >> +/** >> + * Generate VDBE program to remove entry from _index space. >> + * >> + * @param parse_context Parsing context. >> + * @param name Index name. >> + * It will be freed in VDBE. >=20 > 1. This easily fits into one line. Also I don't understand what does > it mean 'freed in VDBE'. =46rom what I see, this function has nothing > to do with freeing 'name' argument, as well as VDBE. It is freed > by the caller in the end of sql_drop_index(). Oops. This line turned out to be a case here. >> + * @param space_def Def of table which index belongs to. >> + * @param errcode Type of printing error: "no such index" or >> + "no such constraint". >> + * @param if_exist True if there was in the query. >> + */ >> +static void >> +vdbe_emit_index_drop(struct Parse *parse_context, const char *name, >> + struct space_def *space_def, int errcode, bool = if_exist) >> +{ >> + assert(errcode =3D=3D ER_NO_SUCH_INDEX_NAME || >> + errcode =3D=3D ER_NO_SUCH_CONSTRAINT); >> + struct Vdbe *vdbe =3D sqlGetVdbe(parse_context); >> + assert(vdbe !=3D NULL); >> + assert(parse_context->db !=3D NULL); >> + int key_reg =3D sqlGetTempRange(parse_context, 3); >> + sqlVdbeAddOp2(vdbe, OP_Integer, space_def->id, key_reg); >> + sqlVdbeAddOp4(vdbe, OP_String8, 0, key_reg + 1, 0, >> + sqlDbStrDup(parse_context->db, name), P4_DYNAMIC); >> + const char *error_msg =3D >> + tt_sprintf(tnt_errcode_desc(errcode), name, = space_def->name); >> + if (vdbe_emit_halt_with_presence_test(parse_context, = BOX_INDEX_ID, 2, >> + key_reg, 2, errcode, = error_msg, >> + if_exist, OP_Found) !=3D = 0) >> + return; >> + sqlVdbeAddOp3(vdbe, OP_MakeRecord, key_reg, 2, key_reg + 2); >> + sqlVdbeAddOp3(vdbe, OP_SDelete, BOX_INDEX_ID, key_reg + 2, 2); >> + sqlReleaseTempRange(parse_context, key_reg, 3); >> +} >> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c >> index a77bf4b16..c15f2e0d1 100644 >> --- a/src/box/sql/pragma.c >> +++ b/src/box/sql/pragma.c >> @@ -192,12 +191,9 @@ sql_pragma_index_info(struct Parse *parse, >> struct space *space =3D space_by_name(tbl_name); >> if (space =3D=3D NULL) >> return; >> - uint32_t iid =3D box_index_id_by_name(space->def->id, idx_name, >> - strlen(idx_name)); >> - if (iid =3D=3D BOX_ID_NIL) >> + struct index *idx =3D space_index_by_name(space, idx_name); >> + if (idx =3D=3D NULL) >> return; >> - struct index *idx =3D space_index(space, iid); >> - assert(idx !=3D NULL); >=20 > 2. Yeah, ok, good enough. The final goal should be to stop using > even struct space pointer here. Sometime later. >=20 >> parse->nMem =3D 6; >> struct Vdbe *v =3D sqlGetVdbe(parse); >> assert(v !=3D NULL);