From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (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 D25894696C3 for ; Sat, 29 Feb 2020 18:31:56 +0300 (MSK) References: <20200229124645.67971-1-roman.habibov@tarantool.org> <20200229124645.67971-3-roman.habibov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <6ff10f65-40f4-373c-e881-cc2d00e72235@tarantool.org> Date: Sat, 29 Feb 2020 16:31:53 +0100 MIME-Version: 1.0 In-Reply-To: <20200229124645.67971-3-roman.habibov@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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: Roman Khabibov Cc: tarantool-patches@dev.tarantool.org Thanks for the patch! See 2 comments below. > 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); > } > > +/** > + * Generate VDBE program to remove entry from _index space. > + * > + * @param parse_context Parsing context. > + * @param name Index name. > + * It will be freed in VDBE. 1. This easily fits into one line. Also I don't understand what does it mean 'freed in VDBE'. From 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(). > + * @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 == ER_NO_SUCH_INDEX_NAME || > + errcode == ER_NO_SUCH_CONSTRAINT); > + struct Vdbe *vdbe = sqlGetVdbe(parse_context); > + assert(vdbe != NULL); > + assert(parse_context->db != NULL); > + int key_reg = 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 = > + 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) != 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 = space_by_name(tbl_name); > if (space == NULL) > return; > - uint32_t iid = box_index_id_by_name(space->def->id, idx_name, > - strlen(idx_name)); > - if (iid == BOX_ID_NIL) > + struct index *idx = space_index_by_name(space, idx_name); > + if (idx == NULL) > return; > - struct index *idx = space_index(space, iid); > - assert(idx != NULL); 2. Yeah, ok, good enough. The final goal should be to stop using even struct space pointer here. Sometime later. > parse->nMem = 6; > struct Vdbe *v = sqlGetVdbe(parse); > assert(v != NULL);