From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 8DEE225A98 for ; Sat, 31 Aug 2019 17:32:45 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id jNtF8Dw2RB1O for ; Sat, 31 Aug 2019 17:32:45 -0400 (EDT) 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 turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4ABF925A19 for ; Sat, 31 Aug 2019 17:32:45 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH v2 6/8] tuple: make update operation tokens consumable Date: Sat, 31 Aug 2019 23:35:56 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: kostja@tarantool.org There is a case: [1][2][3][4] = 100. It is not a problem when it is a single operation, not intersecting with anything. It is a bar then, and works ok. But the next patch allows intersections, and the path [1][2][3][4] can become a tree of arrays. The root array has its child [1] array. The child has its own child [2] array, and so on. They each would call do_op_set(child). But all array update routines expect, that field_no is already extracted from the patch, and will look at update_op.field_no to find a target field. It means, that perhaps each next level of the update [1][2][3][4] should prepare field_no for the next child? In such a case they would need to check type of the child if it is an array, which would complicate code even more: each array update operation would check child types; each do_op_##optype() would check field type. Even if it is not enough, there are maps - they double the complexity. So this patch goes another way - lets each next level of update will check if its field_no/map_key is already prepared by the caller. And if not, extract a next token from the operation path. Part of #1261 --- src/box/update/update_array.c | 42 +++++++++++++++++++++++++++++++++-- src/box/update/update_field.c | 17 ++++++++++++++ src/box/update/update_field.h | 25 +++++++++++++++++++-- 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/src/box/update/update_array.c b/src/box/update/update_array.c index fe50a605a..262eceafb 100644 --- a/src/box/update/update_array.c +++ b/src/box/update/update_array.c @@ -32,6 +32,26 @@ #include "msgpuck.h" #include "fiber.h" +/** + * Make sure @a op contains a valid field number. It can be not + * so, if the array's parent didn't propagate operation's lexer. + * In fact, the parent fills fieldno only in some rare cases like + * branching. Generally, an array should care about fieldno by + * itself. + */ +static inline int +update_op_prepare_num_token(struct update_op *op) +{ + if (op->token_type == JSON_TOKEN_END && + update_op_consume_token(op) != 0) + return -1; + if (op->token_type != JSON_TOKEN_NUM) { + return update_err(op, "can't update an array by not a "\ + "number index"); + } + return 0; +} + /** * Make field number positive and check for the field existence. * @param op Update operation. @@ -43,6 +63,7 @@ static inline int update_op_adjust_field_no(struct update_op *op, int32_t field_count) { + assert(op->token_type == JSON_TOKEN_NUM); if (op->field_no >= 0) { if (op->field_no < field_count) return 0; @@ -217,10 +238,14 @@ do_op_array_insert(struct update_op *op, struct update_field *field) assert(field->type == UPDATE_ARRAY); struct rope *rope = field->array.rope; struct update_array_item *item; + if (update_op_prepare_num_token(op) != 0) + return -1; + if (! update_op_is_term(op)) { item = update_array_extract_item(field, op); if (item == NULL) return -1; + op->token_type = JSON_TOKEN_END; return do_op_insert(op, &item->field); } @@ -241,6 +266,9 @@ do_op_array_set(struct update_op *op, struct update_field *field) { assert(field->type == UPDATE_ARRAY); struct rope *rope = field->array.rope; + if (update_op_prepare_num_token(op) != 0) + return -1; + /* Interpret '=' for n + 1 field as insert. */ if (op->field_no == (int32_t) rope_size(rope)) return do_op_array_insert(op, field); @@ -249,8 +277,10 @@ do_op_array_set(struct update_op *op, struct update_field *field) update_array_extract_item(field, op); if (item == NULL) return -1; - if (! update_op_is_term(op)) + if (! update_op_is_term(op)) { + op->token_type = JSON_TOKEN_END; return do_op_set(op, &item->field); + } op->new_field_len = op->arg.set.length; /* Ignore the previous op, if any. */ item->field.type = UPDATE_SCALAR; @@ -262,11 +292,15 @@ int do_op_array_delete(struct update_op *op, struct update_field *field) { assert(field->type == UPDATE_ARRAY); + if (update_op_prepare_num_token(op) != 0) + return -1; + if (! update_op_is_term(op)) { struct update_array_item *item = update_array_extract_item(field, op); if (item == NULL) return -1; + op->token_type = JSON_TOKEN_END; return do_op_delete(op, &item->field); } struct rope *rope = field->array.rope; @@ -285,12 +319,16 @@ do_op_array_delete(struct update_op *op, struct update_field *field) int \ do_op_array_##op_type(struct update_op *op, struct update_field *field) \ { \ + if (update_op_prepare_num_token(op) != 0) \ + return -1; \ struct update_array_item *item = \ update_array_extract_item(field, op); \ if (item == NULL) \ return -1; \ - if (! update_op_is_term(op)) \ + if (! update_op_is_term(op)) { \ + op->token_type = JSON_TOKEN_END; \ return do_op_##op_type(op, &item->field); \ + } \ if (item->field.type != UPDATE_NOP) \ return update_err_double(op); \ if (update_op_do_##op_type(op, item->field.data) != 0) \ diff --git a/src/box/update/update_field.c b/src/box/update/update_field.c index 6baad02dd..db02959ff 100644 --- a/src/box/update/update_field.c +++ b/src/box/update/update_field.c @@ -594,6 +594,22 @@ update_op_by(char opcode) } } +int +update_op_consume_token(struct update_op *op) +{ + struct json_token token; + int rc = json_lexer_next_token(&op->lexer, &token); + if (rc != 0) + return update_err_bad_json(op, rc); + if (token.type == JSON_TOKEN_END) + return update_err_no_such_field(op); + op->token_type = token.type; + op->key = token.str; + op->key_len = token.len; + op->field_no = token.num; + return 0; +} + int update_op_decode(struct update_op *op, int index_base, struct tuple_dictionary *dict, const char **expr) @@ -622,6 +638,7 @@ update_op_decode(struct update_op *op, int index_base, diag_set(ClientError, ER_UNKNOWN_UPDATE_OP); return -1; } + op->token_type = JSON_TOKEN_NUM; int32_t field_no; switch(mp_typeof(**expr)) { case MP_INT: diff --git a/src/box/update/update_field.h b/src/box/update/update_field.h index d4499eff8..ee42ba5ae 100644 --- a/src/box/update/update_field.h +++ b/src/box/update/update_field.h @@ -175,8 +175,18 @@ struct update_op { const struct update_op_meta *meta; /** Operation arguments. */ union update_op_arg arg; - /** First level field no. */ - int32_t field_no; + /** + * Current level token. END means that it is invalid and + * a next token should be extracted from the lexer. + */ + enum json_token_type token_type; + union { + struct { + const char *key; + uint32_t key_len; + }; + int32_t field_no; + }; /** Size of a new field after it is updated. */ uint32_t new_field_len; /** Opcode symbol: = + - / ... */ @@ -189,6 +199,17 @@ struct update_op { struct json_lexer lexer; }; +/** + * Extract a next token from the operation path lexer. The result + * is used to decide to which child of a current map/array the + * operation should be forwarded. It is not just a synonym to + * json_lexer_next_token, because fills some fields of @a op, + * and should be used only to chose a next child inside a current + * map/array. + */ +int +update_op_consume_token(struct update_op *op); + /** * Decode an update operation from MessagePack. * @param[out] op Update operation. -- 2.20.1 (Apple Git-117)