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 ABB762651F for ; Fri, 8 Jun 2018 16:05:37 -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 zmXFYjWwEcvA for ; Fri, 8 Jun 2018 16:05:37 -0400 (EDT) Received: from smtp21.mail.ru (smtp21.mail.ru [94.100.179.250]) (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 ED5AA2651B for ; Fri, 8 Jun 2018 16:05:36 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH] sql: rework VIEW internals References: <1528129571.147907906@f369.i.mail.ru> <374C812C-1387-42D1-B1B1-EE8FEDAA70A4@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Fri, 8 Jun 2018 23:05:33 +0300 MIME-Version: 1.0 In-Reply-To: <374C812C-1387-42D1-B1B1-EE8FEDAA70A4@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Language: en-US Content-Transfer-Encoding: 8bit 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, "n.pettik" Thanks for the fixes! I have force pushed another portion of minor ones (see below). Now the patchset LGTM. diff --git a/src/box/alter.cc b/src/box/alter.cc index 8440aff64..762b99dbc 100644 --- a/src/box/alter.cc +++ b/src/box/alter.cc @@ -1443,7 +1443,7 @@ alter_space_move_indexes(struct alter_space *alter, uint32_t begin, /** * Walk through all spaces from 'FROM' clause of given select, - * and update their view references counters. + * and update their view reference counters. * * @param select Tables from this select to be updated. * @param update_value +1 on view creation, -1 on drop. diff --git a/src/box/sql.h b/src/box/sql.h index fc068abbf..834f951c4 100644 --- a/src/box/sql.h +++ b/src/box/sql.h @@ -65,7 +65,6 @@ sql_get(); struct Expr; struct Parse; struct Select; -struct SrcList; struct Table; /** diff --git a/test/sql/view_delayed_wal.result b/test/sql/view_delayed_wal.result index 3ed7694ef..50efbee37 100644 --- a/test/sql/view_delayed_wal.result +++ b/test/sql/view_delayed_wal.result @@ -49,9 +49,10 @@ box.space.V1 --- - null ... +-- -- In the same way, we have to drop all referenced spaces before -- dropping view, since view reference counter of space to be --- dropped is checked before firing on_commit trigger. +-- dropped is checked before firing on_commit trigger. -- box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY)') --- diff --git a/test/sql/view_delayed_wal.test.lua b/test/sql/view_delayed_wal.test.lua index b8412a666..630c52baf 100644 --- a/test/sql/view_delayed_wal.test.lua +++ b/test/sql/view_delayed_wal.test.lua @@ -19,9 +19,10 @@ fiber.sleep(0.1) box.space.T1 box.space.V1 +-- -- In the same way, we have to drop all referenced spaces before -- dropping view, since view reference counter of space to be --- dropped is checked before firing on_commit trigger. +-- dropped is checked before firing on_commit trigger. -- box.sql.execute('CREATE TABLE t2 (id INT PRIMARY KEY)') box.sql.execute('CREATE VIEW view2 AS SELECT * FROM t2') On 08/06/2018 16:17, n.pettik wrote: > >> On 7 Jun 2018, at 23:06, Vladislav Shpilevoy wrote: >> >> Thanks for the fixes! See 1 comment below and a commit with my >> fixes on the branch right after your. Most likely, my fixes are >> not passing tests, so please, finish them. I fixed Select * leak on >> exception during trigger_new_xc, and error message. + some style >> fixes. > > Thanks for fixes, I have squashed them and tests seem to pass > (Except for 1 run: box/sql.test.lua once failed on timeout, but I’m not sure > that it is my fault: locally on my machine it works fine both as single run and within suite). > The only change I made - added fiber.sleep(0.1) in tests after > box.error.injection.set("ERRINJ_WAL_DELAY", false) > Without it test turns out to be flaky. > >>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c >>> index 28e4d7a4d..53bb53ab8 100644 >>> --- a/src/box/sql/build.c >>> +++ b/src/box/sql/build.c >>> @@ -2009,230 +2007,99 @@ sqlite3EndTable(Parse * pParse, /* Parse context */ >>> int >>> -sql_view_column_names(struct Parse *parse, struct Table *table) >>> +sql_view_assign_cursors(struct Parse *parse, const char *view_stmt) >>> { >>> + assert(view_stmt != NULL); >>> + struct sqlite3 *db = parse->db; >>> + struct Select *select = sql_view_compile(db, view_stmt); >>> if (select == NULL) >>> return -1; >>> - int n = parse->nTab; >>> sqlite3SrcListAssignCursors(parse, select->pSrc); >> >> Where do you free the struct Select? > > Nowhere, shame on me.. > > +++ b/src/box/sql/build.c > @@ -2090,6 +2090,7 @@ sql_view_assign_cursors(struct Parse *parse, const char *view_stmt) > if (select == NULL) > return -1; > sqlite3SrcListAssignCursors(parse, select->pSrc); > + sql_select_delete(db, select); > >