From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 A8AA746970E for ; Mon, 10 Feb 2020 19:51:43 +0300 (MSK) Date: Mon, 10 Feb 2020 19:51:42 +0300 From: Nikita Pettik Message-ID: <20200210165142.GA99436@tarantool.org> References: <79fa040657f8826cb03bcd4cfd1d572e8cda69ce.1580856721.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <79fa040657f8826cb03bcd4cfd1d572e8cda69ce.1580856721.git.v.shpilevoy@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 2/4] tuple: pass tuple format to xrow_update_*_store() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org On 04 Feb 23:53, Vladislav Shpilevoy wrote: > diff --git a/src/box/xrow_update_array.c b/src/box/xrow_update_array.c > index b97c23d77..717466be7 100644 > --- a/src/box/xrow_update_array.c > +++ b/src/box/xrow_update_array.c > @@ -31,6 +31,7 @@ > #include "xrow_update_field.h" > #include "msgpuck.h" > #include "fiber.h" > +#include "tuple_format.h" > > /** > * Make sure @a op contains a valid field number to where the > @@ -259,26 +260,49 @@ xrow_update_array_sizeof(struct xrow_update_field *field) > } > > uint32_t > -xrow_update_array_store(struct xrow_update_field *field, char *out, > - char *out_end) > +xrow_update_array_store(struct xrow_update_field *field, > + struct json_tree *format_tree, > + struct json_token *this_node, char *out, char *out_end) > { > assert(field->type == XUPDATE_ARRAY); > char *out_begin = out; > out = mp_encode_array(out, xrow_update_rope_size(field->array.rope)); > - uint32_t total_field_count = 0; > struct xrow_update_rope_iter it; > xrow_update_rope_iter_create(&it, field->array.rope); > struct xrow_update_rope_node *node = xrow_update_rope_iter_start(&it); > - for (; node != NULL; node = xrow_update_rope_iter_next(&it)) { > - struct xrow_update_array_item *item = > - xrow_update_rope_leaf_data(node); > - uint32_t field_count = xrow_update_rope_leaf_size(node); > - out += xrow_update_field_store(&item->field, out, out_end); > - assert(item->tail_size == 0 || field_count > 1); > - memcpy(out, item->field.data + item->field.size, > - item->tail_size); > - out += item->tail_size; > - total_field_count += field_count; > + uint32_t total_field_count = 0; > + if (this_node == NULL) { > + for (; node != NULL; node = xrow_update_rope_iter_next(&it)) { > + struct xrow_update_array_item *item = > + xrow_update_rope_leaf_data(node); > + uint32_t field_count = xrow_update_rope_leaf_size(node); > + out += xrow_update_field_store(&item->field, NULL, NULL, > + out, out_end); > + assert(item->tail_size == 0 || field_count > 1); > + memcpy(out, item->field.data + item->field.size, > + item->tail_size); > + out += item->tail_size; > + total_field_count += field_count; > + } > + } else { > + struct json_token token; > + token.type = JSON_TOKEN_NUM; > + token.num = 0; > + struct json_token *next_node; > + for (; node != NULL; node = xrow_update_rope_iter_next(&it)) { > + struct xrow_update_array_item *item = > + xrow_update_rope_leaf_data(node); > + next_node = json_tree_lookup(format_tree, this_node, &token); > + uint32_t field_count = xrow_update_rope_leaf_size(node); > + out += xrow_update_field_store(&item->field, format_tree, > + next_node, out, out_end); > + assert(item->tail_size == 0 || field_count > 1); > + memcpy(out, item->field.data + item->field.size, > + item->tail_size); > + out += item->tail_size; > + token.num += field_count; > + total_field_count += field_count; > + } IMHO this code duplication looks a bit rough. What about next refactoring: diff --git a/src/box/xrow_update_array.c b/src/box/xrow_update_array.c index 717466be7..c8ca886ae 100644 --- a/src/box/xrow_update_array.c +++ b/src/box/xrow_update_array.c @@ -271,38 +271,26 @@ xrow_update_array_store(struct xrow_update_field *field, xrow_update_rope_iter_create(&it, field->array.rope); struct xrow_update_rope_node *node = xrow_update_rope_iter_start(&it); uint32_t total_field_count = 0; - if (this_node == NULL) { - for (; node != NULL; node = xrow_update_rope_iter_next(&it)) { - struct xrow_update_array_item *item = - xrow_update_rope_leaf_data(node); - uint32_t field_count = xrow_update_rope_leaf_size(node); - out += xrow_update_field_store(&item->field, NULL, NULL, - out, out_end); - assert(item->tail_size == 0 || field_count > 1); - memcpy(out, item->field.data + item->field.size, - item->tail_size); - out += item->tail_size; - total_field_count += field_count; - } - } else { - struct json_token token; - token.type = JSON_TOKEN_NUM; - token.num = 0; - struct json_token *next_node; - for (; node != NULL; node = xrow_update_rope_iter_next(&it)) { - struct xrow_update_array_item *item = - xrow_update_rope_leaf_data(node); - next_node = json_tree_lookup(format_tree, this_node, &token); - uint32_t field_count = xrow_update_rope_leaf_size(node); - out += xrow_update_field_store(&item->field, format_tree, - next_node, out, out_end); - assert(item->tail_size == 0 || field_count > 1); - memcpy(out, item->field.data + item->field.size, - item->tail_size); - out += item->tail_size; + struct json_token *next_node = NULL; + struct json_token token; + token.type = JSON_TOKEN_NUM; + token.num = 0; + for (; node != NULL; node = xrow_update_rope_iter_next(&it)) { + struct xrow_update_array_item *item = + xrow_update_rope_leaf_data(node); + uint32_t field_count = xrow_update_rope_leaf_size(node); + if (this_node != NULL) { + next_node = json_tree_lookup(format_tree, this_node, + &token); token.num += field_count; - total_field_count += field_count; } + out += xrow_update_field_store(&item->field, format_tree, + next_node, out, out_end); + assert(item->tail_size == 0 || field_count > 1); + memcpy(out, item->field.data + item->field.size, + item->tail_size); + out += item->tail_size; + total_field_count += field_count; }