[tarantool-patches] [PATCH] sql: remove struct Table

["i.koptelov" <ivan.koptelov@tarantool.org>]

>> On 1 Feb 2019, at 14:05, Ivan Koptelov<ivan.koptelov@tarantool.org>  wrote:
>> 
>> Thank you for the review! Sorry for all these code style errors.
>> I consider review changes minor, so I put diff b/w first and
>> second version of patch at the end of the letter.
>> (second commit on the branch 'review fixes' would be squashed)
> Don’t do it next time, pls. Instead, inline fixes as an answers to comments,
> especially when it comes for huge diff ( 209 insertions(+), 259 deletions(-))
> Otherwise, it takes a while to track fixed chunks of code in a whole diff.
Sorry for this. All review fixes below is inlined.
>>>> Completely removes struct Table. Also the
>>>> patch simplifies memory management as in
>>>> many cases struct space (which replaces
>>>> struct Table) is allocated on region
>>>> and shouldn't be explicitly freed.
>>> Feel free to use up to 72 chars in commit message body.
>>> 
>>> sql: remove struct Table
>>> 
>>> Completely removes struct Table. Also the patch simplifies memory
>>> management as in many cases struct space (which replaces struct Table)
>>> is allocated on region and shouldn't be explicitly freed.
>>> 
>>> Note that after commit subject's prefix we use lower-case.
>>> 
>> Fixed.
>>> Also, for some reason content of this letter looks very strange:
>>> at least it contains unusual fonts… I am not complaining but
>>> are you sure that you set up git-email correctly?
>>> 
> It still looks weird. Please, verify that text you send is plain.
> 
>>>> +
>>>>  #if SQLITE_MAX_COLUMN
>>>> -	if ((int)p->def->field_count + 1 > db->aLimit[SQLITE_LIMIT_COLUMN]) {
>>>> +	if ((int)space->def->field_count + 1 > db->aLimit[SQLITE_LIMIT_COLUMN]) {
>>>>  		sqlite3ErrorMsg(pParse, "too many columns on %s",
>>>> -				p->def->name);
>>>> +				space->def->name);
>>>>  		return;
>>>>  	}
>>>>  #endif
>>>> -	/*
>>>> +	/**
> One more nit: we start comments from single star if they appear inside functions.

Fixed:

@@ -471,7 +445,7 @@ sqlite3AddColumn(Parse * pParse, Token * pName, struct type_def *type_def)
		return;
	}
#endif
-	/**
+	/*
	 * As sql_field_retrieve will allocate memory on region
	 * ensure that p->space->def is also temporal and would be
	 * dropped.


>>>> @@ -435,7 +428,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
>>>>  }
>>>>    void
>>>> -sql_generate_row_delete(struct Parse *parse, struct Table *table,
>>>> +sql_generate_row_delete(struct Parse *parse, struct space *space,
>>>>  			struct sql_trigger *trigger_list, int cursor,
>>>>  			int reg_pk, short npk, bool need_update_count,
>>>>  			enum on_conflict_action onconf, u8 mode,
>>>> @@ -464,31 +457,31 @@ sql_generate_row_delete(struct Parse *parse, struct Table *table,
>>>>  	/* If there are any triggers to fire, allocate a range of registers to
>>>>  	 * use for the old.* references in the triggers.
>>>>  	 */
>>>> -	if (table != NULL &&
>>>> -	    (fkey_is_required(table->def->id, NULL) || trigger_list != NULL)) {
>>>> +	if (space != NULL &&
>>>> +		(fkey_is_required(space->def->id, NULL) ||
>>>> +			trigger_list != NULL)) {
>>>> 
>>> Broken indentation.
>> Fixed now.
> diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
> index e8354f7e4..1dbf244cc 100644
> --- a/src/box/sql/delete.c
> +++ b/src/box/sql/delete.c
> @@ -458,8 +458,7 @@ sql_generate_row_delete(struct Parse *parse, struct space *space,
>          * use for the old.* references in the triggers.
>          */
>         if (space != NULL &&
> -           (fkey_is_required(space->def->id, NULL) ||
> -            trigger_list != NULL)) {
> +           (fkey_is_required(space->def->id, NULL) || trigger_list != NULL)) {
>                 /* Mask of OLD.* columns in use */
>                 /* TODO: Could use temporary registers here. */
>                 uint32_t mask =

Sorry.

diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index e8354f7e4..143803f9d 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -458,8 +458,7 @@ sql_generate_row_delete(struct Parse *parse, struct space *space,
	 * use for the old.* references in the triggers.
	 */
	if (space != NULL &&
-	    (fkey_is_required(space->def->id, NULL) ||
-	     trigger_list != NULL)) {
+	   (fkey_is_required(space, NULL) || trigger_list != NULL)) {

>>>> +/**
>>>>   * Add type and collation information to a column list based on
>>>>   * a SELECT statement.
>>>>   *
>>>> - * The column list presumably came from selectColumnNamesFromExprList().
>>>> - * The column list has only names, not types or collations.  This
>>>> - * routine goes through and adds the types and collations.
>>>> + * The column list presumably came from
>>>> + * selectColumnNamesFromExprList(). The column list has only
>>>> + * names, not types or collations.  This routine goes through
>>>> + * and adds the types and collations.
>>>> 
>>> Why did you change this comment?
>> Because the length of it's lines exceeded 65 chars.
> But this fix is not related to patch: it doesn’t even contain words like ’table’.

Ok, let's keep things to the point. Reverted changes.

>>>> @@ -2000,23 +2006,20 @@ sqlite3ResultSetOfSelect(Parse * pParse, Select * pSelect)
>>>>  	while (pSelect->pPrior)
>>>>  		pSelect = pSelect->pPrior;
>>>>  	user_session->sql_flags = savedFlags;
>>>> -	Table *table = sql_ephemeral_table_new(pParse, NULL);
>>>> -	if (table == NULL)
>>>> +	struct space *space = sql_ephemeral_space_new(pParse, NULL);
>>>> +
>>>> +	if (space == NULL)
>>>>  		return 0;
>>>> -	/* The sqlite3ResultSetOfSelect() is only used n contexts where lookaside
>>>> +	/* The sqlite3ResultSetOfSelect() is only used in contexts where lookaside
>>>>  	 * is disabled
>>>> 
>>> Extra diff.
>> Empty line is removed.
> I mean not (only) empty line, but fix of the comment. Nevermind.
> 
>>>>  	 */
>>>>  	assert(db->lookaside.bDisable);
>>>> -	table->nTabRef = 1;
>>>> -	table->tuple_log_count = DEFAULT_TUPLE_LOG_COUNT;
>>>> -	assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) == DEFAULT_TUPLE_LOG_COUNT);
>>>> -	sqlite3ColumnsFromExprList(pParse, pSelect->pEList, table);
>>>> -	sqlite3SelectAddColumnTypeAndCollation(pParse, table, pSelect);
>>>> +	sqlite3ColumnsFromExprList(pParse, pSelect->pEList, space->def);
>>>> +	sqlite3SelectAddColumnTypeAndCollation(pParse, space, pSelect);
>>>>  	if (db->mallocFailed) {
>>>> -		sqlite3DeleteTable(db, table);
>>>>  		return 0;
> return NULL;
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 79b8eafca..b30aa6200 100644

@@ -1998,13 +1993,13 @@ sqlite3ResultSetOfSelect(Parse * pParse, Select * pSelect)
	user_session->sql_flags &= SQLITE_ShortColNames;
	sqlite3SelectPrep(pParse, pSelect, 0);
	if (pParse->nErr)
-		return 0;
+		return NULL;
	while (pSelect->pPrior)
		pSelect = pSelect->pPrior;
	user_session->sql_flags = savedFlags;
	struct space *space = sql_ephemeral_space_new(pParse, NULL);
	if (space == NULL)
-		return 0;
+		return NULL;
	/* The sqlite3ResultSetOfSelect() is only used in contexts where lookaside
	 * is disabled
	 */
@@ -2012,7 +2007,7 @@ sqlite3ResultSetOfSelect(Parse * pParse, Select * pSelect)
	sqlite3ColumnsFromExprList(pParse, pSelect->pEList, space->def);
	sqlite3SelectAddColumnTypeAndCollation(pParse, space->def, pSelect);
	if (db->mallocFailed)
-		return 0;
+		return NULL;
	return space;
}

> 
>>>>  /**
>>>>   * Generate code for a DELETE FROM statement.
>>>> @@ -3603,10 +3581,11 @@ int sqlite3WhereOkOnePass(WhereInfo *, int *);
>>>>    /**
>>>>   * Generate code that will extract the iColumn-th column from
>>>> - * table pTab and store the column value in a register.
>>>> + * table defined by space_def and store the column value in a
>>>> + * register.
>>>>   *
>>>>   * An effort is made to store the column value in register iReg.
>>>> - * This is not garanteeed for GetColumn() - the result can be
>>>> + * This is not guaranteed for GetColumn() - the result can be
>>>> 
>>> Extra diff.
>> This diff fixes typo 'garanteeed' -> ‘guaranteed'
> It is not related to patch. Instead, it enlarges diff to be reviewed.

Ok, changes reverted.

> As for space_by_id() clean-up: you can make key_is_required() accept
> struct space instead of space_id. You can remove space_checks_expr_list()
> function.

Ok, done.

diff --git a/src/box/sql.h b/src/box/sql.h
index d0b654e9c..e7b3933b9 100644
--- a/src/box/sql.h
+++ b/src/box/sql.h
@@ -172,14 +172,6 @@ sql_expr_extract_select(struct Parse *parser, struct Select *select);
struct Expr*
space_column_default_expr(uint32_t space_id, uint32_t fieldno);
-/**
- * Get server checks list by space_id.
- * @param space_id Space ID.
- * @retval Checks list.
- */
-struct ExprList *
-space_checks_expr_list(uint32_t space_id);
-
/**
 * Return the number of bytes required to create a duplicate of the
 * expression passed as the first argument. The second argument is a

diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 599ece6d5..4b055529c 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -789,16 +763,6 @@ sql_column_collation(struct space_def *def, uint32_t column, uint32_t *coll_id)
	return field->coll;
}
-struct ExprList *
-space_checks_expr_list(uint32_t space_id)
-{
-	struct space *space;
-	space = space_by_id(space_id);
-	assert(space != NULL);
-	assert(space->def != NULL);
-	return space->def->opts.checks;
-}
-
int
vdbe_emit_open_cursor(struct Parse *parse_context, int cursor, int index_id,
		      struct space *space)

diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index e8354f7e4..143803f9d 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -149,7 +149,7 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list,
	assert(space != NULL);
	trigger_list = sql_triggers_exist(space->def, TK_DELETE, NULL, NULL);
	bool is_complex = trigger_list != NULL ||
-			  fkey_is_required(space->def->id, NULL);
+			  fkey_is_required(space, NULL);
	bool is_view = space->def->opts.is_view;
 	/* If table is really a view, make sure it has been

@@ -458,8 +458,7 @@ sql_generate_row_delete(struct Parse *parse, struct space *space,
	 * use for the old.* references in the triggers.
	 */
	if (space != NULL &&
-	    (fkey_is_required(space->def->id, NULL) ||
-	     trigger_list != NULL)) {
+	   (fkey_is_required(space, NULL) || trigger_list != NULL)) {
		/* Mask of OLD.* columns in use */
		/* TODO: Could use temporary registers here. */
		uint32_t mask =
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index 61cfb6fd7..28b53e7fd 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -421,7 +421,7 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src,
		VdbeCoverage(v);
	}
-	struct space *child_space = space_by_id(fkey->child_id);
+	struct space *child_space = src->a[0].space;
	assert(child_space != NULL);
	/*
	 * Create an Expr object representing an SQL expression
@@ -672,9 +672,8 @@ fkey_emit_check(struct Parse *parser, struct space *space, int reg_old,
}
 bool
-fkey_is_required(uint32_t space_id, const int *changes)
+fkey_is_required(struct space *space, const int *changes)
{
-	struct space *space = space_by_id(space_id);
	if (changes == NULL) {
		/*
		 * A DELETE operation. FK processing is required
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index f28567dd1..6bd716767 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -929,7 +929,7 @@ vdbe_emit_constraint_checks(struct Parse *parse_context, struct space *space,
	if (on_conflict == ON_CONFLICT_ACTION_DEFAULT)
		on_conflict = ON_CONFLICT_ACTION_ABORT;
	/* Test all CHECK constraints. */
-	struct ExprList *checks = space_checks_expr_list(def->id);
+	struct ExprList *checks = def->opts.checks;
	enum on_conflict_action on_conflict_check = on_conflict;
	if (on_conflict == ON_CONFLICT_ACTION_REPLACE)
		on_conflict_check = ON_CONFLICT_ACTION_ABORT;
@@ -1244,8 +1244,8 @@ xferOptimization(Parse * pParse,	/* Parser context */
			return 0;
	}
	/* Get server checks. */
-	ExprList *pCheck_src = space_checks_expr_list(src->def->id);
-	ExprList *pCheck_dest = space_checks_expr_list(dest->def->id);
+	ExprList *pCheck_src = src->def->opts.checks;
+	ExprList *pCheck_dest = dest->def->opts.checks;
	if (pCheck_dest != NULL &&
	    sqlite3ExprListCompare(pCheck_src, pCheck_dest, -1) != 0) {
		/* Tables have different CHECK constraints.  Ticket #2252 */

diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index c9ec9b1ca..c4fdccec0 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -4750,12 +4743,12 @@ fkey_emit_actions(struct Parse *parser, struct space *space, int reg_old,
 * changes[] array is set to -1. If the column is modified,
 * the value is 0 or greater.
 *
- * @param space_id Id of space to be modified.
+ * @param space Space to be modified.
 * @param changes Array of modified fields for UPDATE.
 * @retval True, if any foreign key processing will be required.
 */
bool
-fkey_is_required(uint32_t space_id, const int *changes);
+fkey_is_required(struct space *space, const int *changes);

diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index f7b7dc10d..a7affa954 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -201,7 +201,7 @@ sqlite3Update(Parse * pParse,		/* The parser context */
	 */
	pTabList->a[0].colUsed = 0;
-	hasFK = fkey_is_required(space->def->id, aXRef);
+	hasFK = fkey_is_required(space, aXRef);
 	/* Begin generating code. */
	v = sqlite3GetVdbe(pParse);

> Also this fix removes one space_by_id usage:
> 
> diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
> index 61cfb6fd7..238ab7891 100644
> --- a/src/box/sql/fkey.c
> +++ b/src/box/sql/fkey.c
> @@ -421,7 +421,7 @@ fkey_scan_children(struct Parse *parser, struct SrcList *src,
>                 VdbeCoverage(v);
>         }
>  -       struct space *child_space = space_by_id(fkey->child_id);
> +       struct space *child_space = src->a[0].space;

Done.

>> diff --git a/src/box/
>> schema.cc b/src/box/schema.cc
>> 
>> index 8625d92ea..2d12f01d0 100644
>> --- a/src/box/
>> schema.cc
>> 
>> +++ b/src/box/
>> schema.cc
>> 
>> @@ -193,8 +193,8 @@ space_cache_replace(struct space *old_space, struct space *new_space)
>>  			mh_strnptr_del(spaces_by_name, k, NULL);
>>  		}
>>  		/*
>> -		 * Insert @new_space into @spaces cache, replacing
>> -		 * @old_space if it's not NULL.
>> +		 * Insert @updated_space into @spaces cache,
>> +		 * replacing @old_space if it's not NULL.
>>  		 */
>>  		const struct mh_i32ptr_node_t node_p = { space_id(new_space),
>>  							 new_space };
>> @@ -209,7 +209,7 @@ space_cache_replace(struct space *old_space, struct space *new_space)
>>  		assert(old_space_by_id == old_space);
>>  		(void)old_space_by_id;
>>  		/*
>> -		 * Insert @new_space into @spaces_by_name cache.
>> +		 * Insert @updated_space into @spaces_by_name cache.
>>  		 */
> Why these changes are here?

Sorry, they should not be there (auto-refactor error)

diff --git a/src/box/schema.cc b/src/box/schema.cc
index 2d12f01d0..8625d92ea 100644
--- a/src/box/schema.cc
+++ b/src/box/schema.cc
@@ -193,8 +193,8 @@ space_cache_replace(struct space *old_space, struct space *new_space)
			mh_strnptr_del(spaces_by_name, k, NULL);
		}
		/*
-		 * Insert @updated_space into @spaces cache,
-		 * replacing @old_space if it's not NULL.
+		 * Insert @new_space into @spaces cache, replacing
+		 * @old_space if it's not NULL.
		 */
		const struct mh_i32ptr_node_t node_p = { space_id(new_space),
							 new_space };
@@ -209,7 +209,7 @@ space_cache_replace(struct space *old_space, struct space *new_space)
		assert(old_space_by_id == old_space);
		(void)old_space_by_id;
		/*
-		 * Insert @updated_space into @spaces_by_name cache.
+		 * Insert @new_space into @spaces_by_name cache.
		 */
		const char *name = space_name(new_space);
		uint32_t name_len = strlen(name);

>>  /**
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index cae0b3f6e..599ece6d5 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -247,7 +247,7 @@ sql_space_column_is_in_pk(struct space *space, uint32_t column)
>>  void
>>  sql_space_delete(struct sqlite3 *db, struct space *space)
>>  {
>> -	if (!space || !db || db->pnBytesFreed == 0)
>> +	if (space  != NULL || db != NULL|| db->pnBytesFreed == 0)
>>  		return;
>>    	if (space->def->opts.is_temporary) {
>> @@ -310,35 +310,13 @@ sqlite3CheckIdentifierName(Parse *pParse, char *zName)
>>  }
>>    struct index *
>> -sql_table_primary_key(const struct space *space)
>> +sql_space_primary_key(const struct space *space)
> It is used only in build.c, you can make this function static.

Done
@@ -309,7 +280,10 @@ sqlite3CheckIdentifierName(Parse *pParse, char *zName)
	return SQLITE_OK;
}
-struct index *
+/**
+ * Return the PRIMARY KEY index of a table.
+ */
+static struct index *
sql_space_primary_key(const struct space *space)
{
	if (space->index_count == 0 || space->index[0]->def->iid != 0)

@@ -3331,12 +3331,6 @@ void
sqlite3SelectAddColumnTypeAndCollation(Parse *, struct space_def *, Select *);
struct space *sqlite3ResultSetOfSelect(Parse *, Select *);
-/**
- * Return the PRIMARY KEY index of a table.
- */
-struct index *
-sql_space_primary_key(const struct space *space);
-
void sqlite3StartTable(Parse *, Token *, int);
void sqlite3AddColumn(Parse *, Token *, struct type_def *);

> The same is for sql_ephemeral_space_def_new() - lets make it static
> or even inline it (since it used only once). If the latter, then you can
> alloc space at once for space and def.

Done.

@@ -234,16 +226,6 @@ sql_expr_delete(struct sqlite3 *db, struct Expr *expr, bool extern_alloc);
struct space *
sql_ephemeral_space_new(struct Parse *parser, const char *name);
-/**
- * Create and initialize a new ephemeral space_def object.
- * @param parser SQL Parser object.
- * @param name Name of space to be created.
- * @retval NULL on memory allocation error, Parser state changed.
- * @retval not NULL on success.
- */
-struct space_def *
-sql_ephemeral_space_def_new(struct Parse *parser, const char *name);
-
/**
 * Duplicate Expr list.
 * The flags parameter contains a combination of the EXPRDUP_XXX

@@ -1233,7 +1233,14 @@ space_column_default_expr(uint32_t space_id, uint32_t fieldno)
	return space->def->fields[fieldno].default_value_expr;
}
-struct space_def *
+/**
+ * Create and initialize a new ephemeral space_def object.
+ * @param parser SQL Parser object.
+ * @param name Name of space to be created.
+ * @retval NULL on memory allocation error, Parser state changed.
+ * @retval not NULL on success.
+ */
+static struct space_def *
sql_ephemeral_space_def_new(struct Parse *parser, const char *name)
{
	struct space_def *def = NULL;

>  void
> -sqlite3DeleteTable(sqlite3 * db, Table * pTable)
> +sql_space_delete(struct sqlite3 *db, struct space *space, bool is_list_del)
>  {
> -       /* Do not delete the table until the reference count reaches zero. */
> -       if (!pTable)
> -               return;
> -       if (((!db || db->pnBytesFreed == 0) && (--pTable->nTabRef) > 0))
> +       if (space  != NULL || db != NULL || db->pnBytesFreed == 0)
>                 return;
> 
> You refactored this if-clause in a wrong way.
> 
> if (!db) <=> if (db == NULL)
> if (!space) <=> if (space == NULL)
> 
> Fix it since it leads to numerous leaks.

Sorry, fixed.

> What is more, you don’t need to call this function in sqlite3SrcListDelete().
> We must free index defs only during table creation. In this case, they are
> stored in updated_space. Check out diff:
> 
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 599ece6d5..e6da4be97 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -232,35 +232,6 @@ sql_space_column_is_in_pk(struct space *space, uint32_t column)
>         return false;
>  }
>  -/**
> - * Remove the memory data structures associated with the given
> - * space. The only case when some parts of space must be
> - * deleted explicitly is when it comes from building
> - * routine (i.e. it was born during CREATE TABLE
> - * processing). In this case only index defs and check
> - * expressions are allocated using malloc; the rest - on region.
> - * This case is identified by 'is_temporary' flag == TRUE.
> - *
> - * @param db Database handler.
> - * @param space Space to be deleted.
> - */
> -void
> -sql_space_delete(struct sqlite3 *db, struct space *space)
> -{
> -       if (space  != NULL || db != NULL|| db->pnBytesFreed == 0)
> -               return;
> -
> -       if (space->def->opts.is_temporary) {
> -               for (uint32_t i = 0; i < space->index_count; ++i)
> -                       index_def_delete(space->index[i]->def);
> -               /**
> -                * Do not delete space and space->def allocated
> -                * on region.
> -                */
> -               sql_expr_list_delete(db, space->def->opts.checks);
> -       }
> -}
> -
>  /*
>   * Given a token, return a string that consists of the text of that
>   * token.  Space to hold the returned string
> @@ -2813,7 +2784,17 @@ sqlite3SrcListDelete(sqlite3 * db, SrcList * pList)
>                         sqlite3DbFree(db, pItem->u1.zIndexedBy);
>                 if (pItem->fg.isTabFunc)
>                         sql_expr_list_delete(db, pItem->u1.pFuncArg);
> -               sql_space_delete(db, pItem->space);
> +               /*
> +                * Space is either not temporary which means that
> +                * it came from space cache; or space is temporary
> +                * but has no indexes and check constraints.
> +                * The latter proves that it is not the space
> +                * which might come from CREATE TABLE routines.
> +                */
> +               assert(pItem->space == NULL ||
> +                      !pItem->space->def->opts.is_temporary ||
> +                       (pItem->space->index == NULL &&
> +                        pItem->space->def->opts.checks == NULL));
>                 sql_select_delete(db, pItem->pSelect);
>                 sql_expr_delete(db, pItem->pOn, false);
>                 sqlite3IdListDelete(db, pItem->pUsing);
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index c9ec9b1ca..83b78507a 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -3453,7 +3453,6 @@ sql_store_select(struct Parse *parse_context, struct Select *select);
>    void
>  sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
> -void sql_space_delete(sqlite3 *, struct space *);
>  void sqlite3Insert(Parse *, SrcList *, Select *, IdList *,
>                    enum on_conflict_action);
>  void *sqlite3ArrayAllocate(sqlite3 *, void *, int, int *, int *);
> diff --git a/src/box/sql/tokenize.c b/src/box/sql/tokenize.c
> index 3102c75bd..99f87f81c 100644
> --- a/src/box/sql/tokenize.c
> +++ b/src/box/sql/tokenize.c
> @@ -416,6 +416,27 @@ sql_token(const char *z, int *type, bool *is_reserved)
>         return i;
>  }
>  +/**
> + * This function is called to release parsing artifacts
> + * during table creation. The only objects allocated using
> + * malloc are index defs and check constraints.
> + * Note that this functions can't be called on ordinary
> + * space object. It is purpose to clean-up parser->updated_space.
> + *
> + * @param db Database handler.
> + * @param space Space to be deleted.
> + */
> +static void
> +parser_space_delete(struct sqlite3 *db, struct space *space)
> +{
> +       if (space == NULL || db == NULL || db->pnBytesFreed == 0)
> +               return;x
> +       assert(space->def->opts.is_temporary);
> +       for (uint32_t i = 0; i < space->index_count; ++i)
> +               index_def_delete(space->index[i]->def);
> +       sql_expr_list_delete(db, space->def->opts.checks);
> +}
> +
>  /*
>   * Run the parser on the given SQL string.  The parser structure is
>   * passed in.  An SQLITE_ status code is returned.  If an error occurs
> @@ -529,7 +550,7 @@ sqlite3RunParser(Parse * pParse, const char *zSql, char **pzErrMsg)
>                 sqlite3VdbeDelete(pParse->pVdbe);
>                 pParse->pVdbe = 0;
>         }
> -       sql_space_delete(db, pParse->updated_space);
> +       parser_space_delete(db, pParse->updated_space);
> 
> You can even try to move index defs and checks allocations
> to the region. In this case, we won’t have to free anything at all.
> 
> 
> 
Done. The diff is identical to yours.