From: Nikita Pettik <korablev@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 2/4] tuple: pass tuple format to xrow_update_*_store()
Date: Mon, 10 Feb 2020 19:51:42 +0300 [thread overview]
Message-ID: <20200210165142.GA99436@tarantool.org> (raw)
In-Reply-To: <79fa040657f8826cb03bcd4cfd1d572e8cda69ce.1580856721.git.v.shpilevoy@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;
}
next prev parent reply other threads:[~2020-02-10 16:51 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-04 22:53 [Tarantool-patches] [PATCH v2 0/4] Don't truncate float in update Vladislav Shpilevoy
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 1/4] tuple: don't truncate float in :update() Vladislav Shpilevoy
2020-02-10 14:41 ` Nikita Pettik
2020-02-10 21:18 ` Vladislav Shpilevoy
2020-02-19 21:36 ` Nikita Pettik
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 2/4] tuple: pass tuple format to xrow_update_*_store() Vladislav Shpilevoy
2020-02-10 16:51 ` Nikita Pettik [this message]
2020-02-10 21:18 ` Vladislav Shpilevoy
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 3/4] tuple: allow xrow_update sizeof reserve more memory Vladislav Shpilevoy
2020-02-10 16:05 ` Nikita Pettik
2020-02-04 22:53 ` [Tarantool-patches] [PATCH v2 4/4] tuple: use field type in update of a float/double Vladislav Shpilevoy
2020-02-10 16:16 ` Nikita Pettik
2020-02-10 21:18 ` Vladislav Shpilevoy
2020-02-05 11:09 ` [Tarantool-patches] [PATCH v2 0/4] Don't truncate float in update Konstantin Osipov
2020-02-05 22:11 ` Vladislav Shpilevoy
2020-02-20 6:15 ` Kirill Yukhin
2020-02-20 13:27 ` Nikita Pettik
2020-02-20 20:30 ` Vladislav Shpilevoy
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200210165142.GA99436@tarantool.org \
--to=korablev@tarantool.org \
--cc=tarantool-patches@dev.tarantool.org \
--cc=v.shpilevoy@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v2 2/4] tuple: pass tuple format to xrow_update_*_store()' \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox