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 3028E23BB3 for ; Tue, 3 Jul 2018 08:13:10 -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 9FFq9xTdF06Y for ; Tue, 3 Jul 2018 08:13:10 -0400 (EDT) Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (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 DFB522062B for ; Tue, 3 Jul 2018 08:13:09 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v8.5] sql: add index_def to struct Index References: <9A0A687D-2E0B-4AB9-B223-1A1287817824@tarantool.org> <8900ae2a-59e8-d69a-7f5d-95436b23214a@tarantool.org> <0e795fb5-1ee8-e7e6-65e4-7347787f3248@tarantool.org> <898ec1ac-51b9-13a6-6298-3a20eeac5b2b@tarantool.org> <76c1ec30-4f33-bc91-8845-72a99fc4ef0f@tarantool.org> <6fbc3849-a204-6cd9-82cd-2fb22769ccf0@tarantool.org> <9a6283bc-a947-9070-8a92-22d84ff759ae@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 3 Jul 2018 15:13:06 +0300 MIME-Version: 1.0 In-Reply-To: <9a6283bc-a947-9070-8a92-22d84ff759ae@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, Ivan Koptelov , Nikita Pettik Hello. Thanks for the fixes! On 03/07/2018 12:46, Ivan Koptelov wrote: > Thank for the review. Previous mail contains patch with an error. Here it is fixed. >> Hello. Thanks for the fixes! See my 6 comments below. >> >> And I have pushed more fixes on the branch. Please, >> look and squash. >> >>> Now every sqlite struct Index is created with tnt struct >>> index_def inside. This allows us to use tnt index_def >>> in work with sqlite indexes in the same manner as with >>> tnt index and is a step to remove sqlite Index with >>> tnt index. >>> Fields coll_array, coll_id_array, aiColumn, sort_order >>> and zName are removed from Index. All usages of this >>> fields changed to usage of corresponding index_def >>> fields. >>> index_is_unique(), sql_index_collation() and >>> index_column_count() are removed with calls of >>> index_def corresponding fields. >>> >>> Closes: #3369 >>> >>> --- >>> Branch: >>> https://github.com/tarantool/tarantool/tree/sb/gh-3369-use-index-def-in-select-and-where >>> Issue:https://github.com/tarantool/tarantool/issues/3369 >>> >>>   src/box/sql.c                        |  54 ++- >>>   src/box/sql/analyze.c                |  85 ++--- >>>   src/box/sql/build.c                  | 713 +++++++++++++++++------------------ >>>   src/box/sql/delete.c                 |  10 +- >>>   src/box/sql/expr.c                   |  61 +-- >>>   src/box/sql/fkey.c                   | 132 +++---- >>>   src/box/sql/insert.c                 | 145 ++++--- >>>   src/box/sql/pragma.c                 |  30 +- >>>   src/box/sql/select.c                 |   2 +- >>>   src/box/sql/sqliteInt.h              | 111 ++---- >>>   src/box/sql/update.c                 |  39 +- >>>   src/box/sql/vdbeaux.c                |   2 +- >>>   src/box/sql/vdbemem.c                |  21 +- >>>   src/box/sql/where.c                  | 180 ++++----- >>>   src/box/sql/wherecode.c              | 102 ++--- >>>   test/sql-tap/colname.test.lua        |   4 +- >>>   test/sql/message-func-indexes.result |   8 +- >>>   17 files changed, 821 insertions(+), 878 deletions(-) >>> >>> - >>> -/** Return true if given index is unique and not nullable. */ >>> -bool >>> -index_is_unique_not_null(const Index *idx) >> >> 4. Same as on the previous review. Still is used in a pair of places. > Are you sure? I searched through the whole project and didn't find it. > There is only a variable with the same name in one place. Sorry, my fault. It is just a variable. > diff --git a/src/box/sql/where.c b/src/box/sql/where.c > index 85143ed20..7ca02095f 100644 > --- a/src/box/sql/where.c > +++ b/src/box/sql/where.c > @@ -3058,6 +3064,8 @@ whereLoopAddBtree(WhereLoopBuilder * pBuilder, /* WHERE clause information */ > if (pSrc->pIBIndex) > break; > } > + if (&sPk == pProbe && sPk.def != NULL) > + index_def_delete(sPk.def); It will not be deleted since pProbe is changed in the cycle above. I have checked it by putting assert(false) under this 'if'. And I have already fixed it on the branch. > return rc; > } > I have pushed my last fixes on the branch. Please, look, squash and then LGTM. Nikita, please, review after the squash.