From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp34.i.mail.ru (smtp34.i.mail.ru [94.100.177.94])
 (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 6A505445320
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 14 Jul 2020 01:15:38 +0300 (MSK)
References: <e32f85595b4fce26790dccf5968ed1405e2eda86.1593883084.git.v.shpilevoy@tarantool.org>
 <20200712231153.GC13229@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <729aca75-411e-51b4-b6a8-d1ebd0fc6db0@tarantool.org>
Date: Tue, 14 Jul 2020 00:15:35 +0200
MIME-Version: 1.0
In-Reply-To: <20200712231153.GC13229@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH 1/1] tuple: turn invalid bar update
	into nop
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: Nikita Pettik <korablev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Hi! Thanks for the review!

Pushed to master. See 2 answers below.

>> diff --git a/src/box/xrow_update_bar.c b/src/box/xrow_update_bar.c
>> index 0033f0044..1d20d6c74 100644
>> --- a/src/box/xrow_update_bar.c
>> +++ b/src/box/xrow_update_bar.c
>> @@ -31,6 +31,22 @@
>>  #include "xrow_update_field.h"
>>  #include "tuple.h"
>>  
>> +/**
>> + * 'Commit' bar creation only when it is fully initialized and
>> + * valid. Because if all this is happening inside an upsert()
>> + * operation, it won't stop the whole xrow upsert. This field will
>> + * still be saved in the result tuple. But in case of an error
>> + * this operation should be skipped. So this is kept 'nop' when
>> + * error happens.
>> + */
>> +static inline int
>> +xrow_update_bar_commit(struct xrow_update_field *field)
> 
> Mb _done/_finish are better names? Up to you. 

I don't mind, changed to 'finish'.

>> +{
>> +	assert(field->type == XUPDATE_NOP);
>> +	field->type = XUPDATE_BAR;
>> +	return 0;
>> +}
>> +
>>  /**
>>   * Locate a field to update by @a op's JSON path and initialize
>>   * @a field as a bar update.
>> diff --git a/test/box/update.test.lua b/test/box/update.test.lua
>> index 1538cae9c..27cf55467 100644
>> --- a/test/box/update.test.lua
>> +++ b/test/box/update.test.lua
>> @@ -343,20 +343,52 @@ t2:update({{'!', 'g[6][1]', 50}})
>>  t4_array:update({{'!', '[4][1]', 100}})
>>  t4_map:update({{'!', '[4].a', 100}})
>>  -- Test errors.
>> -t:update({{'!', 'a', 100}}) -- No such field.
>> -t:update({{'!', 'f.a', 300}}) -- Key already exists.
>> -t:update({{'!', 'f.c.f[0]', 3.5}}) -- No such index, too small.
>> -t:update({{'!', 'f.c.f[100]', 100}}) -- No such index, too big.
>> -t:update({{'!', 'g[4][100]', 700}}) -- Insert index into map.
>> -t:update({{'!', 'g[1][1]', 300}})
>> -t:update({{'!', 'f.g.a', 700}}) -- Insert key into array.
>> -t:update({{'!', 'f.g[1].a', 700}})
>> -t:update({{'!', 'f[*].k', 20}}) -- 'Any' is not considered valid JSON.
> 
> New tests are cool, but mb it is worth reducing diff by placing
> new tests right after old ones? Don't insist tho.

Yeah, this is what I did, but still got all the diff. Because I
saved the operations array into a new variable 'ops', to avoid
the operations duplication. This changed the old lines as well.