Re: [Tarantool-patches] [PATCH v1 1/1] sql: remove unecessary execute of space_cache_find()

[Nikita Pettik <korablev@tarantool.org>]

On 05 Dec 12:35, Mergen Imeev via Tarantool-patches wrote:
> Due to the fact that space_cache_find () is called unnecessarily, it is
> possible to set diag "Space '0' does not exist", although in this case
> it is not a wrong situation when the space id is 0.

Patch is OK but I don't understand how could it be flaky...
I mean calling space_cache_find() is deterministic (at least here: query plan
shouldn't change from call to call). So I suggest to put assert(space != NULL)
after space_cache_find(), then run test case (as I understand it is query from
Mike Siomkin) and make sure that it always fails. After that, apply your
patch and verify that it no longer fails.
 
> Part of #5592
> ---
> https://github.com/tarantool/tarantool/issues/5592
> https://github.com/tarantool/tarantool/tree/imeevma/gh-5592-remove-unnecessary-diag-set
> 
>  src/box/sql/where.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/box/sql/where.c b/src/box/sql/where.c
> index 0d7590f0e..65d4197f2 100644
> --- a/src/box/sql/where.c
> +++ b/src/box/sql/where.c
> @@ -4581,7 +4581,7 @@ sqlWhereBegin(Parse * pParse,	/* The parser context */
>  		struct SrcList_item *pTabItem = &pTabList->a[pLevel->iFrom];
>  		struct space_def *space_def = pTabItem->space->def;
>  		pLoop = pLevel->pWLoop;
> -		struct space *space = space_cache_find(space_def->id);
> +		struct space *space = pTabItem->space;
>  		if (space_def->id == 0 || space_def->opts.is_view) {
>  			/* Do nothing */
>  		} else if ((pLoop->wsFlags & WHERE_IDX_ONLY) == 0 &&
> -- 
> 2.25.1
>