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

[Nikita Pettik <korablev@tarantool.org>]

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.

> +{
> +	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.
>   *