Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: kostja@tarantool.org
Subject: [tarantool-patches] [PATCH v2 6/8] tuple: make update operation tokens consumable
Date: Sat, 31 Aug 2019 23:35:56 +0200	[thread overview]
Message-ID: <bdeaada09bd27620615ebdfc019cb4666d3c2606.1567287197.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1567287197.git.v.shpilevoy@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)

  parent reply	other threads:[~2019-08-31 21:32 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-31 21:35 [tarantool-patches] [PATCH v2 0/8] JSON updates Vladislav Shpilevoy
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 1/8] tuple: expose JSON go_to_key and go_to_index functions Vladislav Shpilevoy
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 2/8] tuple: rework updates to improve code extendibility Vladislav Shpilevoy
     [not found]   ` <20190903192059.GE15611@atlas>
     [not found]     ` <6ee759cf-a975-e6a9-6f52-f855958ffe06@tarantool.org>
     [not found]       ` <20191005132055.GD3913@atlas>
     [not found]         ` <20191005135037.GJ3913@atlas>
2019-10-19 15:11           ` [Tarantool-patches] [tarantool-patches] " Vladislav Shpilevoy
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 3/8] json: lexer_eof and token_cmp helper functions Vladislav Shpilevoy
     [not found]   ` <20190903192433.GF15611@atlas>
     [not found]     ` <f5612e04-dc56-f4bd-1298-c5841ac909f5@tarantool.org>
     [not found]       ` <20191005132231.GE3913@atlas>
     [not found]         ` <20191005135014.GI3913@atlas>
2019-10-19 15:08           ` [Tarantool-patches] [tarantool-patches] " Vladislav Shpilevoy
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 4/8] tuple: account the whole array in field.data and size Vladislav Shpilevoy
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 5/8] tuple: enable JSON bar updates Vladislav Shpilevoy
2019-08-31 21:35 ` Vladislav Shpilevoy [this message]
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 7/8] tuple: JSON updates support intersection by arrays Vladislav Shpilevoy
2019-08-31 21:35 ` [tarantool-patches] [PATCH v2 8/8] tuple: JSON updates support intersection by maps 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=bdeaada09bd27620615ebdfc019cb4666d3c2606.1567287197.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v2 6/8] tuple: make update operation tokens consumable' \
    /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