[tarantool-patches] Re: [PATCH v1 2/2] sql: use own region in Parser

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

Thanks for the patch! See 2 comments below.

On 31/05/2018 18:01, Kirill Shcherbatov wrote:
> diff --git a/src/box/sql/select.c b/src/box/sql/select.c
> index 5fbcbaf..b26565b 100644
> --- a/src/box/sql/select.c
> +++ b/src/box/sql/select.c
> @@ -1762,7 +1762,7 @@ sqlite3ColumnsFromExprList(Parse * parse, ExprList * expr_list, Table *table)
>   	 * names for existing table.
>   	 */
>   	assert(table->def->fields == NULL);
> -	struct region *region = &fiber()->gc;
> +	struct region *region = &parse->region;
>   	table->def->fields =
>   		region_alloc(region, nCol * sizeof(table->def->fields[0]));
>   	if (table->def->fields == NULL) {
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index abf1543..4275f72 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -2913,8 +2913,8 @@ struct Parse {
>   	u8 eTriggerOp;		/* TK_UPDATE, TK_INSERT or TK_DELETE */
>   	u8 eOrconf;		/* Default ON CONFLICT policy for trigger steps */
>   	u8 disableTriggers;	/* True to disable triggers */
> -	/** Region size at the Parser launch. */
> -	size_t region_initial_size;
> +	/* Region to make SQL temp allocations. */

1. Please, use /** for out of a function body comments.

> +	struct region region;
>   
>     /**************************************************************************
>     * Fields above must be initialized to zero.  The fields that follow,
2. Did you check it works now on the Kirill Yu. patch? (with additional field in
space options).