From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 0BB0346970F for ; Sat, 30 Nov 2019 04:03:21 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: <44d1754c-ee10-aac5-3fdf-66aad8b2fd70@tarantool.org> Date: Sat, 30 Nov 2019 02:03:19 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v2 0/2] Add constraint names hash table to space List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Roman Khabibov , tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! See 2 comments below! On 28/11/2019 19:34, Roman Khabibov wrote: > I have made essential changes to the previous patch, so I decided > to send it as v2. > > 1) I still don't understand, why we need to store constraint id. If we have the > query "ALTER TABLE T DROP CONSTRAINT C", we just get struct space by its name, > then find the corresponding constraint_def node by the name and emit opcode on > replace in _index/_fk_constraint/_ck_constraint depending on > constraint_def->type. 1. ID is a primary index. You can delete by ID. Is it always possible to delete by name? Do all the constraint spaces have a unique index over name consisting of one column? > > 2) space_delete() > I don't know how to check, if the hash table is empty, because it jsut freed. 2. Mhash provides mh_size() method. Add an assert, that it is it == 0. > > salad/mhash.h > void > _mh(delete)(struct _mh(t) *h) > { > if (h->shadow->p) { > free(h->shadow->p); > free(h->shadow->b); > memset(h->shadow, 0, sizeof(*h->shadow)); > } > free(h->shadow); > free(h->b); > free(h->p); > free(h); > } > Roman Khabibov (2): > box: introduce constraint names hash table > sql: make constraint operations transactional > > src/box/CMakeLists.txt | 1 + > src/box/alter.cc | 427 +++++++++++++++++++++++++++++++---- > src/box/constraint_def.c | 59 +++++ > src/box/constraint_def.h | 83 +++++++ > src/box/space.c | 56 +++++ > src/box/space.h | 39 ++++ > test/sql/constraint.result | 190 ++++++++++++++++ > test/sql/constraint.test.lua | 84 +++++++ > 8 files changed, 892 insertions(+), 47 deletions(-) > create mode 100755 src/box/constraint_def.c > create mode 100755 src/box/constraint_def.h > create mode 100644 test/sql/constraint.result > create mode 100755 test/sql/constraint.test.lua >