From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id A813726404 for ; Wed, 6 Feb 2019 12:17:40 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Ej_YZmH8IT83 for ; Wed, 6 Feb 2019 12:17:40 -0500 (EST) Received: from smtp62.i.mail.ru (smtp62.i.mail.ru [217.69.128.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id B2904263FD for ; Wed, 6 Feb 2019 12:17:39 -0500 (EST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.0 \(3445.100.39\)) Subject: [tarantool-patches] [PATCH] sql: remove struct Table From: "i.koptelov" In-Reply-To: <3438F900-EF6D-4475-B3F1-BD4971F78186@tarantool.org> Date: Wed, 6 Feb 2019 20:17:34 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <4c2b87b5-9e83-f96f-4350-edf7a12e5593@tarantool.org> <9f612f00-6222-e3cb-642c-04bfec116fd4@tarantool.org> <3438F900-EF6D-4475-B3F1-BD4971F78186@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: "n.pettik" Cc: tarantool-patches@freelists.org >> On 1 Feb 2019, at 14:05, Ivan Koptelov = wrote: >>=20 >> 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=E2=80=99t 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. >>>=20 >>> sql: remove struct Table >>>=20 >>> 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. >>>=20 >>> Note that after commit subject's prefix we use lower-case. >>>=20 >> Fixed. >>> Also, for some reason content of this letter looks very strange: >>> at least it contains unusual fonts=E2=80=A6 I am not complaining but >>> are you sure that you set up git-email correctly? >>>=20 > It still looks weird. Please, verify that text you send is plain. >=20 >>>> + >>>> #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 !=3D NULL && >>>> - (fkey_is_required(table->def->id, NULL) || trigger_list !=3D = NULL)) { >>>> + if (space !=3D NULL && >>>> + (fkey_is_required(space->def->id, NULL) || >>>> + trigger_list !=3D NULL)) { >>>>=20 >>> 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 !=3D NULL && > - (fkey_is_required(space->def->id, NULL) || > - trigger_list !=3D NULL)) { > + (fkey_is_required(space->def->id, NULL) || trigger_list !=3D= NULL)) { > /* Mask of OLD.* columns in use */ > /* TODO: Could use temporary registers here. */ > uint32_t mask =3D 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 !=3D NULL && - (fkey_is_required(space->def->id, NULL) || - trigger_list !=3D NULL)) { + (fkey_is_required(space, NULL) || trigger_list !=3D 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. >>>>=20 >>> 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=E2=80=99t even contain = words like =E2=80=99table=E2=80=99. Ok, let's keep things to the point. Reverted changes. >>>> @@ -2000,23 +2006,20 @@ sqlite3ResultSetOfSelect(Parse * pParse, = Select * pSelect) >>>> while (pSelect->pPrior) >>>> pSelect =3D pSelect->pPrior; >>>> user_session->sql_flags =3D savedFlags; >>>> - Table *table =3D sql_ephemeral_table_new(pParse, NULL); >>>> - if (table =3D=3D NULL) >>>> + struct space *space =3D sql_ephemeral_space_new(pParse, NULL); >>>> + >>>> + if (space =3D=3D NULL) >>>> return 0; >>>> - /* The sqlite3ResultSetOfSelect() is only used n contexts where = lookaside >>>> + /* The sqlite3ResultSetOfSelect() is only used in contexts where = lookaside >>>> * is disabled >>>>=20 >>> Extra diff. >> Empty line is removed. > I mean not (only) empty line, but fix of the comment. Nevermind. >=20 >>>> */ >>>> assert(db->lookaside.bDisable); >>>> - table->nTabRef =3D 1; >>>> - table->tuple_log_count =3D DEFAULT_TUPLE_LOG_COUNT; >>>> - assert(sqlite3LogEst(DEFAULT_TUPLE_COUNT) =3D=3D = 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 &=3D SQLITE_ShortColNames; sqlite3SelectPrep(pParse, pSelect, 0); if (pParse->nErr) - return 0; + return NULL; while (pSelect->pPrior) pSelect =3D pSelect->pPrior; user_session->sql_flags =3D savedFlags; struct space *space =3D sql_ephemeral_space_new(pParse, NULL); if (space =3D=3D 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; } >=20 >>>> /** >>>> * 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 >>>>=20 >>> Extra diff. >> This diff fixes typo 'garanteeed' -> =E2=80=98guaranteed' > 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 =3D space_by_id(space_id); - assert(space !=3D NULL); - assert(space->def !=3D 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 !=3D NULL); trigger_list =3D sql_triggers_exist(space->def, TK_DELETE, NULL, = NULL); bool is_complex =3D trigger_list !=3D NULL || - fkey_is_required(space->def->id, NULL); + fkey_is_required(space, NULL); bool is_view =3D 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 !=3D NULL && - (fkey_is_required(space->def->id, NULL) || - trigger_list !=3D NULL)) { + (fkey_is_required(space, NULL) || trigger_list !=3D NULL)) { /* Mask of OLD.* columns in use */ /* TODO: Could use temporary registers here. */ uint32_t mask =3D 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 =3D space_by_id(fkey->child_id); + struct space *child_space =3D src->a[0].space; assert(child_space !=3D 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 =3D space_by_id(space_id); if (changes =3D=3D 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 =3D=3D ON_CONFLICT_ACTION_DEFAULT) on_conflict =3D ON_CONFLICT_ACTION_ABORT; /* Test all CHECK constraints. */ - struct ExprList *checks =3D space_checks_expr_list(def->id); + struct ExprList *checks =3D def->opts.checks; enum on_conflict_action on_conflict_check =3D on_conflict; if (on_conflict =3D=3D ON_CONFLICT_ACTION_REPLACE) on_conflict_check =3D ON_CONFLICT_ACTION_ABORT; @@ -1244,8 +1244,8 @@ xferOptimization(Parse * pParse, /* Parser = context */ return 0; } /* Get server checks. */ - ExprList *pCheck_src =3D space_checks_expr_list(src->def->id); - ExprList *pCheck_dest =3D space_checks_expr_list(dest->def->id); + ExprList *pCheck_src =3D src->def->opts.checks; + ExprList *pCheck_dest =3D dest->def->opts.checks; if (pCheck_dest !=3D NULL && sqlite3ExprListCompare(pCheck_src, pCheck_dest, -1) !=3D 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 =3D 0; - hasFK =3D fkey_is_required(space->def->id, aXRef); + hasFK =3D fkey_is_required(space, aXRef); /* Begin generating code. */ v =3D sqlite3GetVdbe(pParse); > Also this fix removes one space_by_id usage: >=20 > 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 =3D space_by_id(fkey->child_id); > + struct space *child_space =3D src->a[0].space; Done. >> diff --git a/src/box/ >> schema.cc b/src/box/schema.cc >>=20 >> index 8625d92ea..2d12f01d0 100644 >> --- a/src/box/ >> schema.cc >>=20 >> +++ b/src/box/ >> schema.cc >>=20 >> @@ -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 =3D { = 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 =3D=3D 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 =3D { = 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 =3D=3D 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 =3D space_name(new_space); uint32_t name_len =3D 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 =3D=3D 0) >> + if (space !=3D NULL || db !=3D NULL|| db->pnBytesFreed =3D=3D = 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 =3D=3D 0 || space->index[0]->def->iid !=3D = 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 =3D 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 =3D=3D 0) && (--pTable->nTabRef) = > 0)) > + if (space !=3D NULL || db !=3D NULL || db->pnBytesFreed =3D=3D = 0) > return; >=20 > You refactored this if-clause in a wrong way. >=20 > if (!db) <=3D> if (db =3D=3D NULL) > if (!space) <=3D> if (space =3D=3D NULL) >=20 > Fix it since it leads to numerous leaks. Sorry, fixed. > What is more, you don=E2=80=99t 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: >=20 > 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 =3D=3D TRUE. > - * > - * @param db Database handler. > - * @param space Space to be deleted. > - */ > -void > -sql_space_delete(struct sqlite3 *db, struct space *space) > -{ > - if (space !=3D NULL || db !=3D NULL|| db->pnBytesFreed =3D=3D = 0) > - return; > - > - if (space->def->opts.is_temporary) { > - for (uint32_t i =3D 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 =3D=3D NULL || > + !pItem->space->def->opts.is_temporary || > + (pItem->space->index =3D=3D NULL && > + pItem->space->def->opts.checks =3D=3D 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 =3D=3D NULL || db =3D=3D NULL || db->pnBytesFreed =3D=3D= 0) > + return;x > + assert(space->def->opts.is_temporary); > + for (uint32_t i =3D 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 =3D 0; > } > - sql_space_delete(db, pParse->updated_space); > + parser_space_delete(db, pParse->updated_space); >=20 > You can even try to move index defs and checks allocations > to the region. In this case, we won=E2=80=99t have to free anything at = all. >=20 >=20 >=20 Done. The diff is identical to yours.=