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 C825520F6F for ; Fri, 18 May 2018 17:07:22 -0400 (EDT) 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 iBY09OPPDLAw for ; Fri, 18 May 2018 17:07:22 -0400 (EDT) Received: from smtp55.i.mail.ru (smtp55.i.mail.ru [217.69.128.35]) (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 17FD520F65 for ; Fri, 18 May 2018 17:07:21 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH] sql: refactor SQL delete to allow Lua spaces References: <87358913270ee98c98bc276209289d6b7ac496b6.1526650490.git.kyukhin@tarantool.org> From: Vladislav Shpilevoy Message-ID: <946fafd7-9e8a-0263-ecaa-02df83c82223@tarantool.org> Date: Sat, 19 May 2018 00:07:18 +0300 MIME-Version: 1.0 In-Reply-To: <87358913270ee98c98bc276209289d6b7ac496b6.1526650490.git.kyukhin@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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: tarantool-patches@freelists.org, Kirill Yukhin Hello. Thanks for the patch! See 1 comment below. On 18/05/2018 16:36, Kirill Yukhin wrote: > Branch: https://github.com/tarantool/tarantool/tree/kyukhin/gh-3235-sql-truncate-on-lua-spaces > Issue: https://github.com/tarantool/tarantool/issues/3235 > > This is a first step toward fully-featured deletion > of spaces created in Lua by means of SQL language. > This change to handle most simple case: > DELETE * FROM and will be improved in > nearest future. > > Part of #3235 > --- > src/box/sql/analyze.c | 3 +- > src/box/sql/build.c | 3 +- > src/box/sql/delete.c | 69 ++++++++++++++++++++++++++----------------- > src/box/sql/vdbe.c | 2 +- > test/sql-tap/delete1.test.lua | 23 ++++++++++++++- > 5 files changed, 69 insertions(+), 31 deletions(-) > > diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c > index f4d248e..dfc91cb 100644 > --- a/src/box/sql/delete.c > +++ b/src/box/sql/delete.c > @@ -29,6 +29,7 @@ > * SUCH DAMAGE. > */ > > +#include "box/box.h" > #include "box/session.h" > #include "box/schema.h" > #include "sqliteInt.h" > @@ -86,35 +87,50 @@ sql_table_delete_from(struct Parse *parse, struct SrcList *tab_list, > + bool is_complex = false; > + const char *tab_name = tab_list->a->zName; > + if (sqlite3LocateTable(parse, LOCATE_NOERR, tab_name) == NULL) { > + space_id = box_space_id_by_name(tab_name, > + strlen(tab_name)); > + if (space_id == BOX_ID_NIL) > + goto delete_from_cleanup; > + } else { > + table = sql_list_lookup_table(parse, tab_list); > + if (table == NULL) > + goto delete_from_cleanup; > + space_id = SQLITE_PAGENO_TO_SPACEID(table->tnum); > + trigger_list =sqlite3TriggersExist(table, TK_DELETE, > + NULL, NULL); > + is_complex = trigger_list != NULL || > + sqlite3FkRequired(table, NULL); > + } > + space = space_by_id(space_id); > + assert(space != NULL); > + > + bool is_view = space->def->opts.is_view; > > /* If table is really a view, make sure it has been > * initialized. > */ > - if (sqlite3ViewGetColumnNames(parse, table)) > - goto delete_from_cleanup; > + if (is_view) { 1. sqlite3ViewGetColumnNames makes the check too. Lets turn it into assertion inside sqlite3ViewGetColumnNames, and add the 'if (is_view)' in other places, where it is called and the check is necessary. For example, in selectExpander it is checked twice too. > + if (sqlite3ViewGetColumnNames(parse, table)) > + goto delete_from_cleanup; > > > /* Assign cursor numbers to the table and all its indices.