From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 A2BB3469719 for ; Wed, 14 Oct 2020 14:47:21 +0300 (MSK) References: <743a25a986ebbe4388d8f6ffc7d1502f20a5efb9.1601729099.git.korablev@tarantool.org> <343c3a98-2eec-1d3b-797b-77ed76c304bb@tarantool.org> <20201014001531.GA20094@tarantool.org> <20201014104222.GA23071@tarantool.org> From: Aleksandr Lyapunov Message-ID: <0724fa81-2b0a-4aec-48d6-dc6059e7386f@tarantool.org> Date: Wed, 14 Oct 2020 14:47:20 +0300 MIME-Version: 1.0 In-Reply-To: <20201014104222.GA23071@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Content-Language: en-US Subject: Re: [Tarantool-patches] [PATCH v3 1/2] vinyl: rework upsert operation List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Nikita Pettik Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org Great, LGTM! On 10/14/20 1:42 PM, Nikita Pettik wrote: > On 14 Oct 11:58, Aleksandr Lyapunov wrote: >>> if (stmt_is_void) { >>> ups_cnt--; >>> mp_next(&ups_ops); >>> + stmt = vy_stmt_new_replace(tuple_format(upsert), result_mp, >>> + result_mp_end); >> Why can't you just use `upsert` tuple itself instead of created tuple by its >> format and data? >> > For sure we can. Sorry it was late night so I didn't pay attention to this fact. > Diff: > > diff --git a/src/box/vy_upsert.c b/src/box/vy_upsert.c > index 0b4baf6ff..fdae931f6 100644 > --- a/src/box/vy_upsert.c > +++ b/src/box/vy_upsert.c > @@ -102,15 +102,13 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt, > const char *ups_ops = new_ops; > /* > * In case upsert folds into insert, we must skip first > - * update operations. Moreover, we should create (in case of delete > - * statement - re-create since delete contains only key) tuple with > - * format to use it for PK modification check. > + * update operations. Moreover, we should use upsert's tuple > + * to provide PK modification check. > */ > if (stmt_is_void) { > ups_cnt--; > mp_next(&ups_ops); > - stmt = vy_stmt_new_replace(tuple_format(upsert), result_mp, > - result_mp_end); > + stmt = upsert; > } > for (uint32_t i = 0; i < ups_cnt; ++i) { > assert(mp_typeof(*ups_ops) == MP_ARRAY); > @@ -126,8 +124,6 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt, > assert(e != NULL); > /* Bail out immediately in case of OOM. */ > if (e->type != &type_ClientError) { > - if (stmt_is_void) > - tuple_unref(stmt); > region_truncate(region, region_svp); > return NULL; > } > @@ -169,8 +165,6 @@ vy_apply_upsert_on_terminal_stmt(struct tuple *upsert, struct tuple *stmt, > struct tuple *new_terminal_stmt = vy_stmt_new_replace(format, result_mp, > result_mp_end); > region_truncate(region, region_svp); > - if (stmt_is_void) > - tuple_unref(stmt); > if (new_terminal_stmt == NULL) > return NULL; > vy_stmt_set_lsn(new_terminal_stmt, vy_stmt_lsn(upsert)); > >