From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <korablev@tarantool.org>
Received: from smtp41.i.mail.ru (smtp41.i.mail.ru [94.100.177.101])
 (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 16107445320
 for <tarantool-patches@dev.tarantool.org>;
 Mon,  3 Aug 2020 11:42:36 +0300 (MSK)
Date: Mon, 3 Aug 2020 08:42:35 +0000
From: Nikita Pettik <korablev@tarantool.org>
Message-ID: <20200803084235.GA8230@tarantool.org>
References: <20200724200633.12122-1-i.kosarev@tarantool.org>
 <20200730142620.GA25238@tarantool.org>
 <1596201198.417099977@f747.i.mail.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <1596201198.417099977@f747.i.mail.ru>
Subject: Re: [Tarantool-patches] [PATCH] tuple: drop extra restrictions for
	multikey index
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Ilya Kosarev <i.kosarev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

On 31 Jul 16:13, Ilya Kosarev wrote:
> 
> Hi!
>  
> Thanks for your review.
>  
> See my answers below.
>  
> I will send v2 of the patch but i think 2 questions above should be clarified.
>   
> >Четверг, 30 июля 2020, 17:26 +03:00 от Nikita Pettik <korablev@tarantool.org>:
> > 
> >On 24 Jul 23:06, Ilya Kosarev wrote:
> >
> >I *strongly dislike* that the patch is in fact should be 10 lines
> >but instead we have 250 diff lines. Please elaborate on that
> >(comments above are mostly about that).
> Well, this patch reverts the majority of changes from
> 4cf94ef8cb90b84ea71f313cff3e016f85894fd5 (tuple: make fields nullable
> by default except array/map). That is where the size comes from.

Then you are able simply to revert that patch and introduce new one.
But I'd provide only required fixes.

> >> diff --git a/src/box/memtx_space.c b/src/box/memtx_space.c
> >> index 8452ab430..875592026 100644
> >> --- a/src/box/memtx_space.c
> >> +++ b/src/box/memtx_space.c
> >> @@ -589,9 +589,7 @@ memtx_space_ephemeral_rowid_next(struct space *space, uint64_t *rowid)
> >> static int
> >> memtx_space_check_index_def(struct space *space, struct index_def *index_def)
> >> {
> >> - struct key_def *key_def = index_def->key_def;
> >> -
> >> - if (key_def->is_nullable) {
> >> + if (index_def->key_def->is_nullable) {
> >
> >Again I see this redundant diff. Please drop it, it doesn't fix
> >refactor/fix anything but makes diff bigger and complicates git
> >history.
> The problem is that this is the reversion of that diff. Well, I guess
> as far as that patch was pushed to master we can leave that change
> there and revert only the meaningful part?

Yes.