[tarantool-patches] Re: [PATCH v1 1/2] sql: use schema API to get index info in analyze

["n.pettik" <korablev@tarantool.org>]

> On 24 Jul 2018, at 14:05, Kirill Shcherbatov <kshcherbatov@tarantool.org> wrote:
> 
> We would like to avoid starting transactions in ANALYZE
> so we need to use schema API that is more tolerant.

Please, provide more detailed explanation, such as:

“
Previously, ANALYZE routine used to invoke box_space_id_by_name
function, which in turn relies on sysview engine. On the other hand,
ANALYZE processing is surrounded by transaction started in memtx
(_sql_stat1 and _sql_stat4 are system spaces stored in memtx).
Thus, multi-engine transaction error may  occur. To avoid such
situation lets use internal schema_find_id() function.
"

> 
> Part of #3551.
> ---
> src/box/sql/analyze.c | 82 ++++++++++++++++++++++++++++++---------------------
> 1 file changed, 48 insertions(+), 34 deletions(-)
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 00d96d2..c7b85dc 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -1219,6 +1219,26 @@ decode_stat_string(const char *stat_string, int stat_size, tRowcnt *stat_exact,
> }
> 
> /**
> + * Find index with specified @name in specified @space.
> + * @param space to lookup.
> + * @param name to use in comparation.
> + * @param len lenth of @name string.

Typo: length.

> + * @retval NULL on nothing found.

If nothing is found.

> + * @retval not NULL pointer on index AST else.

Index AST? I guess “pointer to index otherwise”.

> + */
> +static struct index *
> +space_index_by_name(struct space *space, const char *name, uint32_t len)
> +{
> +	uint32_t idx_cnt = space->index_count;
> +	for (uint32_t i = 0; i < idx_cnt; i++) {
> +		const char *idx_name = space->index[i]->def->name;
> +		if (strlen(idx_name) == len && memcmp(idx_name, name, len) == 0)
> +			return space->index[i];
> +	}
> +	return NULL;
> +}

Overall it is sad that we can't use schema_find_id for this purpose.
Mb it is worth to refactor this function so that it will take not id but
already encoded key to be found? It is only suggestion tho,
it can be contradictory way..