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

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

[Nikita Pettik]

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.
 	 *
-- 
2.15.1

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

[Vladislav Shpilevoy]

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.
>   	 *
> 

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

[Kirill Yukhin]

Hello,
On 07 июн 15:31, Vladislav Shpilevoy wrote:
> 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.
I've checked the patch into 2.0 branch.

--
Regards, Kirill Yukhin