From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp18.mail.ru (smtp18.mail.ru [94.100.176.155]) (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 DCBE2469719 for ; Wed, 14 Oct 2020 13:42:23 +0300 (MSK) Date: Wed, 14 Oct 2020 10:42:22 +0000 From: Nikita Pettik Message-ID: <20201014104222.GA23071@tarantool.org> References: <743a25a986ebbe4388d8f6ffc7d1502f20a5efb9.1601729099.git.korablev@tarantool.org> <343c3a98-2eec-1d3b-797b-77ed76c304bb@tarantool.org> <20201014001531.GA20094@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: 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 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));