From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp50.i.mail.ru (smtp50.i.mail.ru [94.100.177.110]) (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 53350469719 for ; Thu, 15 Oct 2020 03:36:16 +0300 (MSK) Date: Thu, 15 Oct 2020 00:36:15 +0000 From: Nikita Pettik Message-ID: <20201015003615.GA25221@tarantool.org> References: <743a25a986ebbe4388d8f6ffc7d1502f20a5efb9.1601729099.git.korablev@tarantool.org> <343c3a98-2eec-1d3b-797b-77ed76c304bb@tarantool.org> <20201014001531.GA20094@tarantool.org> <20201014104222.GA23071@tarantool.org> <0724fa81-2b0a-4aec-48d6-dc6059e7386f@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <0724fa81-2b0a-4aec-48d6-dc6059e7386f@tarantool.org> 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: Aleksandr Lyapunov Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org On 14 Oct 14:47, Aleksandr Lyapunov wrote: > Great, LGTM! Pushed to master, changelog is updated, branch is dropped. > 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)); > >