Re: [Tarantool-patches] [PATCH v2 0/3] Add constraint names hash table to space

[Roman Khabibov <roman.habibov@tarantool.org>]

> On Nov 30, 2019, at 04:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> 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?
I know a space_id. So, I will delete tuples from _index, _ck, _fk by their unique
composite indexes:
+----------+------------+
| space_id | name/iid   |
+----------+------------+

If we want to delete by ID, let’s add it to _ck and _fk and keep it in struct constraint_def also. But, IMO, it is not about my patchset.

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

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