Re: [Tarantool-patches] [PATCH v2 2/3] sql: don't select from _index during parsing

[Vladislav Shpilevoy <v.shpilevoy@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 <IF EXISTS> 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);