From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 DF2F843D67A for ; Sat, 19 Oct 2019 18:07:38 +0300 (MSK) References: <20191005133140.GF3913@atlas> <20191005134949.GH3913@atlas> From: Vladislav Shpilevoy Message-ID: Date: Sat, 19 Oct 2019 17:12:49 +0200 MIME-Version: 1.0 In-Reply-To: <20191005134949.GH3913@atlas> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [tarantool-patches] Re: [PATCH 1/3] tuple: rework updates to improve code extendibility List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Konstantin Osipov Cc: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org Thanks for the review! On 05/10/2019 15:49, Konstantin Osipov wrote: > I don't know why my mutt folder hook are messed up, will > take some time to figure out. > > Resending from the right email. > > * Vladislav Shpilevoy [19/09/28 09:10]: >> Before the patch update was implemented as a set of operations >> applicable for arrays only. It was ok until field names and JSON >> paths appearance, because tuple is array on the top level. >> >> But now there are four reasons to allow more complex updates of >> tuple field internals via JSON paths: >> * tuple field access by JSON path is allowed so for consistency >> JSON paths should be allowed in updates as well; >> * JSON indexes are supported. JSON update should be able to >> change an indexed field without rewriting half of a tuple; >> * Tarantool is going to support documents in storage so JSON >> path updates is one more step forward; >> * JSON updates are going to be faster than get + in-memory >> Lua/connector update + replace (or update of a whole tuple >> field). >> >> The patch prepares current update code to be extended by updates >> of map, so named isolated 'bar' updates, and multilevel updates. >> >> The concept is to build a tree of update objects. Each updates a >> scalar terminal value (leaf of that tree), or a complex object >> like array or map. In the latter case these objects contain >= 2 >> children updates. >> >> This organization allows to split update implementation into >> several independent files-modules for each update type. Next >> commits will introduce them one by one. > > Now that you provide some rationale for the rename, does the > naming scheme you used reflects this rationale? > > 1) update - a very general name, there is sql update, tuple > update, lsm tree update, configuration update >From my point of view 'update' is a very short and descriptive name. Anyone who would somehow fail to understand it still may open update/update.h and see 'tuple_update_execute'. That makes it already obvious what is it for. And since all the update-related files are in own directory, it becomes obvious that all the other files are also about tuple updates. > 2) update_array - impossible to say by looking at the code whether > it is a function which udpates an array or an array of updates. In the code I said explicitly what it is: /** * Field is an updated array. Check the rope for updates * of individual fields. */ UPDATE_ARRAY, Even twice: /** * This update changes an array. Children fields * are stored in rope nodes. */ struct { struct rope *rope; } array; > > Generally, the key reason to exist for the update interface > is being able to represent partial tuple changes in the binary > log - i.e. update micro-language exists only because we want > to send compact changes over the network to replicas. There is > no other reason *for the microlanguage* to exist. How is it related to my patchset? > > So maybe it should be called xrow_update, to avoid ambiguity with > other types of updates? > It has nothing to do with xrow. There is no a header, metadata, persistency, or networking. Additionally, this code uses names, tuple formats, which do not exist in xrow.