Re: [Tarantool-patches] [PATCH 1/2] sql: refactor index drop code

[Roman Khabibov <roman.habibov@tarantool.org>]

> On Jan 28, 2020, at 20:44, Nikita Pettik <korablev@tarantool.org> wrote:
> 
> On 24 Jan 17:21, Roman Khabibov wrote:
>> Wrap index drop opcode emitting into single function.
>> 
>> Part of #4120
>> ---
>> src/box/sql/build.c | 52 ++++++++++++++++++++++++---------------------
>> 1 file changed, 28 insertions(+), 24 deletions(-)
>> 
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index bc50ecbfa..58a6a8a6b 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -1469,6 +1469,26 @@ 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 with @a index_id and @a
>> + * space_id from _index space.
> 
> Is @a common shortcat for @param? If so, please share a link to docs,
> since I've managed to find only this:
> '''
> \a <word>
> Displays the argument in italics. Use this command to emphasize words.
> Use this command to refer to member arguments in the running text.
> '''
> 
>> + * Note: index_id_reg have to be allocated immediately after
>> + * space_id_reg.
>> + */
> 
> Nit: have -> has.
> 
> Please, add an assertion verifing that.
> 
>> +static void
>> +vdbe_emit_index_drop(struct Parse *parse_context, uint32_t index_id,
>> +		     uint32_t index_id_reg, uint32_t space_id_reg)
> 
> I'd also consider moving storing of index_id to register out of
> this function. Then you'll need only space_id_reg to process
> deletion.
I decided to drop this patch, because the function will be too small
and it will not reduce the code. So, IMO, it is pointless then.

>> +{
>> +	struct Vdbe *v = sqlGetVdbe(parse_context);
>> +	assert(v != NULL);
>> +	int record_reg = ++parse_context->nMem;
>> +	sqlVdbeAddOp2(v, OP_Integer, index_id, index_id_reg);
>> +	sqlVdbeAddOp3(v, OP_MakeRecord, space_id_reg, 2, record_reg);
>> +	sqlVdbeAddOp2(v, OP_SDelete, BOX_INDEX_ID, record_reg);
>> +	VdbeComment((v, "Remove index iid =  %u", index_id));
>> +}
>> +
>> /**
>>  * Generate VDBE program to remove entry from _fk_constraint space.
>>  *