<HTML><BODY>Thanks for the patch!<br><br>The changes looks good to me. Frankly, I didn't perform a byte-exact <br>comparison of factorization between files - testing is green for the branch.<br><br>Regards,<br>Sergos<br><br><br><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">
Thursday, November 7, 2019 1:53 PM +03:00 from Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:<br>
<br>
<div id="">
<div class="js-helper js-readmsg-msg">
<style type="text/css"></style>
<div>
<div id="style_15731239800503800174_BODY">I gave up trying to split these changes, because they<br>
are either too minor, or are not separable at all. Kostja<br>
proposed to describe them in a changelist, and here is a<br>
new commit message:<br>
<br>
====================================================================<br>
<br>
tuple: rework updates to improve code extendibility<br>
<br>
Before the patch update was implemented as a set of operations<br>
applicable for arrays only. It was ok until field names and JSON<br>
paths appearance, because tuple is an array on the top level.<br>
<br>
But now there are four reasons to allow more complex updates of<br>
tuple field internals by JSON paths:<br>
<br>
- tuple field access by JSON path is allowed so for consistency<br>
JSON paths should be allowed in updates as well;<br>
<br>
- JSON indexes are supported. JSON update should be able to<br>
change an indexed field without rewriting half of a tuple, and<br>
its full replacement;<br>
<br>
- Tarantool is going to support documents in storage so JSON<br>
path updates is one more step forward;<br>
<br>
- JSON updates are going to be faster and more compact in WAL<br>
than get + in-memory Lua/connector update + replace (or update<br>
of a whole tuple field).<br>
<br>
The patch reworks the current update code in such a way, that now<br>
update is not just an array of operations, applied to tuple's top<br>
level fields. Now it is a tree, just like tuples are.<br>
<br>
The concept is to build a tree of xrow_update_field objects. Each<br>
updates a part of a tuple. Leafs in the tree contain update<br>
operations, specified by a user, as xrow_update_op objects.<br>
<br>
To make the code support and understanding simpler, the patch<br>
splits update implementation into several independent<br>
files-modules for each type of an updated field. One file<br>
describes how to update an array field, another file - how to<br>
update a map field, etc. This commit introduces only array. Just<br>
because it was already supported before the patch. Next commits<br>
will introduce more types one by one.<br>
<br>
Besides, the patch makes some minor changes, not separable from<br>
this commit:<br>
<br>
- The big comment about xrow updates in xrow_update.c is<br>
updated. Now it describes the tree-idea presented above;<br>
<br>
- Comments were properly aligned by 66 symbols in all the moved<br>
or changed code. Not affected code is kept as is so as not to<br>
increase the diff even more;<br>
<br>
- Added missing comments to moved or changed structures and<br>
their attributes such as struct xrow_update,<br>
struct xrow_update_op_meta, struct xrow_update_op.<br>
<br>
- Struct xrow_update_field was significantly reworked. Now it is<br>
not just a couple of pointers at tuple's top level array. From<br>
now it stores type of the updated field, range of its source<br>
data in the original tuple, and a subtree of other update<br>
fields applied to the original data.<br>
<br>
- Added missing comments to some functions which I moved and<br>
decided worth commenting alongside, such as<br>
xrow_update_op_adjust_field_no(), xrow_update_alloc().<br>
<br>
- Functions xrow_update_op_do_f, xrow_update_op_read_arg_f,<br>
xrow_update_op_store_f are separated from struct xrow_update,<br>
so as they could be called on any updated field in the tree.<br>
From this moment they are methods of struct xrow_update_op.<br>
They take an op as a first argument (like 'this' in C++), and<br>
are applied to a given struct xrow_update_field.<br>
<br>
Another notable, but not separable, change is a new naming schema<br>
for the methods of struct xrow_update_field and struct<br>
xrow_update_op. This is motivated by the fact that struct<br>
xrow_update_field now has a type, and might be not a terminal.<br>
<br>
There are now two groups of functions. Generic functions working<br>
with struct xrow_update_field of any type:<br>
<br>
xrow_update_field_sizeof<br>
xrow_update_field_store<br>
xrow_update_op_do_field_<operation><br>
<br>
And typed functions:<br>
<br>
xrow_update_<type>_sizeof<br>
xrow_update_<type>_store<br>
xrow_update_op_do_<type>_<operation><br>
<br>
Where<br>
operation = insert/delete/set/arith ...<br>
type = array/map/bar/scalar ...<br>
<br>
Common functions are used when type of a field to update is not<br>
known in advance. For example, when an operation is applied to one<br>
of fields of an array, it is not known what a type this field has:<br>
another array, scalar, not changed field, map, etc. Common<br>
functions do nothing more than just a switch by field type to<br>
choose a more specific function.<br>
<br>
Typed functions work with a specific type. They may change the<br>
given field (add a new array element, replace it with a new value,<br>
...), or may forward an operation deeper in case they see that its<br>
JSON path is not fully passed yet.<br>
<br>
Part of #1261<br>
</div>
</div>
</div>
</div>
</blockquote>
<br></BODY></HTML>