[tarantool-patches] Re: [PATCH] sql: remove unnecessary def rebuild in endTable()

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

Thanks for the patch! LGTM.

On 07/06/2018 14:24, Nikita Pettik wrote:
> sqlite3EndTable() routine is called twice during table creation: firsly
> after initial parsing alongside with generating VDBE program; secondly -
> as a callback in order to add struct Table to internal table hash.
> sqlite3EndTable() contains call of sql_table_def_rebuild() which in
> turn copies space_def to malloc. However, it makes no sence to call
> rebuild during the first stage, since this sample of struct Table will
> be destroyed at the end of initial parsing and will be resurrected at
> the second stage from the scratch.
> ---
> Branch: https://github.com/tarantool/tarantool/tree/np/remove-unnecessary-def-rebuild
> Issue: no corresponding issue
> 
>   src/box/sql/build.c | 21 +++++++++++++--------
>   1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 28e4d7a4d..62d687b17 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -1869,14 +1869,19 @@ sqlite3EndTable(Parse * pParse,	/* Parse context */
>   		return;
>   	}
>   
> -	/*
> -	 * As rebuild creates a new ExpList tree and old_def
> -	 * is allocated on region release old tree manually.
> -	 */
> -	struct ExprList *old_checks = p->def->opts.checks;
> -	if (sql_table_def_rebuild(db, p) != 0)
> -		return;
> -	sql_expr_list_delete(db, old_checks);
> +	if (db->init.busy) {
> +		/*
> +		 * As rebuild creates a new ExpList tree and
> +		 * old_def is allocated on region release old
> +		 * tree manually. This procedure is necessary
> +		 * only at second stage of table creation, i.e.
> +		 * before adding to table hash.
> +		 */
> +		struct ExprList *old_checks = p->def->opts.checks;
> +		if (sql_table_def_rebuild(db, p) != 0)
> +			return;
> +		sql_expr_list_delete(db, old_checks);
> +	}
>   
>   	/* If not initializing, then create new Tarantool space.
>   	 *
>