From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (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 DE1AE46970E for ; Mon, 30 Dec 2019 01:28:41 +0300 (MSK) Date: Mon, 30 Dec 2019 00:28:39 +0200 From: Nikita Pettik Message-ID: <20191229222839.GB44588@tarantool.org> References: <77b2090c-4bfd-c42c-e744-3395eef2e8a3@tarantool.org> <721EA98D-2EC6-4C4C-97B0-6BCEBE889EDA@tarantool.org> <20007117-f560-e06d-16a5-67e811c8f9f8@tarantool.org> <88F2A14C-2D86-4C17-90F1-4B0388CF71C9@tarantool.org> <20191228001833.GA45089@tarantool.org> <20191229000717.GB44517@tarantool.org> <741cb846-6f91-1427-9e18-3d4be6855f5e@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <741cb846-6f91-1427-9e18-3d4be6855f5e@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 2/3] box: make constraint operations transactional List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org On 29 Dec 18:51, Vladislav Shpilevoy wrote: > >>>> index c660b1c70..094a63ee1 100644 > >>>> --- a/src/box/errcode.h > >>>> +++ b/src/box/errcode.h > >>>> @@ -222,7 +222,7 @@ struct errcode_record { > >>>> /*167 */_(ER_CREATE_FK_CONSTRAINT, "Failed to create foreign key constraint '%s': %s") \ > >>>> /*168 */_(ER_DROP_FK_CONSTRAINT, "Failed to drop foreign key constraint '%s': %s") \ > >>>> /*169 */_(ER_NO_SUCH_CONSTRAINT, "Constraint %s does not exist") \ > >>>> - /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s already exists") \ > >>>> + /*170 */_(ER_CONSTRAINT_EXISTS, "Constraint %s '%s' already exists in space '%s'") \ > >>> > >>> I'd better place type of constraint at the beggining of sentence: > >>> > >>> Check constraint xxx already exists > >>> Foreign key constraint ... > >>> Unique constraint ... > >>> > >>> Sounds way much better, doesn't it? > >> > >> It is subjective. I changed it to this order on purpose. > >> 1) Because I wanted to go from more common term to a more > >> concrete: 'Constraint' - common, - more > >> concrete, - even more concrete. > >> 2) Because when I see ' constraint already exists ...' > >> for me it looks like someone tried to create constraint of > >> type . But it is not always so. Type is not the most > >> important part of the message. > > > > Well, I think it may turn out to be good argument. Why do > > you add type of constraint to message at all? My guess > > is to simplify user's life and make finding constraint > > with given name easy. If so, does it make sense? > > Your guess is right. I wanted to make it simpler to determine > with what a constraint there is a conflict. So as a user could > do rename of index/fk/ck/whatever-else which occupies the needed > name. I mean if user finds out that name is already occupied, one is likely to use another name rather than drop/rename existing constraint. In this case type of constraint is not vital. Anyway, patch LGTM. This is second one approve so it can be pushed, but Sergos wrote that he is concerned about memory management during constraint id creation/drop. Sergos, did you find any issues, or we can push this patch?