[tarantool-patches] Re: [PATCH] sql: print space names in VDBE dump

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Hello. Thanks for the patch! See 3 comments below.

On 17/05/2018 16:25, Kirill Yukhin wrote:
> Before the patch, when LoadPtr instruction is dumped,
> raw pointer value was printed, which was looked like a
> garbade. The patch inroduces new pointer type: space ptr,
> which allows to print space name during VDBE dump.
> ---

1. Please, post here branch and issue link (if issue exists).

2. Can not find any similar branch on github by kyukhin/ prefix.

>   src/box/sql/analyze.c |  4 ++--
>   src/box/sql/build.c   |  3 +--
>   src/box/sql/insert.c  |  2 +-
>   src/box/sql/vdbe.c    |  6 +++---
>   src/box/sql/vdbe.h    | 19 ++++++++++++++++++-
>   src/box/sql/vdbeInt.h |  3 ++-
>   src/box/sql/vdbeaux.c |  9 +++++++--
>   7 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
> index 50d212a..563cd18 100644
> --- a/src/box/sql/analyze.c
> +++ b/src/box/sql/analyze.c
> @@ -910,8 +910,8 @@ analyzeOneTable(Parse * pParse,	/* Parser context */
>   		struct space *space =
>   			space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
>   		assert(space != NULL);
> -		sqlite3VdbeAddOp4Ptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> -				     (void *) space);
> +		sql_vdbe_add_op4_spaceptr(v, OP_LoadPtr, 0, space_ptr_reg, 0,
> +					  space);

3. I think, all VdbeAppOp functions family must be refactored at once.
You can introduce P4_SPACEPTR and use
sqlite3VdbeAddOp4(v, OP_LoadPtr, 0, space_ptr_reg, 0, space, P4_SPACEPTR);.