[PATCH v1 0/5] box: JSON preparatory patchset

[PATCH v1 0/5] box: JSON preparatory patchset

[Kirill Shcherbatov]

Preparatory patch set for JSON indexes:
- Implemented tuple_validate with tuple_init_field_map.
- Refactored field type/nullability check routines.
- Implemented a new json_token_path_snprint routine able to print
  JSON path to field by field specified working like cannonical
  snprintf routine
- Reworked box error messages to use the json_token_path_snprint
  helper to report field names in error in error messages.
- Reworked tuple_init_field_map with required fields bitmap -
  a scallable approach able to work with JSON multilevel fields
  tree.

Branch: http://github.com/tarantool/tarantool/tree/kshch/gh-1012-json-indexes-preparational
Issue: https://github.com/tarantool/tarantool/issues/1012

Kirill Shcherbatov (5):
  box: refactor tuple_validate_raw
  box: refactor field type and nullability checks
  lib: introduce json_token_path_snprint
  box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}
  box: refactor tuple_init_field_map to use bitmap

 src/box/errcode.h                   |   6 +-
 src/box/index.cc                    |  22 +--
 src/box/key_def.c                   |  12 +-
 src/box/key_def.h                   |  30 ++--
 src/box/memtx_rtree.c               |   2 +-
 src/box/tuple.c                     |  35 ++---
 src/box/tuple.h                     |  15 +-
 src/box/tuple_format.c              | 213 +++++++++++++++++++++-------
 src/box/tuple_format.h              |  23 +++
 src/lib/json/json.c                 |  46 ++++++
 src/lib/json/json.h                 |  13 ++
 test/box/alter.result               |   6 +-
 test/box/alter_limits.result        |  15 +-
 test/box/blackhole.result           |   6 +-
 test/box/ddl.result                 |  32 +++--
 test/box/hash.result                |  15 +-
 test/box/misc.result                |   2 +-
 test/box/rtree_misc.result          |  18 ++-
 test/box/sequence.result            |   3 +-
 test/box/sql.result                 |  12 +-
 test/box/tree_pk.result             |   9 +-
 test/box/tree_pk_multipart.result   |   8 +-
 test/engine/ddl.result              | 116 ++++++++-------
 test/engine/indices_any_type.result |  12 +-
 test/engine/null.result             |  85 ++++++-----
 test/engine/tree.result             |  12 +-
 test/engine/tree_variants.result    |  12 +-
 test/engine/update.result           |   6 +-
 test/engine/upsert.result           |   9 +-
 test/replication/misc.result        |   3 +-
 test/unit/json.c                    |  20 ++-
 test/unit/json.result               | 113 +++++++++------
 test/vinyl/constraint.result        |  30 ++--
 test/vinyl/errinj.result            |  15 +-
 test/vinyl/gh.result                |   3 +-
 test/vinyl/savepoint.result         |  11 +-
 36 files changed, 640 insertions(+), 350 deletions(-)

-- 
2.19.2

[PATCH v1 1/5] box: refactor tuple_validate_raw

[Kirill Shcherbatov]

Since the tuple_validate_raw and tuple_init_field_map functions
make the same data checks, we implemented tuple_validate_raw via
tuple_init_field_map called with region memory chunk is passed
as field map. This is required because in subsequent patches
the tuple_init_field_map routine logic will be complicated,
and we want to avoid writing the same checks twice.
---
 src/box/tuple.c | 35 +++++++++--------------------------
 1 file changed, 9 insertions(+), 26 deletions(-)

diff --git a/src/box/tuple.c b/src/box/tuple.c
index aae1c3cdd..4ad932f07 100644
--- a/src/box/tuple.c
+++ b/src/box/tuple.c
@@ -141,35 +141,18 @@ tuple_validate_raw(struct tuple_format *format, const char *tuple)
 	if (tuple_format_field_count(format) == 0)
 		return 0; /* Nothing to check */
 
-	/* Check to see if the tuple has a sufficient number of fields. */
-	uint32_t field_count = mp_decode_array(&tuple);
-	if (format->exact_field_count > 0 &&
-	    format->exact_field_count != field_count) {
-		diag_set(ClientError, ER_EXACT_FIELD_COUNT,
-			 (unsigned) field_count,
-			 (unsigned) format->exact_field_count);
+	struct region *region = &fiber()->gc;
+	uint32_t used = region_used(region);
+	uint32_t *field_map = region_alloc(region, format->field_map_size);
+	if (field_map == NULL) {
+		diag_set(OutOfMemory, format->field_map_size, "region_alloc",
+			 "field_map");
 		return -1;
 	}
-	if (unlikely(field_count < format->min_field_count)) {
-		diag_set(ClientError, ER_MIN_FIELD_COUNT,
-			 (unsigned) field_count,
-			 (unsigned) format->min_field_count);
+	field_map = (uint32_t *)((char *)field_map + format->field_map_size);
+	if (tuple_init_field_map(format, field_map, tuple, true) != 0)
 		return -1;
-	}
-
-	/* Check field types */
-	struct tuple_field *field = tuple_format_field(format, 0);
-	uint32_t i = 0;
-	uint32_t defined_field_count =
-		MIN(field_count, tuple_format_field_count(format));
-	for (; i < defined_field_count; ++i) {
-		field = tuple_format_field(format, i);
-		if (key_mp_type_validate(field->type, mp_typeof(*tuple),
-					 ER_FIELD_TYPE, i + TUPLE_INDEX_BASE,
-					 tuple_field_is_nullable(field)))
-			return -1;
-		mp_next(&tuple);
-	}
+	region_truncate(region, used);
 	return 0;
 }
 
-- 
2.19.2

[PATCH v1 2/5] box: refactor field type and nullability checks

[Kirill Shcherbatov]

Reworked field types and nullability checks to set error message
in tuple_init_field_map manually. We need to specify full JSON
path to field sting in further patches.
- introduced field_mp_type_is_compatible routine making
  field_type and mp_type compatibility test taking into account
  field nullability
- refactored key_part_validate to pass const char * key argument
  and to reuse field_mp_type_is_compatible code

Needed for #1012
---
 src/box/index.cc       | 22 +++++++---------------
 src/box/key_def.c      | 12 +++++-------
 src/box/key_def.h      | 30 ++++++++++++++++++++----------
 src/box/tuple_format.c | 18 +++++++++++-------
 4 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/src/box/index.cc b/src/box/index.cc
index cf81eca49..1dbf364a8 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -89,9 +89,7 @@ key_validate(const struct index_def *index_def, enum iterator_type type,
 			return -1;
 		}
 		if (part_count == 1) {
-			enum mp_type mp_type = mp_typeof(*key);
-			if (key_mp_type_validate(FIELD_TYPE_ARRAY, mp_type,
-						 ER_KEY_PART_TYPE, 0, false))
+			if (key_part_validate(FIELD_TYPE_ARRAY, key, 0, false))
 				return -1;
 			uint32_t array_size = mp_decode_array(&key);
 			if (array_size != d && array_size != d * 2) {
@@ -100,23 +98,17 @@ key_validate(const struct index_def *index_def, enum iterator_type type,
 				return -1;
 			}
 			for (uint32_t part = 0; part < array_size; part++) {
-				enum mp_type mp_type = mp_typeof(*key);
-				mp_next(&key);
-				if (key_mp_type_validate(FIELD_TYPE_NUMBER,
-							 mp_type,
-							 ER_KEY_PART_TYPE, 0,
-							 false))
+				if (key_part_validate(FIELD_TYPE_NUMBER, key,
+						      0, false))
 					return -1;
+				mp_next(&key);
 			}
 		} else {
 			for (uint32_t part = 0; part < part_count; part++) {
-				enum mp_type mp_type = mp_typeof(*key);
-				mp_next(&key);
-				if (key_mp_type_validate(FIELD_TYPE_NUMBER,
-							 mp_type,
-							 ER_KEY_PART_TYPE,
-							 part, false))
+				if (key_part_validate(FIELD_TYPE_NUMBER, key,
+						      part, false))
 					return -1;
+				mp_next(&key);
 			}
 		}
 	} else {
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 2119ca389..4cc06de47 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -88,7 +88,7 @@ const char *mp_type_strs[] = {
 	/* .MP_EXT    = */ "extension",
 };
 
-const uint32_t key_mp_type[] = {
+const uint32_t field_mp_type[] = {
 	/* [FIELD_TYPE_ANY]      =  */ UINT32_MAX,
 	/* [FIELD_TYPE_UNSIGNED] =  */ 1U << MP_UINT,
 	/* [FIELD_TYPE_STRING]   =  */ 1U << MP_STR,
@@ -608,14 +608,12 @@ key_validate_parts(const struct key_def *key_def, const char *key,
 		   uint32_t part_count, bool allow_nullable)
 {
 	for (uint32_t i = 0; i < part_count; i++) {
-		enum mp_type mp_type = mp_typeof(*key);
 		const struct key_part *part = &key_def->parts[i];
-		mp_next(&key);
-
-		if (key_mp_type_validate(part->type, mp_type, ER_KEY_PART_TYPE,
-					 i, key_part_is_nullable(part)
-					 && allow_nullable))
+		if (key_part_validate(part->type, key, i,
+				      key_part_is_nullable(part) &&
+				      allow_nullable))
 			return -1;
+		mp_next(&key);
 	}
 	return 0;
 }
diff --git a/src/box/key_def.h b/src/box/key_def.h
index d4da6c5a1..2d9a3a7c2 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -392,27 +392,37 @@ key_def_has_collation(const struct key_def *key_def)
 	return false;
 }
 
-/** A helper table for key_mp_type_validate */
-extern const uint32_t key_mp_type[];
+/** A helper table for field_mp_type_is_compatible */
+extern const uint32_t field_mp_type[];
+
+/** Checks if mp_type (MsgPack) is compatible with field type. */
+static inline bool
+field_mp_type_is_compatible(enum field_type type, enum mp_type mp_type,
+			    bool is_nullable)
+{
+	assert(type < field_type_MAX);
+	assert((size_t)mp_type < CHAR_BIT * sizeof(*field_mp_type));
+	uint32_t mask = field_mp_type[type] | (is_nullable * (1U << MP_NIL));
+	return (mask & (1U << mp_type)) != 0;
+}
 
 /**
  * @brief Checks if \a field_type (MsgPack) is compatible \a type (KeyDef).
  * @param type KeyDef type
- * @param field_type MsgPack type
+ * @param key Pointer to MsgPack field to be tested.
  * @param field_no - a field number (is used to store an error message)
  *
  * @retval 0  mp_type is valid.
  * @retval -1 mp_type is invalid.
  */
 static inline int
-key_mp_type_validate(enum field_type key_type, enum mp_type mp_type,
-		     int err, uint32_t field_no, bool is_nullable)
+key_part_validate(enum field_type key_type, const char *key,
+		  uint32_t field_no, bool is_nullable)
 {
-	assert(key_type < field_type_MAX);
-	assert((size_t) mp_type < CHAR_BIT * sizeof(*key_mp_type));
-	uint32_t mask = key_mp_type[key_type] | (is_nullable * (1U << MP_NIL));
-	if (unlikely((mask & (1U << mp_type)) == 0)) {
-		diag_set(ClientError, err, field_no, field_type_strs[key_type]);
+	if (unlikely(!field_mp_type_is_compatible(key_type, mp_typeof(*key),
+						  is_nullable))) {
+		diag_set(ClientError, ER_KEY_PART_TYPE, field_no,
+			 field_type_strs[key_type]);
 		return -1;
 	}
 	return 0;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 3af39a37e..04343fd53 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -451,13 +451,15 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 	}
 
 	/* first field is simply accessible, so we do not store offset to it */
-	enum mp_type mp_type = mp_typeof(*pos);
 	const struct tuple_field *field =
 		tuple_format_field((struct tuple_format *)format, 0);
 	if (validate &&
-	    key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE,
-				 TUPLE_INDEX_BASE, tuple_field_is_nullable(field)))
+	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
+					 tuple_field_is_nullable(field))) {
+		diag_set(ClientError, ER_FIELD_TYPE, TUPLE_INDEX_BASE,
+			 field_type_strs[field->type]);
 		return -1;
+	}
 	mp_next(&pos);
 	/* other fields...*/
 	uint32_t i = 1;
@@ -474,12 +476,14 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 	}
 	for (; i < defined_field_count; ++i) {
 		field = tuple_format_field((struct tuple_format *)format, i);
-		mp_type = mp_typeof(*pos);
 		if (validate &&
-		    key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE,
-					 i + TUPLE_INDEX_BASE,
-					 tuple_field_is_nullable(field)))
+		    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
+						 tuple_field_is_nullable(field))) {
+			diag_set(ClientError, ER_FIELD_TYPE,
+				 i + TUPLE_INDEX_BASE,
+				 field_type_strs[field->type]);
 			return -1;
+		}
 		if (field->offset_slot != TUPLE_OFFSET_SLOT_NIL) {
 			field_map[field->offset_slot] =
 				(uint32_t) (pos - tuple);
-- 
2.19.2

[PATCH v1 3/5] lib: introduce json_token_path_snprint

[Kirill Shcherbatov]

Implemented a helper function for getting a path to a json_token
in a json_tree. When routine is called with size=0, return value
(as always) as the number of characters that would have been
written in case the output string has been large enough.
We will use this function for reporting tuple format violations
in further patches.

Needed for #1012
---
 src/lib/json/json.c   |  46 +++++++++++++++++
 src/lib/json/json.h   |  13 +++++
 test/unit/json.c      |  20 +++++++-
 test/unit/json.result | 113 ++++++++++++++++++++++++------------------
 4 files changed, 144 insertions(+), 48 deletions(-)

diff --git a/src/lib/json/json.c b/src/lib/json/json.c
index c7909fde2..e933abe68 100644
--- a/src/lib/json/json.c
+++ b/src/lib/json/json.c
@@ -317,6 +317,52 @@ json_path_validate(const char *path, int path_len, int index_base)
 	return rc;
 }
 
+static int
+json_token_snprint(char *buf, int size, const struct json_token *token,
+		   int index_base)
+{
+	enum json_token_type type = token->type;
+	assert(type == JSON_TOKEN_NUM || type == JSON_TOKEN_STR);
+	int len = 0;
+	if (type == JSON_TOKEN_NUM)
+		len = snprintf(buf, size, "[%d]", token->num + index_base);
+	else if (type == JSON_TOKEN_STR)
+		len = snprintf(buf, size, "[\"%.*s\"]", token->len, token->str);
+	return len;
+}
+
+int
+json_token_path_snprint(char *buf, int size, const struct json_token *token,
+		       int index_base)
+{
+	int ret = 0;
+	const struct json_token *ptr = token;
+	while (ptr->type != JSON_TOKEN_END) {
+		ret += json_token_snprint(NULL, 0, ptr, index_base);
+		ptr = ptr->parent;
+	}
+	if (size == 0)
+		return ret;
+
+	int len = ret;
+	char last = '\0';
+	ret = MIN(ret, size - 1);
+	ptr = token;
+	while (ptr->type != JSON_TOKEN_END) {
+		len -= json_token_snprint(NULL, 0, ptr, index_base);
+		assert(len >= 0);
+		if (len + 1 < size) {
+			int rc = json_token_snprint(buf + len, size - len, ptr,
+						    index_base);
+			buf[len + rc] = last;
+			last = buf[len];
+		}
+		ptr = ptr->parent;
+	}
+	buf[ret] = '\0';
+	return ret;
+}
+
 #define MH_SOURCE 1
 #define mh_name _json
 #define mh_key_t struct json_token *
diff --git a/src/lib/json/json.h b/src/lib/json/json.h
index 7ce10ce2c..5df5a1efb 100644
--- a/src/lib/json/json.h
+++ b/src/lib/json/json.h
@@ -253,6 +253,19 @@ json_path_cmp(const char *a, int a_len, const char *b, int b_len,
 int
 json_path_validate(const char *path, int path_len, int index_base);
 
+/**
+ * Write path to JSON tree token to memory buffer buf, at most
+ * size bytes (including the null byte used to end output to\
+ * strings).
+ * When routine is called with size=0, return value
+ * (as always) as the number of characters that would have been
+ * written in case the output string has been large enough.
+ * Return the number of characters printed (excluding '\0').
+ */
+int
+json_token_path_snprint(char *buf, int size, const struct json_token *token,
+			int index_base);
+
 /**
  * Initialize a new empty JSON tree.
  *
diff --git a/test/unit/json.c b/test/unit/json.c
index e553ff23c..7cfea161f 100644
--- a/test/unit/json.c
+++ b/test/unit/json.c
@@ -211,7 +211,7 @@ void
 test_tree()
 {
 	header();
-	plan(54);
+	plan(73);
 
 	struct json_tree tree;
 	int rc = json_tree_create(&tree);
@@ -265,6 +265,24 @@ test_tree()
 					   struct test_struct, node);
 	is(node, NULL, "lookup unregistered path '%s'", path_unregistered);
 
+	/* Test path snprintf. */
+	char buff[64];
+	int len = json_token_path_snprint(NULL, 0, &records[5].node, INDEX_BASE);
+	int path4_copy_len = strlen(path4_copy);
+	is(len, path4_copy_len, "estimate path length: have %d expected %d",
+	   len, path4_copy_len);
+	for (int i = path4_copy_len + 1;
+	     i > strrchr(path4_copy, '[') - path4_copy - 2; i--) {
+		int rc = json_token_path_snprint(buff, i + 1, &records[5].node,
+						 INDEX_BASE);
+		len = MIN(path4_copy_len, i);
+		is(rc, len, "correct char count");
+		is(memcmp(buff, path4_copy, len) == 0, true,
+		   "correct string fragment: have \"%s\", expected \"%.*s\"",
+		   buff, len, path4_copy);
+		is(buff[len], '\0', "terminating '\\0' at appropriate place");
+	}
+
 	/* Test iterators. */
 	struct json_token *token = NULL, *tmp;
 	const struct json_token *tokens_preorder[] =
diff --git a/test/unit/json.result b/test/unit/json.result
index a17451099..4ca6cf4b5 100644
--- a/test/unit/json.result
+++ b/test/unit/json.result
@@ -101,7 +101,7 @@ ok 1 - subtests
 ok 2 - subtests
 	*** test_errors: done ***
 	*** test_tree ***
-    1..54
+    1..73
     ok 1 - add path '[1][10]'
     ok 2 - add path '[1][20].file'
     ok 3 - add path '[1][20].file[2]'
@@ -110,52 +110,71 @@ ok 2 - subtests
     ok 6 - lookup path '[1][10]'
     ok 7 - lookup path '[1][20].file'
     ok 8 - lookup unregistered path '[1][3]'
-    ok 9 - test foreach pre order 0: have 0 expected of 0
-    ok 10 - test foreach pre order 1: have 1 expected of 1
-    ok 11 - test foreach pre order 2: have 2 expected of 2
-    ok 12 - test foreach pre order 3: have 3 expected of 3
-    ok 13 - test foreach pre order 4: have 4 expected of 4
-    ok 14 - test foreach pre order 5: have 5 expected of 5
-    ok 15 - records iterated count 6 of 6
-    ok 16 - test foreach post order 0: have 1 expected of 1
-    ok 17 - test foreach post order 1: have 4 expected of 4
-    ok 18 - test foreach post order 2: have 5 expected of 5
-    ok 19 - test foreach post order 3: have 3 expected of 3
-    ok 20 - test foreach post order 4: have 2 expected of 2
-    ok 21 - test foreach post order 5: have 0 expected of 0
-    ok 22 - records iterated count 6 of 6
-    ok 23 - test foreach safe order 0: have 1 expected of 1
-    ok 24 - test foreach safe order 1: have 4 expected of 4
-    ok 25 - test foreach safe order 2: have 5 expected of 5
-    ok 26 - test foreach safe order 3: have 3 expected of 3
-    ok 27 - test foreach safe order 4: have 2 expected of 2
-    ok 28 - test foreach safe order 5: have 0 expected of 0
-    ok 29 - records iterated count 6 of 6
-    ok 30 - test foreach entry pre order 0: have 0 expected of 0
-    ok 31 - test foreach entry pre order 1: have 1 expected of 1
-    ok 32 - test foreach entry pre order 2: have 2 expected of 2
-    ok 33 - test foreach entry pre order 3: have 3 expected of 3
-    ok 34 - test foreach entry pre order 4: have 4 expected of 4
-    ok 35 - test foreach entry pre order 5: have 5 expected of 5
-    ok 36 - records iterated count 6 of 6
-    ok 37 - test foreach entry post order 0: have 1 expected of 1
-    ok 38 - test foreach entry post order 1: have 4 expected of 4
-    ok 39 - test foreach entry post order 2: have 5 expected of 5
-    ok 40 - test foreach entry post order 3: have 3 expected of 3
-    ok 41 - test foreach entry post order 4: have 2 expected of 2
-    ok 42 - test foreach entry post order 5: have 0 expected of 0
-    ok 43 - records iterated count 6 of 6
-    ok 44 - max_child_index 7 expected of 7
-    ok 45 - max_child_index 1 expected of 1
-    ok 46 - max_child_index -1 expected of -1
-    ok 47 - lookup removed path '[1][20].file[2]'
-    ok 48 - lookup removed path '[1][20].file[8]'
-    ok 49 - lookup path was not corrupted '[1][20].file'
-    ok 50 - test foreach entry safe order 0: have 1 expected of 1
-    ok 51 - test foreach entry safe order 1: have 3 expected of 3
-    ok 52 - test foreach entry safe order 2: have 2 expected of 2
-    ok 53 - test foreach entry safe order 3: have 0 expected of 0
-    ok 54 - records iterated count 4 of 4
+    ok 9 - estimate path length: have 18 expected 18
+    ok 10 - correct char count
+    ok 11 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
+    ok 12 - terminating '\0' at appropriate place
+    ok 13 - correct char count
+    ok 14 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
+    ok 15 - terminating '\0' at appropriate place
+    ok 16 - correct char count
+    ok 17 - correct string fragment: have "[1][20]["file"][8", expected "[1][20]["file"][8"
+    ok 18 - terminating '\0' at appropriate place
+    ok 19 - correct char count
+    ok 20 - correct string fragment: have "[1][20]["file"][", expected "[1][20]["file"]["
+    ok 21 - terminating '\0' at appropriate place
+    ok 22 - correct char count
+    ok 23 - correct string fragment: have "[1][20]["file"]", expected "[1][20]["file"]"
+    ok 24 - terminating '\0' at appropriate place
+    ok 25 - correct char count
+    ok 26 - correct string fragment: have "[1][20]["file"", expected "[1][20]["file""
+    ok 27 - terminating '\0' at appropriate place
+    ok 28 - test foreach pre order 0: have 0 expected of 0
+    ok 29 - test foreach pre order 1: have 1 expected of 1
+    ok 30 - test foreach pre order 2: have 2 expected of 2
+    ok 31 - test foreach pre order 3: have 3 expected of 3
+    ok 32 - test foreach pre order 4: have 4 expected of 4
+    ok 33 - test foreach pre order 5: have 5 expected of 5
+    ok 34 - records iterated count 6 of 6
+    ok 35 - test foreach post order 0: have 1 expected of 1
+    ok 36 - test foreach post order 1: have 4 expected of 4
+    ok 37 - test foreach post order 2: have 5 expected of 5
+    ok 38 - test foreach post order 3: have 3 expected of 3
+    ok 39 - test foreach post order 4: have 2 expected of 2
+    ok 40 - test foreach post order 5: have 0 expected of 0
+    ok 41 - records iterated count 6 of 6
+    ok 42 - test foreach safe order 0: have 1 expected of 1
+    ok 43 - test foreach safe order 1: have 4 expected of 4
+    ok 44 - test foreach safe order 2: have 5 expected of 5
+    ok 45 - test foreach safe order 3: have 3 expected of 3
+    ok 46 - test foreach safe order 4: have 2 expected of 2
+    ok 47 - test foreach safe order 5: have 0 expected of 0
+    ok 48 - records iterated count 6 of 6
+    ok 49 - test foreach entry pre order 0: have 0 expected of 0
+    ok 50 - test foreach entry pre order 1: have 1 expected of 1
+    ok 51 - test foreach entry pre order 2: have 2 expected of 2
+    ok 52 - test foreach entry pre order 3: have 3 expected of 3
+    ok 53 - test foreach entry pre order 4: have 4 expected of 4
+    ok 54 - test foreach entry pre order 5: have 5 expected of 5
+    ok 55 - records iterated count 6 of 6
+    ok 56 - test foreach entry post order 0: have 1 expected of 1
+    ok 57 - test foreach entry post order 1: have 4 expected of 4
+    ok 58 - test foreach entry post order 2: have 5 expected of 5
+    ok 59 - test foreach entry post order 3: have 3 expected of 3
+    ok 60 - test foreach entry post order 4: have 2 expected of 2
+    ok 61 - test foreach entry post order 5: have 0 expected of 0
+    ok 62 - records iterated count 6 of 6
+    ok 63 - max_child_index 7 expected of 7
+    ok 64 - max_child_index 1 expected of 1
+    ok 65 - max_child_index -1 expected of -1
+    ok 66 - lookup removed path '[1][20].file[2]'
+    ok 67 - lookup removed path '[1][20].file[8]'
+    ok 68 - lookup path was not corrupted '[1][20].file'
+    ok 69 - test foreach entry safe order 0: have 1 expected of 1
+    ok 70 - test foreach entry safe order 1: have 3 expected of 3
+    ok 71 - test foreach entry safe order 2: have 2 expected of 2
+    ok 72 - test foreach entry safe order 3: have 0 expected of 0
+    ok 73 - records iterated count 4 of 4
 ok 3 - subtests
 	*** test_tree: done ***
 	*** test_path_cmp ***
-- 
2.19.2

[PATCH v1 4/5] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}

[Kirill Shcherbatov]

Reworked ER_FIELD_TYPE and ER_ACTION_MISMATCH error to pass
JSON path to field string instead of field number.
This patch is required for further JSON patches, to give detailed
information about field on error.

Needed for #1012
---
 src/box/errcode.h                   |  4 +-
 src/box/memtx_rtree.c               |  2 +-
 src/box/tuple.h                     | 15 +++--
 src/box/tuple_format.c              | 36 ++++++------
 test/box/alter.result               |  6 +-
 test/box/alter_limits.result        |  7 ++-
 test/box/blackhole.result           |  6 +-
 test/box/ddl.result                 |  8 ++-
 test/box/hash.result                | 15 +++--
 test/box/rtree_misc.result          | 18 ++++--
 test/box/sequence.result            |  3 +-
 test/box/tree_pk.result             |  9 ++-
 test/engine/ddl.result              | 88 ++++++++++++++++++-----------
 test/engine/indices_any_type.result | 12 ++--
 test/engine/null.result             | 33 +++++++----
 test/engine/tree.result             | 12 ++--
 test/engine/tree_variants.result    | 12 ++--
 test/engine/update.result           |  6 +-
 test/engine/upsert.result           |  9 ++-
 test/replication/misc.result        |  3 +-
 test/vinyl/constraint.result        | 18 ++++--
 test/vinyl/errinj.result            |  3 +-
 test/vinyl/gh.result                |  3 +-
 test/vinyl/savepoint.result         |  3 +-
 24 files changed, 206 insertions(+), 125 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 73359ebdf..7d1f8ddc7 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -75,7 +75,7 @@ struct errcode_record {
 	/* 20 */_(ER_INVALID_MSGPACK,		"Invalid MsgPack - %s") \
 	/* 21 */_(ER_PROC_RET,			"msgpack.encode: can not encode Lua type '%s'") \
 	/* 22 */_(ER_TUPLE_NOT_ARRAY,		"Tuple/Key must be MsgPack array") \
-	/* 23 */_(ER_FIELD_TYPE,		"Tuple field %u type does not match one required by operation: expected %s") \
+	/* 23 */_(ER_FIELD_TYPE,		"Tuple field %s type does not match one required by operation: expected %s") \
 	/* 24 */_(ER_INDEX_PART_TYPE_MISMATCH,	"Field %s has type '%s' in one index, but type '%s' in another") \
 	/* 25 */_(ER_SPLICE,			"SPLICE error on field %u: %s") \
 	/* 26 */_(ER_UPDATE_ARG_TYPE,		"Argument type in operation '%c' on field %u does not match field type: expected %s") \
@@ -214,7 +214,7 @@ struct errcode_record {
 	/*159 */_(ER_SQL_EXECUTE,               "Failed to execute SQL statement: %s") \
 	/*160 */_(ER_SQL,			"SQL error: %s") \
 	/*161 */_(ER_SQL_BIND_NOT_FOUND,	"Parameter %s was not found in the statement") \
-	/*162 */_(ER_ACTION_MISMATCH,		"Field %d contains %s on conflict action, but %s in index parts") \
+	/*162 */_(ER_ACTION_MISMATCH,		"Field %s contains %s on conflict action, but %s in index parts") \
 	/*163 */_(ER_VIEW_MISSING_SQL,		"Space declared as a view must have SQL statement") \
 	/*164 */_(ER_FOREIGN_KEY_CONSTRAINT,	"Can not commit transaction: deferred foreign keys violations are not resolved") \
 	/*165 */_(ER_NO_SUCH_MODULE,		"Module '%s' does not exist") \
diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
index f2aa6c3e5..269660ac3 100644
--- a/src/box/memtx_rtree.c
+++ b/src/box/memtx_rtree.c
@@ -48,7 +48,7 @@ mp_decode_num(const char **data, uint32_t fieldno, double *ret)
 {
 	if (mp_read_double(data, ret) != 0) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			 fieldno + TUPLE_INDEX_BASE,
+			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_NUMBER]);
 		return -1;
 	}
diff --git a/src/box/tuple.h b/src/box/tuple.h
index 3361b6757..3c8b8825e 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -640,8 +640,8 @@ tuple_next_with_type(struct tuple_iterator *it, enum mp_type type)
 	}
 	if (mp_typeof(*field) != type) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			  fieldno + TUPLE_INDEX_BASE,
-			  mp_type_strs[type]);
+			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),
+			 mp_type_strs[type]);
 		return NULL;
 	}
 	return field;
@@ -658,7 +658,7 @@ tuple_next_u32(struct tuple_iterator *it, uint32_t *out)
 	uint32_t val = mp_decode_uint(&field);
 	if (val > UINT32_MAX) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			 fieldno + TUPLE_INDEX_BASE,
+			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_UNSIGNED]);
 		return -1;
 	}
@@ -706,7 +706,8 @@ tuple_field_with_type(const struct tuple *tuple, uint32_t fieldno,
 	}
 	if (mp_typeof(*field) != type) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			 fieldno + TUPLE_INDEX_BASE, mp_type_strs[type]);
+			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),
+			 mp_type_strs[type]);
 		return NULL;
 	}
 	return field;
@@ -751,7 +752,8 @@ tuple_field_i64(const struct tuple *tuple, uint32_t fieldno, int64_t *out)
 		}
 		FALLTHROUGH;
 	default:
-		diag_set(ClientError, ER_FIELD_TYPE, fieldno + TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE,
+			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_INTEGER]);
 		return -1;
 	}
@@ -784,7 +786,8 @@ tuple_field_u32(const struct tuple *tuple, uint32_t fieldno, uint32_t *out)
 		return -1;
 	*out = mp_decode_uint(&field);
 	if (*out > UINT32_MAX) {
-		diag_set(ClientError, ER_FIELD_TYPE, fieldno + TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE,
+			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_UNSIGNED]);
 		return -1;
 	}
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 04343fd53..e29f84dc5 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -61,9 +61,17 @@ tuple_field_delete(struct tuple_field *field)
 	free(field);
 }
 
+static const char *
+tuple_field_path(const struct tuple_field *field)
+{
+	char *msg = tt_static_buf();
+	json_token_path_snprint(msg, TT_STATIC_BUF_LEN, &field->token,
+				TUPLE_INDEX_BASE);
+	return msg;
+}
+
 static int
-tuple_format_use_key_part(struct tuple_format *format,
-			  const struct field_def *fields, uint32_t field_count,
+tuple_format_use_key_part(struct tuple_format *format, uint32_t field_count,
 			  const struct key_part *part, bool is_sequential,
 			  int *current_slot)
 {
@@ -94,9 +102,9 @@ tuple_format_use_key_part(struct tuple_format *format,
 			field->nullable_action = part->nullable_action;
 	} else if (field->nullable_action != part->nullable_action) {
 		diag_set(ClientError, ER_ACTION_MISMATCH,
-				part->fieldno + TUPLE_INDEX_BASE,
-				on_conflict_action_strs[field->nullable_action],
-				on_conflict_action_strs[part->nullable_action]);
+			 tuple_field_path(field),
+			 on_conflict_action_strs[field->nullable_action],
+			 on_conflict_action_strs[part->nullable_action]);
 		return -1;
 	}
 
@@ -111,21 +119,12 @@ tuple_format_use_key_part(struct tuple_format *format,
 		field->type = part->type;
 	} else if (!field_type1_contains_type2(part->type,
 					       field->type)) {
-		const char *name;
-		int fieldno = part->fieldno + TUPLE_INDEX_BASE;
-		if (part->fieldno >= field_count) {
-			name = tt_sprintf("%d", fieldno);
-		} else {
-			const struct field_def *def =
-				&fields[part->fieldno];
-			name = tt_sprintf("'%s'", def->name);
-		}
 		int errcode;
 		if (!field->is_key_part)
 			errcode = ER_FORMAT_MISMATCH_INDEX_PART;
 		else
 			errcode = ER_INDEX_PART_TYPE_MISMATCH;
-		diag_set(ClientError, errcode, name,
+		diag_set(ClientError, errcode, tuple_field_path(field),
 			 field_type_strs[field->type],
 			 field_type_strs[part->type]);
 		return -1;
@@ -190,8 +189,7 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
 		const struct key_part *parts_end = part + key_def->part_count;
 
 		for (; part < parts_end; part++) {
-			if (tuple_format_use_key_part(format, fields,
-						      field_count, part,
+			if (tuple_format_use_key_part(format, field_count, part,
 						      is_sequential,
 						      &current_slot) != 0)
 				return -1;
@@ -456,7 +454,7 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 	if (validate &&
 	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
 					 tuple_field_is_nullable(field))) {
-		diag_set(ClientError, ER_FIELD_TYPE, TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE, tuple_field_path(field),
 			 field_type_strs[field->type]);
 		return -1;
 	}
@@ -480,7 +478,7 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 		    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
 						 tuple_field_is_nullable(field))) {
 			diag_set(ClientError, ER_FIELD_TYPE,
-				 i + TUPLE_INDEX_BASE,
+				 tuple_field_path(field),
 				 field_type_strs[field->type]);
 			return -1;
 		}
diff --git a/test/box/alter.result b/test/box/alter.result
index 9a1086e0c..95746a27b 100644
--- a/test/box/alter.result
+++ b/test/box/alter.result
@@ -36,7 +36,8 @@ _space:insert{_space.id, ADMIN, 'test', 'memtx', 0, EMPTY_MAP, {}}
 --
 _space:insert{'hello', 'world', 'test', 'memtx', 0, EMPTY_MAP, {}}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 --
 -- Can't create a space which has wrong field count - field_count must be NUM
@@ -1025,7 +1026,8 @@ s:replace{1}
 ...
 pk:alter{parts = {{1, 'string'}}} -- Must fail.
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 s:drop()
 ---
diff --git a/test/box/alter_limits.result b/test/box/alter_limits.result
index 4fd80a374..7f7e5a24e 100644
--- a/test/box/alter_limits.result
+++ b/test/box/alter_limits.result
@@ -564,7 +564,7 @@ index = s:create_index('t1', { type = 'hash' })
 -- field type contradicts field type of another index
 index = s:create_index('t2', { type = 'hash', parts = { 1, 'string' }})
 ---
-- error: Field 1 has type 'unsigned' in one index, but type 'string' in another
+- error: Field [1] has type 'unsigned' in one index, but type 'string' in another
 ...
 -- ok
 index = s:create_index('t2', { type = 'hash', parts = { 2, 'string' }})
@@ -837,7 +837,7 @@ s.index.primary:select{}
 -- ambiguous field type
 index = s:create_index('string', { type = 'tree', unique =  false, parts = { 2, 'string'}})
 ---
-- error: Field 2 has type 'unsigned' in one index, but type 'string' in another
+- error: Field [2] has type 'unsigned' in one index, but type 'string' in another
 ...
 -- create index on a non-existing field
 index = s:create_index('nosuchfield', { type = 'tree', unique = true, parts = { 3, 'string'}})
@@ -855,7 +855,8 @@ s:insert{'Der Baader Meinhof Komplex', '2009 '}
 -- create an index on a field with a wrong type
 index = s:create_index('year', { type = 'tree', unique = false, parts = { 2, 'unsigned'}})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 -- a field is missing
 s:replace{'Der Baader Meinhof Komplex'}
diff --git a/test/box/blackhole.result b/test/box/blackhole.result
index 945b2755c..fef85aa27 100644
--- a/test/box/blackhole.result
+++ b/test/box/blackhole.result
@@ -28,11 +28,13 @@ t, t.key, t.value
 ...
 s:insert{1, 2, 3} -- error
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 s:replace{'a', 'b', 'c'} -- error
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 s:format{}
 ---
diff --git a/test/box/ddl.result b/test/box/ddl.result
index d3b0d1e0e..7b4aba156 100644
--- a/test/box/ddl.result
+++ b/test/box/ddl.result
@@ -146,7 +146,7 @@ _ = {fiber.create(insert_tuple, {1, 2, 'a'}), fiber.create(add_index), fiber.cre
 {ch:get(), ch:get(), ch:get()}
 ---
 - - - false
-    - 'Tuple field 2 type does not match one required by operation: expected unsigned'
+    - 'Tuple field [2] type does not match one required by operation: expected unsigned'
   - - true
     - [1, 2, 'a']
   - true
@@ -371,11 +371,13 @@ box.space._collation.index.name:delete{'nothing'} -- allowed
 ...
 box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', 42}
 ---
-- error: 'Tuple field 6 type does not match one required by operation: expected map'
+- error: 'Tuple field [6] type does not match one required by operation: expected
+    map'
 ...
 box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', 'options'}
 ---
-- error: 'Tuple field 6 type does not match one required by operation: expected map'
+- error: 'Tuple field [6] type does not match one required by operation: expected
+    map'
 ...
 box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', {ping='pong'}}
 ---
diff --git a/test/box/hash.result b/test/box/hash.result
index 6893a1be0..4846cfebf 100644
--- a/test/box/hash.result
+++ b/test/box/hash.result
@@ -37,7 +37,8 @@ tmp:bsize() > bsize
 -- Insert invalid fields
 hash:insert{'invalid key', 'value1 v1.0', 'value2 v1.0'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 -------------------------------------------------------------------------------
 -- 32-bit hash replace fields tests
@@ -58,7 +59,8 @@ hash:replace{2, 'value1 v1.43', 'value2 1.92'}
 -- Replace invalid fields
 hash:replace{'invalid key', 'value1 v1.0', 'value2 v1.0'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 -------------------------------------------------------------------------------
 -- 32-bit hash select fields test
@@ -175,7 +177,8 @@ hash:insert{103, 'value1 v1.0', 'value2 v1.0'}
 ...
 hash:insert{'invalid key', 'value1 v1.0', 'value2 v1.0'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 -------------------------------------------------------------------------------
 -- 64-bit hash replace fields tests
@@ -208,7 +211,8 @@ hash:replace{2, 'value1 v1.43', 'value2 1.92'}
 ...
 hash:replace{'invalid key', 'value1 v1.0', 'value2 v1.0'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 -------------------------------------------------------------------------------
 -- 64-bit hash select fields test
@@ -766,7 +770,8 @@ _ = box.space.test:create_index('i',{parts={1,'string'}})
 ...
 box.space.test:insert{1}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 box.space.test:drop()
 ---
diff --git a/test/box/rtree_misc.result b/test/box/rtree_misc.result
index 36d5b8f55..1c9396264 100644
--- a/test/box/rtree_misc.result
+++ b/test/box/rtree_misc.result
@@ -50,11 +50,13 @@ i = s:create_index('secondary', { type = 'hash', parts = {2, 'unsigned'}})
 -- adding a tuple with array instead of num will fail
 i = s:insert{{1, 2, 3}, 4}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 i = s:insert{1, {2, 3, 4}}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 -- rtree index must be one-part
 i = s:create_index('spatial', { type = 'rtree', unique = false, parts = {1, 'array', 2, 'array'}})
@@ -87,15 +89,18 @@ i = s:create_index('spatial', { type = 'rtree', unique = false, parts = {3, 'arr
 -- inserting wrong values (should fail)
 s:insert{1, 2, 3}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected array'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    array'
 ...
 s:insert{1, 2, "3"}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected array'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    array'
 ...
 s:insert{1, 2, nil, 3}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected array'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    array'
 ...
 s:insert{1, 2, {}}
 ---
@@ -554,7 +559,8 @@ s.index.s:drop()
 -- with wrong args
 box.space._index:insert{s.id, 2, 's', 'rtree', nil, {{2, 'array'}}}
 ---
-- error: 'Tuple field 5 type does not match one required by operation: expected map'
+- error: 'Tuple field [5] type does not match one required by operation: expected
+    map'
 ...
 box.space._index:insert{s.id, 2, 's', 'rtree', utils.setmap({}), {{2, 'array'}}}
 ---
diff --git a/test/box/sequence.result b/test/box/sequence.result
index b3907659f..e59498db2 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -1019,7 +1019,8 @@ s.index.pk.sequence_id == nil
 ...
 s:insert{nil, 'x'} -- error
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 box.sequence.test_seq == nil
 ---
diff --git a/test/box/tree_pk.result b/test/box/tree_pk.result
index df3c78bed..fe6385b57 100644
--- a/test/box/tree_pk.result
+++ b/test/box/tree_pk.result
@@ -64,15 +64,18 @@ s0:delete{3}
 -- https://bugs.launchpad.net/tarantool/+bug/1072624
 s0:insert{'xxxxxxx'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 s0:insert{''}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 s0:insert{'12'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 s1 = box.schema.space.create('tweedledee')
 ---
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 3c84e942d..09001c9a1 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -683,7 +683,8 @@ sk = s:create_index('sk', { parts = { 2, 'string' } })
 ...
 s:replace{1, 1}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 sk:drop()
 ---
@@ -707,8 +708,8 @@ pk = s:create_index('pk')
 ...
 sk1 = s:create_index('sk1', { parts = { 2, 'unsigned' } })
 ---
-- error: Field 'field2' has type 'string' in space format, but type 'unsigned' in
-    index definition
+- error: Field [2] has type 'string' in space format, but type 'unsigned' in index
+    definition
 ...
 -- Check space format conflicting with index parts.
 sk3 = s:create_index('sk3', { parts = { 2, 'string' } })
@@ -719,8 +720,8 @@ format[2].type = 'unsigned'
 ...
 s:format(format)
 ---
-- error: Field 'field2' has type 'unsigned' in space format, but type 'string' in
-    index definition
+- error: Field [2] has type 'unsigned' in space format, but type 'string' in index
+    definition
 ...
 s:format()
 ---
@@ -822,27 +823,33 @@ s:replace{1, '2', {3, 3}, 4.4, -5, true, {value=7}, 8, 9}
 ...
 s:replace{1, 2, {3, 3}, 4.4, -5, true, {value=7}, 8, 9}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 s:replace{1, '2', 3, 4.4, -5, true, {value=7}, 8, 9}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected array'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    array'
 ...
 s:replace{1, '2', {3, 3}, '4', -5, true, {value=7}, 8, 9}
 ---
-- error: 'Tuple field 4 type does not match one required by operation: expected number'
+- error: 'Tuple field [4] type does not match one required by operation: expected
+    number'
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5.5, true, {value=7}, 8, 9}
 ---
-- error: 'Tuple field 5 type does not match one required by operation: expected integer'
+- error: 'Tuple field [5] type does not match one required by operation: expected
+    integer'
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5, {6, 6}, {value=7}, 8, 9}
 ---
-- error: 'Tuple field 6 type does not match one required by operation: expected scalar'
+- error: 'Tuple field [6] type does not match one required by operation: expected
+    scalar'
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5, true, {7}, 8, 9}
 ---
-- error: 'Tuple field 7 type does not match one required by operation: expected map'
+- error: 'Tuple field [7] type does not match one required by operation: expected
+    map'
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5, true, {value=7}}
 ---
@@ -1137,7 +1144,7 @@ inspector:cmd("setopt delimiter ''");
 -- any --X--> unsigned
 fail_format_change(2, 'unsigned')
 ---
-- 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- 'Tuple field [2] type does not match one required by operation: expected unsigned'
 ...
 -- unsigned -----> any
 ok_format_change(3, 'any')
@@ -1146,7 +1153,7 @@ ok_format_change(3, 'any')
 -- unsigned --X--> string
 fail_format_change(3, 'string')
 ---
-- 'Tuple field 3 type does not match one required by operation: expected string'
+- 'Tuple field [3] type does not match one required by operation: expected string'
 ...
 -- unsigned -----> number
 ok_format_change(3, 'number')
@@ -1163,7 +1170,7 @@ ok_format_change(3, 'scalar')
 -- unsigned --X--> map
 fail_format_change(3, 'map')
 ---
-- 'Tuple field 3 type does not match one required by operation: expected map'
+- 'Tuple field [3] type does not match one required by operation: expected map'
 ...
 -- string -----> any
 ok_format_change(4, 'any')
@@ -1176,7 +1183,7 @@ ok_format_change(4, 'scalar')
 -- string --X--> boolean
 fail_format_change(4, 'boolean')
 ---
-- 'Tuple field 4 type does not match one required by operation: expected boolean'
+- 'Tuple field [4] type does not match one required by operation: expected boolean'
 ...
 -- number -----> any
 ok_format_change(5, 'any')
@@ -1189,7 +1196,7 @@ ok_format_change(5, 'scalar')
 -- number --X--> integer
 fail_format_change(5, 'integer')
 ---
-- 'Tuple field 5 type does not match one required by operation: expected integer'
+- 'Tuple field [5] type does not match one required by operation: expected integer'
 ...
 -- integer -----> any
 ok_format_change(6, 'any')
@@ -1206,7 +1213,7 @@ ok_format_change(6, 'scalar')
 -- integer --X--> unsigned
 fail_format_change(6, 'unsigned')
 ---
-- 'Tuple field 6 type does not match one required by operation: expected unsigned'
+- 'Tuple field [6] type does not match one required by operation: expected unsigned'
 ...
 -- boolean -----> any
 ok_format_change(7, 'any')
@@ -1219,7 +1226,7 @@ ok_format_change(7, 'scalar')
 -- boolean --X--> string
 fail_format_change(7, 'string')
 ---
-- 'Tuple field 7 type does not match one required by operation: expected string'
+- 'Tuple field [7] type does not match one required by operation: expected string'
 ...
 -- scalar -----> any
 ok_format_change(8, 'any')
@@ -1228,7 +1235,7 @@ ok_format_change(8, 'any')
 -- scalar --X--> unsigned
 fail_format_change(8, 'unsigned')
 ---
-- 'Tuple field 8 type does not match one required by operation: expected unsigned'
+- 'Tuple field [8] type does not match one required by operation: expected unsigned'
 ...
 -- array -----> any
 ok_format_change(9, 'any')
@@ -1237,7 +1244,7 @@ ok_format_change(9, 'any')
 -- array --X--> scalar
 fail_format_change(9, 'scalar')
 ---
-- 'Tuple field 9 type does not match one required by operation: expected scalar'
+- 'Tuple field [9] type does not match one required by operation: expected scalar'
 ...
 -- map -----> any
 ok_format_change(10, 'any')
@@ -1246,7 +1253,7 @@ ok_format_change(10, 'any')
 -- map --X--> scalar
 fail_format_change(10, 'scalar')
 ---
-- 'Tuple field 10 type does not match one required by operation: expected scalar'
+- 'Tuple field [10] type does not match one required by operation: expected scalar'
 ...
 s:drop()
 ---
@@ -1478,7 +1485,8 @@ format[2].is_nullable = false
 ...
 s:format(format)
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 _ = s:delete(1)
 ---
@@ -1536,7 +1544,8 @@ s:insert({1, NULL})
 ...
 s.index.secondary:alter({ parts = {{2, 'string', is_nullable = false} }})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 _ = s:delete({1})
 ---
@@ -1546,7 +1555,8 @@ s.index.secondary:alter({ parts = {{2, 'string', is_nullable = false} }})
 ...
 s:insert({1, NULL})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 s:insert({2, 'xxx'})
 ---
@@ -1666,7 +1676,8 @@ s:replace{1, box.NULL, 1}
 ...
 sk1:alter({parts = {{2, 'unsigned', is_nullable = false}}})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:replace{1, 1, 1}
 ---
@@ -1677,7 +1688,8 @@ sk1:alter({parts = {{2, 'unsigned', is_nullable = false}}})
 ...
 s:replace{1, 1, box.NULL}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    unsigned'
 ...
 sk2:alter({parts = {{3, 'unsigned', is_nullable = true}}})
 ---
@@ -1761,11 +1773,13 @@ s:format()
 ...
 s:replace{1, '100', -20.2}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:replace{1, 100, -20.2}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected integer'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    integer'
 ...
 s:replace{1, 100, -20}
 ---
@@ -1829,11 +1843,13 @@ sk4:alter{parts = {{3, 'integer'}}}
 ...
 s:replace{1, 50.5, 1.5}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:replace{1, 50, 1.5}
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected integer'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    integer'
 ...
 s:replace{5, 5, 5}
 ---
@@ -2006,11 +2022,13 @@ s:create_index('sk', {parts = {2, 'string'}}) -- error: unique constraint
 ...
 s:create_index('sk', {parts = {3, 'string'}}) -- error: nullability constraint
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected string'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    string'
 ...
 s:create_index('sk', {parts = {4, 'unsigned'}}) -- error: field type
 ---
-- error: 'Tuple field 4 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [4] type does not match one required by operation: expected
+    unsigned'
 ...
 s:create_index('sk', {parts = {4, 'integer', 5, 'string'}}) -- error: field missing
 ---
@@ -2059,11 +2077,13 @@ i1:alter{unique = true} -- error: unique contraint
 ...
 i2:alter{parts = {3, 'string'}} -- error: nullability contraint
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected string'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    string'
 ...
 i3:alter{parts = {4, 'unsigned'}} -- error: field type
 ---
-- error: 'Tuple field 4 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [4] type does not match one required by operation: expected
+    unsigned'
 ...
 i3:alter{parts = {4, 'integer', 5, 'string'}} -- error: field missing
 ---
diff --git a/test/engine/indices_any_type.result b/test/engine/indices_any_type.result
index 8158bda4e..02c6c96b1 100644
--- a/test/engine/indices_any_type.result
+++ b/test/engine/indices_any_type.result
@@ -698,15 +698,18 @@ i4_1 = s4:create_index('my_space5_idx1', {type='TREE', parts={1, 'scalar', 2, 'i
 ...
 s4:insert({mp.NULL, 1, 1, 1})
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected scalar'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    scalar'
 ...
 s4:insert({2, mp.NULL, 2, 2}) -- all nulls must fail
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected integer'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    integer'
 ...
 s4:insert({3, 3, mp.NULL, 3})
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected number'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    number'
 ...
 s4:insert({4, 4, 4, mp.NULL})
 ---
@@ -782,7 +785,8 @@ s5:insert({7, true})
 ...
 s5:insert({8, mp.NULL}) -- must fail
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected scalar'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    scalar'
 ...
 s5:insert({9, -40.5})
 ---
diff --git a/test/engine/null.result b/test/engine/null.result
index 757e63185..05f47332d 100644
--- a/test/engine/null.result
+++ b/test/engine/null.result
@@ -970,7 +970,8 @@ s:replace{50, 50}
 ...
 s:replace{25, box.NULL}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 format[2].is_nullable = true
 ---
@@ -1025,7 +1026,8 @@ s:replace{50, 50}
 ...
 s:replace{25, box.NULL}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 sk:alter({parts = {{2, 'unsigned', is_nullable = true}}})
 ---
@@ -1617,7 +1619,8 @@ s:replace{1, 1}
 ...
 s:replace{2, box.NULL}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:select{}
 ---
@@ -1774,7 +1777,8 @@ s:insert{5}
 ...
 s:insert{5, box.NULL}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- This is allowed.
 ---
@@ -1782,7 +1786,8 @@ s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}} -- This is al
 -- Without space format setting this fails.
 s:insert{5, box.NULL}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5}
 ---
@@ -1801,7 +1806,8 @@ s:format(format) -- This is also allowed.
 -- inserts still fail due to not nullable index parts.
 s:insert{5, box.NULL}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5}
 ---
@@ -1838,11 +1844,13 @@ format[2].is_nullable = false
 ...
 s:format(format) -- Fail.
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Fail.
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 _ = s:delete{5}
 ---
@@ -1869,7 +1877,8 @@ s:format(format)
 ...
 s:insert{5, box.NULL} -- Fail.
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5} -- Fail.
 ---
@@ -1887,7 +1896,8 @@ s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
 ...
 s:insert{5, box.NULL} -- Fail.
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5} -- Fail.
 ---
@@ -1935,7 +1945,8 @@ sk2 = s:create_index('sk2', {parts={{2, 'number', is_nullable=true}}})
 ...
 s:insert{2, nil, 2} --error
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected number'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    number'
 ...
 s:drop()
 ---
diff --git a/test/engine/tree.result b/test/engine/tree.result
index 0645f41ef..e1c1fd927 100644
--- a/test/engine/tree.result
+++ b/test/engine/tree.result
@@ -2877,19 +2877,23 @@ sort(space:select{})
 -- https://bugs.launchpad.net/tarantool/+bug/1072624
 space:insert{'', 1, 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:insert{'xxxxxxxx', 1, 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:insert{1, '', 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 space:insert{1, 'xxxxxxxxxxx', 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 space:drop()
 ---
diff --git a/test/engine/tree_variants.result b/test/engine/tree_variants.result
index 0b223d252..ad81a239e 100644
--- a/test/engine/tree_variants.result
+++ b/test/engine/tree_variants.result
@@ -184,19 +184,23 @@ sort(space:select{})
 -- https://bugs.launchpad.net/tarantool/+bug/1072624
 space:insert{'', 1, 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:insert{'xxxxxxxx', 1, 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:insert{1, '', 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 space:insert{1, 'xxxxxxxxxxx', 2, '', '', '', '', '', 0}
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 space:drop()
 ---
diff --git a/test/engine/update.result b/test/engine/update.result
index b73037ebc..c3d7394a8 100644
--- a/test/engine/update.result
+++ b/test/engine/update.result
@@ -722,7 +722,8 @@ aa.VAL
 -- invalid update
 aa:update({{'=',2, 666}})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 -- test transform integrity
 aa:transform(-1, 1)
@@ -753,7 +754,8 @@ box.space.tst_sample:get(2).VAL
 -- invalid upsert
 s:upsert({2, 666}, {{'=', 2, 666}})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 s:drop()
 ---
diff --git a/test/engine/upsert.result b/test/engine/upsert.result
index b35545588..f70ebe665 100644
--- a/test/engine/upsert.result
+++ b/test/engine/upsert.result
@@ -1085,7 +1085,8 @@ index = space:create_index('primary', { type = 'tree', parts = {1, 'unsigned', 2
 ...
 space:upsert({0, 'key', 0}, {{'+', 3, 1}})
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 space:drop()
 ---
@@ -1805,7 +1806,8 @@ index1 = space:create_index('primary', { parts = {1, 'string'} })
 ...
 space:upsert({1}, {{'!', 2, 100}}) -- must fail on checking tuple
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 space:upsert({'a'}, {{'a', 2, 100}}) -- must fail on checking ops
 ---
@@ -1859,7 +1861,8 @@ index2:select{}
 -- test upsert that executes as update
 space:upsert({'a', 100, 100}, {{'=', 3, -200}}) -- must fail on cheking new tuple in secondary index
 ---
-- error: 'Tuple field 3 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    unsigned'
 ...
 space:upsert({'b', 100, 200}, {{'=', 1, 'd'}}) -- must fail with attempt to modify primary index
 ---
diff --git a/test/replication/misc.result b/test/replication/misc.result
index c32681a7a..ede688a01 100644
--- a/test/replication/misc.result
+++ b/test/replication/misc.result
@@ -207,7 +207,8 @@ c = net_box.connect(box.cfg.listen)
 ...
 c.space.space1:insert{box.NULL, "data"} -- fails, but bumps sequence value
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    unsigned'
 ...
 c.space.space1:insert{box.NULL, 1, "data"}
 ---
diff --git a/test/vinyl/constraint.result b/test/vinyl/constraint.result
index 46ed1c9eb..4c533f7f2 100644
--- a/test/vinyl/constraint.result
+++ b/test/vinyl/constraint.result
@@ -7,11 +7,13 @@ index = space:create_index('primary', { type = 'tree', parts = {1, 'string'} })
 ...
 space:insert{1}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 space:replace{1}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 space:delete{1}
 ---
@@ -23,7 +25,8 @@ space:update({1}, {{'=', 1, 101}})
 ...
 space:upsert({1}, {{'+', 1, 10}})
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 space:get{1}
 ---
@@ -45,11 +48,13 @@ index = space:create_index('primary', { type = 'tree', parts = {1, 'unsigned'} }
 ...
 space:insert{'A'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:replace{'A'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:delete{'A'}
 ---
@@ -61,7 +66,8 @@ space:update({'A'}, {{'=', 1, 101}})
 ...
 space:upsert({'A'}, {{'+', 1, 10}})
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:get{'A'}
 ---
diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
index a081575be..dec469423 100644
--- a/test/vinyl/errinj.result
+++ b/test/vinyl/errinj.result
@@ -1449,7 +1449,8 @@ format[2].is_nullable = false
 ...
 s:format(format) -- must fail
 ---
-- error: 'Tuple field 2 type does not match one required by operation: expected string'
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0)
 ---
diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result
index 76beab094..d702b42e7 100644
--- a/test/vinyl/gh.result
+++ b/test/vinyl/gh.result
@@ -158,7 +158,8 @@ i = s:create_index('primary',{parts={1, 'string'}})
 ...
 box.space.t:insert{1,'A'}
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected string'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    string'
 ...
 s:drop()
 ---
diff --git a/test/vinyl/savepoint.result b/test/vinyl/savepoint.result
index d7b57a775..fb93b8fe1 100644
--- a/test/vinyl/savepoint.result
+++ b/test/vinyl/savepoint.result
@@ -424,7 +424,8 @@ space:upsert({12}, {})
 ...
 space:insert({'abc'})
 ---
-- error: 'Tuple field 1 type does not match one required by operation: expected unsigned'
+- error: 'Tuple field [1] type does not match one required by operation: expected
+    unsigned'
 ...
 space:update({1}, {{'#', 2, 1}})
 ---
-- 
2.19.2

[PATCH v1 5/5] box: refactor tuple_init_field_map to use bitmap

[Kirill Shcherbatov]

Refactored tuple_init_field_map to fill temporal bitmap and
compare it with template req_fields_bitmap containing information
about required fields. Each field is mapped to bitmap using
unique field identifier.
This approach to check the required fields will work even after
the introduction of JSON paths, when the field tree becomes
multilevel.

Needed for #1012
---
 src/box/errcode.h                 |   2 +-
 src/box/tuple_format.c            | 183 +++++++++++++++++++++++-------
 src/box/tuple_format.h            |  23 ++++
 test/box/alter_limits.result      |   8 +-
 test/box/ddl.result               |  24 ++--
 test/box/misc.result              |   2 +-
 test/box/sql.result               |  12 +-
 test/box/tree_pk_multipart.result |   8 +-
 test/engine/ddl.result            |  28 ++---
 test/engine/null.result           |  52 ++++-----
 test/vinyl/constraint.result      |  12 +-
 test/vinyl/errinj.result          |  12 +-
 test/vinyl/savepoint.result       |   8 +-
 13 files changed, 250 insertions(+), 124 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 7d1f8ddc7..812e643d8 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -91,7 +91,7 @@ struct errcode_record {
 	/* 36 */_(ER_NO_SUCH_SPACE,		"Space '%s' does not exist") \
 	/* 37 */_(ER_NO_SUCH_FIELD,		"Field %d was not found in the tuple") \
 	/* 38 */_(ER_EXACT_FIELD_COUNT,		"Tuple field count %u does not match space field count %u") \
-	/* 39 */_(ER_MIN_FIELD_COUNT,		"Tuple field count %u is less than required by space format or defined indexes (expected at least %u)") \
+	/* 39 */_(ER_FIELD_STRUCTURE_MISMATCH,	"Tuple field %s does not math document structure defined by space format or index: %s") \
 	/* 40 */_(ER_WAL_IO,			"Failed to write to disk") \
 	/* 41 */_(ER_MORE_THAN_ONE_TUPLE,	"Get() doesn't support partial keys and non-unique indexes") \
 	/* 42 */_(ER_ACCESS_DENIED,		"%s access to %s '%s' is denied for user '%s'") \
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index e29f84dc5..6e269fd77 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -28,6 +28,8 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
+#include "bit/bit.h"
+#include "fiber.h"
 #include "json/json.h"
 #include "tuple_format.h"
 #include "coll_id_cache.h"
@@ -206,6 +208,30 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
 		return -1;
 	}
 	format->field_map_size = field_map_size;
+
+	/* Allocate required fields bitmap. */
+	uint32_t req_fields_bitmap_sz = (format->total_field_count +
+					 CHAR_BIT * sizeof(unsigned long) - 1) /
+					 CHAR_BIT * sizeof(unsigned long);
+	format->req_fields_bitmap = calloc(1, req_fields_bitmap_sz);
+	if (format->req_fields_bitmap == NULL) {
+		diag_set(OutOfMemory, req_fields_bitmap_sz, "calloc",
+			 "format->req_fields_bitmap");
+		return -1;
+	}
+	struct tuple_field *field;
+	struct json_token *root = (struct json_token *)&format->fields.root;
+	json_tree_foreach_entry_preorder(field, root, struct tuple_field,
+					 token) {
+		/*
+		 * Mark all leaf non-nullable fields as required
+		 * setting corresponding bit 1 in format
+		 * req_fields_bitmap.
+		 */
+		if (tuple_field_is_leaf(field) &&
+		    !tuple_field_is_nullable(field))
+			bit_set(format->req_fields_bitmap, field->id);
+	}
 	return 0;
 }
 
@@ -304,6 +330,7 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
 		struct tuple_field *field = tuple_field_new();
 		if (field == NULL)
 			goto error;
+		field->id = fieldno;
 		field->token.num = fieldno;
 		field->token.type = JSON_TOKEN_NUM;
 		if (json_tree_add(&format->fields, &format->fields.root,
@@ -323,6 +350,8 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
 		format->dict = dict;
 		tuple_dictionary_ref(dict);
 	}
+	format->total_field_count = field_count;
+	format->req_fields_bitmap = NULL;
 	format->refs = 0;
 	format->id = FORMAT_ID_NIL;
 	format->index_field_count = index_field_count;
@@ -339,6 +368,7 @@ error:
 static inline void
 tuple_format_destroy(struct tuple_format *format)
 {
+	free(format->req_fields_bitmap);
 	tuple_format_destroy_fields(format);
 	tuple_dictionary_unref(format->dict);
 }
@@ -422,6 +452,74 @@ tuple_format1_can_store_format2_tuples(struct tuple_format *format1,
 	return true;
 }
 
+/**
+ * Return meta information of a tuple field given a format
+ * and a unique field identifier.
+ */
+static struct tuple_field *
+tuple_format_field_by_id(const struct tuple_format *format, uint32_t id)
+{
+	struct tuple_field *field;
+	struct json_token *root = (struct json_token *)&format->fields.root;
+	json_tree_foreach_entry_preorder(field, root, struct tuple_field,
+					 token) {
+		if (field->id == id)
+			return field;
+	}
+	return NULL;
+}
+
+/**
+ * Analyze fields_bitmap to ensure that all required fields
+ * present in tuple. Routine relies on req_fields_bitmap is
+ * initialized on tuple_format_create and all required field's
+ * id bits are set 1.
+ */
+static int
+tuple_format_fields_bitmap_test(const struct tuple_format *format,
+				const char *fields_bitmap)
+{
+	struct tuple_field *missed_field = NULL;
+	const char *req_fields_bitmap = format->req_fields_bitmap;
+	for (uint32_t i = 0; i < format->total_field_count; i++) {
+		bool is_required = bit_test(req_fields_bitmap, i);
+		if (is_required && is_required != bit_test(fields_bitmap, i)) {
+			missed_field = tuple_format_field_by_id(format, i);
+			assert(missed_field != NULL);
+			break;
+		}
+	}
+	if (missed_field == NULL)
+		return 0;
+
+	struct json_token *token = &missed_field->token;
+	const char *err;
+	if (missed_field->token.type == JSON_TOKEN_STR) {
+		err = tt_sprintf("map does not contain a key \"%.*s\"",
+				 token->len, token->str);
+	} else if (missed_field->token.type == JSON_TOKEN_NUM) {
+		struct json_token *parent = token->parent;
+		/*
+		 * Determine minimal field size looking for
+		 * the greatest fieldno between non-nullable
+		 * missed_field neighbors.
+		 */
+		uint32_t expected_size;
+		for (int i = 0; i <= parent->max_child_idx; i++) {
+			struct tuple_field *field =
+				tuple_format_field((struct tuple_format *)format,
+						   i);
+			if (!tuple_field_is_nullable(field))
+				expected_size = i + 1;
+		}
+		err = tt_sprintf("invalid array size %d (expected at least %d)",
+				 token->num, expected_size);
+	}
+	diag_set(ClientError, ER_FIELD_STRUCTURE_MISMATCH,
+		 tuple_field_path(missed_field), err);
+	return -1;
+}
+
 /** @sa declaration for details. */
 int
 tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
@@ -441,54 +539,59 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 			 (unsigned) format->exact_field_count);
 		return -1;
 	}
-	if (validate && field_count < format->min_field_count) {
-		diag_set(ClientError, ER_MIN_FIELD_COUNT,
-			 (unsigned) field_count,
-			 (unsigned) format->min_field_count);
-		return -1;
-	}
-
-	/* first field is simply accessible, so we do not store offset to it */
-	const struct tuple_field *field =
-		tuple_format_field((struct tuple_format *)format, 0);
-	if (validate &&
-	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
-					 tuple_field_is_nullable(field))) {
-		diag_set(ClientError, ER_FIELD_TYPE, tuple_field_path(field),
-			 field_type_strs[field->type]);
-		return -1;
-	}
-	mp_next(&pos);
-	/* other fields...*/
-	uint32_t i = 1;
 	uint32_t defined_field_count = MIN(field_count, validate ?
 					   tuple_format_field_count(format) :
 					   format->index_field_count);
-	if (field_count < format->index_field_count) {
-		/*
-		 * Nullify field map to be able to detect by 0,
-		 * which key fields are absent in tuple_field().
-		 */
-		memset((char *)field_map - format->field_map_size, 0,
-		       format->field_map_size);
-	}
-	for (; i < defined_field_count; ++i) {
-		field = tuple_format_field((struct tuple_format *)format, i);
-		if (validate &&
-		    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
-						 tuple_field_is_nullable(field))) {
-			diag_set(ClientError, ER_FIELD_TYPE,
-				 tuple_field_path(field),
-				 field_type_strs[field->type]);
+	struct region *region = &fiber()->gc;
+	char *fields_bitmap = NULL;
+	if (validate) {
+		uint32_t fields_bitmap_sz = (format->total_field_count +
+					     CHAR_BIT *
+					     sizeof(unsigned long) - 1) /
+					     CHAR_BIT * sizeof(unsigned long);
+		fields_bitmap = region_alloc(region, fields_bitmap_sz);
+		if (fields_bitmap == NULL) {
+			diag_set(OutOfMemory, fields_bitmap_sz, "calloc",
+				"fields_bitmap");
 			return -1;
 		}
-		if (field->offset_slot != TUPLE_OFFSET_SLOT_NIL) {
-			field_map[field->offset_slot] =
-				(uint32_t) (pos - tuple);
+		memset(fields_bitmap, 0, fields_bitmap_sz);
+	}
+	memset((char *)field_map - format->field_map_size, 0,
+	       format->field_map_size);
+	struct json_tree *tree = (struct json_tree *)&format->fields;
+	struct json_token *parent = &tree->root;
+	struct json_token token;
+	token.parent = NULL;
+	token.type = JSON_TOKEN_NUM;
+	token.num = 0;
+	while ((uint32_t)token.num < defined_field_count) {
+		struct tuple_field *field =
+			json_tree_lookup_entry(tree, parent, &token,
+					       struct tuple_field, token);
+		if (field != NULL) {
+			bool is_nullable = tuple_field_is_nullable(field);
+			if (validate &&
+			    !field_mp_type_is_compatible(field->type,
+							 mp_typeof(*pos),
+							 is_nullable) != 0) {
+				diag_set(ClientError, ER_FIELD_TYPE,
+					 tuple_field_path(field),
+					 field_type_strs[field->type]);
+				return -1;
+			}
+			if (field->offset_slot != TUPLE_OFFSET_SLOT_NIL) {
+				field_map[field->offset_slot] =
+					(uint32_t)(pos - tuple);
+			}
+			if (validate)
+				bit_set(fields_bitmap, field->id);
 		}
+		token.num++;
 		mp_next(&pos);
 	}
-	return 0;
+	return validate ?
+	       tuple_format_fields_bitmap_test(format, fields_bitmap) : 0;
 }
 
 uint32_t
diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
index 949337807..947d0d8a5 100644
--- a/src/box/tuple_format.h
+++ b/src/box/tuple_format.h
@@ -31,6 +31,7 @@
  * SUCH DAMAGE.
  */
 
+#include "bit/bit.h"
 #include "key_def.h"
 #include "field_def.h"
 #include "errinj.h"
@@ -114,6 +115,8 @@ struct tuple_field {
 	struct coll *coll;
 	/** Collation identifier. */
 	uint32_t coll_id;
+	/** Field unique identifier in tuple_format. */
+	uint32_t id;
 	/** Link in tuple_format::fields. */
 	struct json_token token;
 };
@@ -173,6 +176,20 @@ struct tuple_format {
 	 * Shared names storage used by all formats of a space.
 	 */
 	struct tuple_dictionary *dict;
+	/**
+	 * This bitmap of "required fields" contains information
+	 * about fields that must present in tuple to be inserted.
+	 * Look for tuple_format_fields_bitmap_test comment for
+	 * more details.
+	 */
+	char *req_fields_bitmap;
+	/**
+	 * Total count of format fields in fields subtree.
+	 * Required to allocate temporal objects containing
+	 * attributes for all fields. Particularly to allocate
+	 * temporal req_fields_bitmap on region.
+	 */
+	uint32_t total_field_count;
 	/**
 	 * Fields comprising the format, organized in a tree.
 	 * First level nodes correspond to tuple fields.
@@ -194,6 +211,12 @@ tuple_format_field_count(const struct tuple_format *format)
 	return root->children != NULL ? root->max_child_idx + 1 : 0;
 }
 
+static inline bool
+tuple_field_is_leaf(struct tuple_field *field)
+{
+	return field->token.max_child_idx == -1;
+}
+
 /**
  * Return meta information of a top-level tuple field given
  * a format and a field index.
diff --git a/test/box/alter_limits.result b/test/box/alter_limits.result
index 7f7e5a24e..d32b7bed5 100644
--- a/test/box/alter_limits.result
+++ b/test/box/alter_limits.result
@@ -842,8 +842,8 @@ index = s:create_index('string', { type = 'tree', unique =  false, parts = { 2,
 -- create index on a non-existing field
 index = s:create_index('nosuchfield', { type = 'tree', unique = true, parts = { 3, 'string'}})
 ---
-- error: Tuple field count 2 is less than required by space format or defined indexes
-    (expected at least 3)
+- error: 'Tuple field [3] does not math document structure defined by space format
+    or index: invalid array size 2 (expected at least 3)'
 ...
 s.index.year:drop()
 ---
@@ -865,8 +865,8 @@ s:replace{'Der Baader Meinhof Komplex'}
 ...
 index = s:create_index('year', { type = 'tree', unique = false, parts = { 2, 'unsigned'}})
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s:drop()
 ---
diff --git a/test/box/ddl.result b/test/box/ddl.result
index 7b4aba156..eaf58fe78 100644
--- a/test/box/ddl.result
+++ b/test/box/ddl.result
@@ -326,33 +326,33 @@ box.internal.collation.drop('test')
 ...
 box.space._collation:auto_increment{'test'}
 ---
-- error: Tuple field count 2 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [3] does not math document structure defined by space format
+    or index: invalid array size 2 (expected at least 6)'
 ...
 box.space._collation:auto_increment{'test', 0, 'ICU'}
 ---
-- error: Tuple field count 4 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [5] does not math document structure defined by space format
+    or index: invalid array size 4 (expected at least 6)'
 ...
 box.space._collation:auto_increment{'test', 'ADMIN', 'ICU', 'ru_RU'}
 ---
-- error: Tuple field count 5 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [3] type does not match one required by operation: expected
+    unsigned'
 ...
 box.space._collation:auto_increment{42, 0, 'ICU', 'ru_RU'}
 ---
-- error: Tuple field count 5 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [2] type does not match one required by operation: expected
+    string'
 ...
 box.space._collation:auto_increment{'test', 0, 42, 'ru_RU'}
 ---
-- error: Tuple field count 5 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [4] type does not match one required by operation: expected
+    string'
 ...
 box.space._collation:auto_increment{'test', 0, 'ICU', 42}
 ---
-- error: Tuple field count 5 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [5] type does not match one required by operation: expected
+    string'
 ...
 box.space._collation:auto_increment{'test', 0, 'ICU', 'ru_RU', setmap{}} --ok
 ---
diff --git a/test/box/misc.result b/test/box/misc.result
index d266bb334..479a401ea 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -369,7 +369,7 @@ t;
   36: box.error.NO_SUCH_SPACE
   37: box.error.NO_SUCH_FIELD
   38: box.error.EXACT_FIELD_COUNT
-  39: box.error.MIN_FIELD_COUNT
+  39: box.error.FIELD_STRUCTURE_MISMATCH
   40: box.error.WAL_IO
   41: box.error.MORE_THAN_ONE_TUPLE
   42: box.error.ACCESS_DENIED
diff --git a/test/box/sql.result b/test/box/sql.result
index 1818b294d..cfbe3fe41 100644
--- a/test/box/sql.result
+++ b/test/box/sql.result
@@ -299,8 +299,8 @@ s:truncate()
 -- get away with it.
 space:insert{'Britney'}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 sorted(space.index.secondary:select('Anything'))
 ---
@@ -308,8 +308,8 @@ sorted(space.index.secondary:select('Anything'))
 ...
 space:insert{'Stephanie'}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 sorted(space.index.secondary:select('Anything'))
 ---
@@ -638,8 +638,8 @@ sorted(space.index.secondary:select('Britney'))
 -- try to insert the incoplete tuple
 space:replace{'Spears'}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 -- check that nothing has been updated
 space:select{'Spears'}
diff --git a/test/box/tree_pk_multipart.result b/test/box/tree_pk_multipart.result
index 28cab3f94..e05d3ed00 100644
--- a/test/box/tree_pk_multipart.result
+++ b/test/box/tree_pk_multipart.result
@@ -490,13 +490,13 @@ i1 = space:create_index('primary', { type = 'tree', parts = {1, 'unsigned', 3, '
 ...
 space:insert{1, 1}
 ---
-- error: Tuple field count 2 is less than required by space format or defined indexes
-    (expected at least 3)
+- error: 'Tuple field [3] does not math document structure defined by space format
+    or index: invalid array size 2 (expected at least 3)'
 ...
 space:replace{1, 1}
 ---
-- error: Tuple field count 2 is less than required by space format or defined indexes
-    (expected at least 3)
+- error: 'Tuple field [3] does not math document structure defined by space format
+    or index: invalid array size 2 (expected at least 3)'
 ...
 space:drop()
 ---
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 09001c9a1..5f9bd7fa9 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -77,8 +77,8 @@ index = space:create_index('primary', {type = 'tree', parts = {1, 'unsigned', 2,
 ...
 space:insert({13})
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 space:drop()
 ---
@@ -853,13 +853,13 @@ s:replace{1, '2', {3, 3}, 4.4, -5, true, {7}, 8, 9}
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5, true, {value=7}}
 ---
-- error: Tuple field count 7 is less than required by space format or defined indexes
-    (expected at least 9)
+- error: 'Tuple field [8] does not math document structure defined by space format
+    or index: invalid array size 7 (expected at least 9)'
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5, true, {value=7}, 8}
 ---
-- error: Tuple field count 8 is less than required by space format or defined indexes
-    (expected at least 9)
+- error: 'Tuple field [9] does not math document structure defined by space format
+    or index: invalid array size 8 (expected at least 9)'
 ...
 s:truncate()
 ---
@@ -1343,8 +1343,8 @@ s:format(format)
 -- Fail, not enough fields.
 s:replace{2, 2, 2, 2, 2}
 ---
-- error: Tuple field count 5 is less than required by space format or defined indexes
-    (expected at least 6)
+- error: 'Tuple field [6] does not math document structure defined by space format
+    or index: invalid array size 5 (expected at least 6)'
 ...
 s:replace{2, 2, 2, 2, 2, 2, 2}
 ---
@@ -1356,8 +1356,8 @@ format[7] = {name = 'field7', type = 'unsigned'}
 -- Fail, the tuple {1, ... 1} is invalid for a new format.
 s:format(format)
 ---
-- error: Tuple field count 6 is less than required by space format or defined indexes
-    (expected at least 7)
+- error: 'Tuple field [7] does not math document structure defined by space format
+    or index: invalid array size 6 (expected at least 7)'
 ...
 s:drop()
 ---
@@ -2032,8 +2032,8 @@ s:create_index('sk', {parts = {4, 'unsigned'}}) -- error: field type
 ...
 s:create_index('sk', {parts = {4, 'integer', 5, 'string'}}) -- error: field missing
 ---
-- error: Tuple field count 4 is less than required by space format or defined indexes
-    (expected at least 5)
+- error: 'Tuple field [5] does not math document structure defined by space format
+    or index: invalid array size 4 (expected at least 5)'
 ...
 i1 = s:create_index('i1', {parts = {2, 'string'}, unique = false})
 ---
@@ -2087,8 +2087,8 @@ i3:alter{parts = {4, 'unsigned'}} -- error: field type
 ...
 i3:alter{parts = {4, 'integer', 5, 'string'}} -- error: field missing
 ---
-- error: Tuple field count 4 is less than required by space format or defined indexes
-    (expected at least 5)
+- error: 'Tuple field [5] does not math document structure defined by space format
+    or index: invalid array size 4 (expected at least 5)'
 ...
 i3:alter{parts = {2, 'string', 4, 'integer'}} -- ok
 ---
diff --git a/test/engine/null.result b/test/engine/null.result
index 05f47332d..797a525dc 100644
--- a/test/engine/null.result
+++ b/test/engine/null.result
@@ -458,8 +458,8 @@ sk = s:create_index('sk', {parts = {2, 'unsigned'}})
 ...
 s:replace{1, 2} -- error
 ---
-- error: Tuple field count 2 is less than required by space format or defined indexes
-    (expected at least 3)
+- error: 'Tuple field [3] does not math document structure defined by space format
+    or index: invalid array size 2 (expected at least 3)'
 ...
 t1 = s:replace{2, 3, 4}
 ---
@@ -530,18 +530,18 @@ sk = s:create_index('sk', {parts = {2, 'unsigned'}})
 ...
 s:replace{1, 2} -- error
 ---
-- error: Tuple field count 2 is less than required by space format or defined indexes
-    (expected at least 5)
+- error: 'Tuple field [3] does not math document structure defined by space format
+    or index: invalid array size 2 (expected at least 5)'
 ...
 s:replace{2, 3, 4} -- error
 ---
-- error: Tuple field count 3 is less than required by space format or defined indexes
-    (expected at least 5)
+- error: 'Tuple field [5] does not math document structure defined by space format
+    or index: invalid array size 4 (expected at least 5)'
 ...
 s:replace{3, 4, 5, 6} -- error
 ---
-- error: Tuple field count 4 is less than required by space format or defined indexes
-    (expected at least 5)
+- error: 'Tuple field [5] does not math document structure defined by space format
+    or index: invalid array size 4 (expected at least 5)'
 ...
 t1 = s:replace{4, 5, 6, 7, 8}
 ---
@@ -1071,8 +1071,8 @@ sk = s:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}})
 -- Test tuple_compare_slowpath, tuple_compare_with_key_slowpath.
 s:replace{} -- Fail
 ---
-- error: Tuple field count 0 is less than required by space format or defined indexes
-    (expected at least 1)
+- error: 'Tuple field [1] does not math document structure defined by space format
+    or index: invalid array size 0 (expected at least 1)'
 ...
 -- Compare full vs not full.
 s:replace{2}
@@ -1772,8 +1772,8 @@ s:format(format)
 -- Field 2 is not nullable.
 s:insert{5}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s:insert{5, box.NULL}
 ---
@@ -1791,8 +1791,8 @@ s:insert{5, box.NULL}
 ...
 s:insert{5}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}}
 ---
@@ -1811,8 +1811,8 @@ s:insert{5, box.NULL}
 ...
 s:insert{5}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=true}}}
 ---
@@ -1857,13 +1857,13 @@ _ = s:delete{5}
 ...
 s:format(format) -- Still fail.
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s.index.secondary:alter{parts={{2, 'unsigned', is_nullable=false}}} -- Still fail.
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 -- Now check we can set nullability to false step by step.
 _ = s:delete{6}
@@ -1882,8 +1882,8 @@ s:insert{5, box.NULL} -- Fail.
 ...
 s:insert{5} -- Fail.
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 format[2].is_nullable = true
 ---
@@ -1901,8 +1901,8 @@ s:insert{5, box.NULL} -- Fail.
 ...
 s:insert{5} -- Fail.
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 format[2].is_nullable = false
 ---
@@ -1916,8 +1916,8 @@ s:select{}
 ...
 s:insert{5} -- Fail.
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s:insert{9, 10} -- Success.
 ---
diff --git a/test/vinyl/constraint.result b/test/vinyl/constraint.result
index 4c533f7f2..61161ee98 100644
--- a/test/vinyl/constraint.result
+++ b/test/vinyl/constraint.result
@@ -89,13 +89,13 @@ index = space:create_index('primary', { type = 'tree', parts = {1,'unsigned',2,'
 ...
 space:insert{1}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 space:replace{1}
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 space:delete{1}
 ---
@@ -107,8 +107,8 @@ space:update(1, {{'=', 1, 101}})
 ...
 space:upsert({1}, {{'+', 1, 10}})
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 space:get{1}
 ---
diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
index dec469423..f176cc029 100644
--- a/test/vinyl/errinj.result
+++ b/test/vinyl/errinj.result
@@ -1635,8 +1635,8 @@ errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0.001)
 ...
 s:create_index('sk', {parts = {2, 'unsigned'}}) -- must fail
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0)
 ---
@@ -2088,8 +2088,8 @@ fiber.sleep(0)
 ...
 s:format{{'key', 'unsigned'}, {'value', 'unsigned'}} -- must fail
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s:select()
 ---
@@ -2113,8 +2113,8 @@ fiber.sleep(0)
 ...
 s:create_index('sk', {parts = {2, 'unsigned'}})
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 2)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 2)'
 ...
 s:select()
 ---
diff --git a/test/vinyl/savepoint.result b/test/vinyl/savepoint.result
index fb93b8fe1..e80f21a8c 100644
--- a/test/vinyl/savepoint.result
+++ b/test/vinyl/savepoint.result
@@ -124,8 +124,8 @@ index2 = space:create_index('secondary', { parts = {2, 'int', 3, 'str'} })
 ...
 space:insert({1})
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 3)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 3)'
 ...
 space:insert({1, 1, 'a'})
 ---
@@ -624,8 +624,8 @@ space:insert({4, 2, 'b'})
 ...
 space:upsert({2}, {{'=', 4, 1000}})
 ---
-- error: Tuple field count 1 is less than required by space format or defined indexes
-    (expected at least 3)
+- error: 'Tuple field [2] does not math document structure defined by space format
+    or index: invalid array size 1 (expected at least 3)'
 ...
 index3:delete({3, 'a'})
 ---
-- 
2.19.2

Re: [PATCH v1 1/5] box: refactor tuple_validate_raw

[Vladimir Davydov]

Pushed to 2.1.

Re: [PATCH v1 2/5] box: refactor field type and nullability checks

[Vladimir Davydov]

Pushed to 2.1.

Re: [PATCH v1 3/5] lib: introduce json_token_path_snprint

[Vladimir Davydov]

On Sun, Dec 23, 2018 at 03:40:38PM +0300, Kirill Shcherbatov wrote:
> Implemented a helper function for getting a path to a json_token
> in a json_tree. When routine is called with size=0, return value
> (as always) as the number of characters that would have been
> written in case the output string has been large enough.
> We will use this function for reporting tuple format violations
> in further patches.
> 
> Needed for #1012
> ---
>  src/lib/json/json.c   |  46 +++++++++++++++++
>  src/lib/json/json.h   |  13 +++++
>  test/unit/json.c      |  20 +++++++-
>  test/unit/json.result | 113 ++++++++++++++++++++++++------------------
>  4 files changed, 144 insertions(+), 48 deletions(-)
> 
> diff --git a/src/lib/json/json.c b/src/lib/json/json.c
> index c7909fde2..e933abe68 100644
> --- a/src/lib/json/json.c
> +++ b/src/lib/json/json.c
> @@ -317,6 +317,52 @@ json_path_validate(const char *path, int path_len, int index_base)
>  	return rc;
>  }
>  

Comment is missing.

> +static int
> +json_token_snprint(char *buf, int size, const struct json_token *token,
> +		   int index_base)
> +{
> +	enum json_token_type type = token->type;
> +	assert(type == JSON_TOKEN_NUM || type == JSON_TOKEN_STR);
> +	int len = 0;
> +	if (type == JSON_TOKEN_NUM)
> +		len = snprintf(buf, size, "[%d]", token->num + index_base);
> +	else if (type == JSON_TOKEN_STR)
> +		len = snprintf(buf, size, "[\"%.*s\"]", token->len, token->str);
> +	return len;
> +}
> +
> +int
> +json_token_path_snprint(char *buf, int size, const struct json_token *token,
> +		       int index_base)

The function name is confusing: it implies that the function is
applicable to any token while in fact it's only relevant to tokens
linked in a tree. What about json_tree_snprint_path()?

Also, I think that this function should take not only the token itself,
but also the tree root as it would be more flexible that way. We may
need this in future, e.g. for properly reporting tuple field names
specified in the tuple format dictionary.

> +{
> +	int ret = 0;
> +	const struct json_token *ptr = token;
> +	while (ptr->type != JSON_TOKEN_END) {

It would be nice to see some comments here, explaining what the two
loops comprising this function do.

> +		ret += json_token_snprint(NULL, 0, ptr, index_base);
> +		ptr = ptr->parent;
> +	}
> +	if (size == 0)
> +		return ret;
> +
> +	int len = ret;

Looking at how this variable is used, I can say that it's named
improperly: I'd rather call it 'pos'. And I'd also rename 'ret' to
'len', because it denotes the length of the output string, actually.

> +	char last = '\0';
> +	ret = MIN(ret, size - 1);
> +	ptr = token;
> +	while (ptr->type != JSON_TOKEN_END) {
> +		len -= json_token_snprint(NULL, 0, ptr, index_base);
> +		assert(len >= 0);
> +		if (len + 1 < size) {
> +			int rc = json_token_snprint(buf + len, size - len, ptr,
> +						    index_base);
> +			buf[len + rc] = last;

Please add a comment explaining why you need to restore the last
character in the output here.

> +			last = buf[len];
> +		}
> +		ptr = ptr->parent;
> +	}
> +	buf[ret] = '\0';
> +	return ret;
> +}
> +
>  #define MH_SOURCE 1
>  #define mh_name _json
>  #define mh_key_t struct json_token *
> diff --git a/src/lib/json/json.h b/src/lib/json/json.h
> index 7ce10ce2c..5df5a1efb 100644
> --- a/src/lib/json/json.h
> +++ b/src/lib/json/json.h
> @@ -253,6 +253,19 @@ json_path_cmp(const char *a, int a_len, const char *b, int b_len,
>  int
>  json_path_validate(const char *path, int path_len, int index_base);
>  
> +/**
> + * Write path to JSON tree token to memory buffer buf, at most
> + * size bytes (including the null byte used to end output to\

What's '\' here for?

> + * strings).
> + * When routine is called with size=0, return value
> + * (as always) as the number of characters that would have been

s/as/is

> + * written in case the output string has been large enough.

s/has/had

> + * Return the number of characters printed (excluding '\0').
> + */
> +int
> +json_token_path_snprint(char *buf, int size, const struct json_token *token,
> +			int index_base);
> +

Please carefully read snprintf() manual page and fix this function so
that it behaves in the same way, i.e. always returns the number of
characters that would have been printed if there had been enough space
in the buffer.


>  /**
>   * Initialize a new empty JSON tree.
>   *
> diff --git a/test/unit/json.c b/test/unit/json.c
> index e553ff23c..7cfea161f 100644
> --- a/test/unit/json.c
> +++ b/test/unit/json.c
> @@ -211,7 +211,7 @@ void
>  test_tree()
>  {
>  	header();
> -	plan(54);
> +	plan(73);
>  
>  	struct json_tree tree;
>  	int rc = json_tree_create(&tree);
> @@ -265,6 +265,24 @@ test_tree()
>  					   struct test_struct, node);
>  	is(node, NULL, "lookup unregistered path '%s'", path_unregistered);
>  
> +	/* Test path snprintf. */
> +	char buff[64];
> +	int len = json_token_path_snprint(NULL, 0, &records[5].node, INDEX_BASE);
> +	int path4_copy_len = strlen(path4_copy);
> +	is(len, path4_copy_len, "estimate path length: have %d expected %d",
> +	   len, path4_copy_len);
> +	for (int i = path4_copy_len + 1;
> +	     i > strrchr(path4_copy, '[') - path4_copy - 2; i--) {
> +		int rc = json_token_path_snprint(buff, i + 1, &records[5].node,
> +						 INDEX_BASE);
> +		len = MIN(path4_copy_len, i);
> +		is(rc, len, "correct char count");
> +		is(memcmp(buff, path4_copy, len) == 0, true,
> +		   "correct string fragment: have \"%s\", expected \"%.*s\"",
> +		   buff, len, path4_copy);
> +		is(buff[len], '\0', "terminating '\\0' at appropriate place");
> +	}
> +
>  	/* Test iterators. */
>  	struct json_token *token = NULL, *tmp;
>  	const struct json_token *tokens_preorder[] =
> diff --git a/test/unit/json.result b/test/unit/json.result
> index a17451099..4ca6cf4b5 100644
> --- a/test/unit/json.result
> +++ b/test/unit/json.result
> @@ -101,7 +101,7 @@ ok 1 - subtests
>  ok 2 - subtests
>  	*** test_errors: done ***
>  	*** test_tree ***
> -    1..54
> +    1..73
>      ok 1 - add path '[1][10]'
>      ok 2 - add path '[1][20].file'
>      ok 3 - add path '[1][20].file[2]'
> @@ -110,52 +110,71 @@ ok 2 - subtests
>      ok 6 - lookup path '[1][10]'
>      ok 7 - lookup path '[1][20].file'
>      ok 8 - lookup unregistered path '[1][3]'
> -    ok 9 - test foreach pre order 0: have 0 expected of 0
> -    ok 10 - test foreach pre order 1: have 1 expected of 1
> -    ok 11 - test foreach pre order 2: have 2 expected of 2
> -    ok 12 - test foreach pre order 3: have 3 expected of 3
> -    ok 13 - test foreach pre order 4: have 4 expected of 4
> -    ok 14 - test foreach pre order 5: have 5 expected of 5
> -    ok 15 - records iterated count 6 of 6
> -    ok 16 - test foreach post order 0: have 1 expected of 1
> -    ok 17 - test foreach post order 1: have 4 expected of 4
> -    ok 18 - test foreach post order 2: have 5 expected of 5
> -    ok 19 - test foreach post order 3: have 3 expected of 3
> -    ok 20 - test foreach post order 4: have 2 expected of 2
> -    ok 21 - test foreach post order 5: have 0 expected of 0
> -    ok 22 - records iterated count 6 of 6
> -    ok 23 - test foreach safe order 0: have 1 expected of 1
> -    ok 24 - test foreach safe order 1: have 4 expected of 4
> -    ok 25 - test foreach safe order 2: have 5 expected of 5
> -    ok 26 - test foreach safe order 3: have 3 expected of 3
> -    ok 27 - test foreach safe order 4: have 2 expected of 2
> -    ok 28 - test foreach safe order 5: have 0 expected of 0
> -    ok 29 - records iterated count 6 of 6
> -    ok 30 - test foreach entry pre order 0: have 0 expected of 0
> -    ok 31 - test foreach entry pre order 1: have 1 expected of 1
> -    ok 32 - test foreach entry pre order 2: have 2 expected of 2
> -    ok 33 - test foreach entry pre order 3: have 3 expected of 3
> -    ok 34 - test foreach entry pre order 4: have 4 expected of 4
> -    ok 35 - test foreach entry pre order 5: have 5 expected of 5
> -    ok 36 - records iterated count 6 of 6
> -    ok 37 - test foreach entry post order 0: have 1 expected of 1
> -    ok 38 - test foreach entry post order 1: have 4 expected of 4
> -    ok 39 - test foreach entry post order 2: have 5 expected of 5
> -    ok 40 - test foreach entry post order 3: have 3 expected of 3
> -    ok 41 - test foreach entry post order 4: have 2 expected of 2
> -    ok 42 - test foreach entry post order 5: have 0 expected of 0
> -    ok 43 - records iterated count 6 of 6
> -    ok 44 - max_child_index 7 expected of 7
> -    ok 45 - max_child_index 1 expected of 1
> -    ok 46 - max_child_index -1 expected of -1
> -    ok 47 - lookup removed path '[1][20].file[2]'
> -    ok 48 - lookup removed path '[1][20].file[8]'
> -    ok 49 - lookup path was not corrupted '[1][20].file'
> -    ok 50 - test foreach entry safe order 0: have 1 expected of 1
> -    ok 51 - test foreach entry safe order 1: have 3 expected of 3
> -    ok 52 - test foreach entry safe order 2: have 2 expected of 2
> -    ok 53 - test foreach entry safe order 3: have 0 expected of 0
> -    ok 54 - records iterated count 4 of 4
> +    ok 9 - estimate path length: have 18 expected 18
> +    ok 10 - correct char count
> +    ok 11 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
> +    ok 12 - terminating '\0' at appropriate place
> +    ok 13 - correct char count
> +    ok 14 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
> +    ok 15 - terminating '\0' at appropriate place
> +    ok 16 - correct char count
> +    ok 17 - correct string fragment: have "[1][20]["file"][8", expected "[1][20]["file"][8"
> +    ok 18 - terminating '\0' at appropriate place
> +    ok 19 - correct char count
> +    ok 20 - correct string fragment: have "[1][20]["file"][", expected "[1][20]["file"]["
> +    ok 21 - terminating '\0' at appropriate place
> +    ok 22 - correct char count
> +    ok 23 - correct string fragment: have "[1][20]["file"]", expected "[1][20]["file"]"
> +    ok 24 - terminating '\0' at appropriate place
> +    ok 25 - correct char count
> +    ok 26 - correct string fragment: have "[1][20]["file"", expected "[1][20]["file""
> +    ok 27 - terminating '\0' at appropriate place

Please add the new test case to the end of the file so that it doesn't
inflate the diff by shifting the output of existing test cases.

> +    ok 28 - test foreach pre order 0: have 0 expected of 0
> +    ok 29 - test foreach pre order 1: have 1 expected of 1
> +    ok 30 - test foreach pre order 2: have 2 expected of 2
> +    ok 31 - test foreach pre order 3: have 3 expected of 3
> +    ok 32 - test foreach pre order 4: have 4 expected of 4
> +    ok 33 - test foreach pre order 5: have 5 expected of 5

Re: [PATCH v1 4/5] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}

[Vladimir Davydov]

On Sun, Dec 23, 2018 at 03:40:39PM +0300, Kirill Shcherbatov wrote:
> Reworked ER_FIELD_TYPE and ER_ACTION_MISMATCH error to pass
> JSON path to field string instead of field number.
> This patch is required for further JSON patches, to give detailed
> information about field on error.
> 
> Needed for #1012
> ---
>  src/box/errcode.h                   |  4 +-
>  src/box/memtx_rtree.c               |  2 +-
>  src/box/tuple.h                     | 15 +++--
>  src/box/tuple_format.c              | 36 ++++++------
>  test/box/alter.result               |  6 +-
>  test/box/alter_limits.result        |  7 ++-
>  test/box/blackhole.result           |  6 +-
>  test/box/ddl.result                 |  8 ++-
>  test/box/hash.result                | 15 +++--
>  test/box/rtree_misc.result          | 18 ++++--
>  test/box/sequence.result            |  3 +-
>  test/box/tree_pk.result             |  9 ++-
>  test/engine/ddl.result              | 88 ++++++++++++++++++-----------
>  test/engine/indices_any_type.result | 12 ++--
>  test/engine/null.result             | 33 +++++++----
>  test/engine/tree.result             | 12 ++--
>  test/engine/tree_variants.result    | 12 ++--
>  test/engine/update.result           |  6 +-
>  test/engine/upsert.result           |  9 ++-
>  test/replication/misc.result        |  3 +-
>  test/vinyl/constraint.result        | 18 ++++--
>  test/vinyl/errinj.result            |  3 +-
>  test/vinyl/gh.result                |  3 +-
>  test/vinyl/savepoint.result         |  3 +-
>  24 files changed, 206 insertions(+), 125 deletions(-)
> 
> diff --git a/src/box/tuple.h b/src/box/tuple.h
> index 3361b6757..3c8b8825e 100644
> --- a/src/box/tuple.h
> +++ b/src/box/tuple.h
> @@ -784,7 +786,8 @@ tuple_field_u32(const struct tuple *tuple, uint32_t fieldno, uint32_t *out)
>  		return -1;
>  	*out = mp_decode_uint(&field);
>  	if (*out > UINT32_MAX) {
> -		diag_set(ClientError, ER_FIELD_TYPE, fieldno + TUPLE_INDEX_BASE,
> +		diag_set(ClientError, ER_FIELD_TYPE,
> +			 tt_sprintf("%u", fieldno + TUPLE_INDEX_BASE),

Please use int2str helper instead of tt_sprintf wherever applicable.
BTW I told you that during the previous review iteration.

>  			 field_type_strs[FIELD_TYPE_UNSIGNED]);
>  		return -1;
>  	}
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index 04343fd53..e29f84dc5 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -61,9 +61,17 @@ tuple_field_delete(struct tuple_field *field)
>  	free(field);
>  }
>  

Comment is missing.

> +static const char *
> +tuple_field_path(const struct tuple_field *field)
> +{
> +	char *msg = tt_static_buf();
> +	json_token_path_snprint(msg, TT_STATIC_BUF_LEN, &field->token,
> +				TUPLE_INDEX_BASE);

I don't like that sometimes we report a field number as [NUM] and
sometimes simply as NUM (e.g. see tuple_field_u32). Let's always
use NUM for top-level tuple fields. This means that for now this
function should look as simple as

	assert(field->token.parent != NULL);
	assert(field->token.parent->parent == NULL)
	assert(field->token.type == JSON_TOKEN_NUM);
	return int2str(field->token.num + TUPLE_INDEX_BASE);

We should start using json_token_path_snprint() later, in the main patch
of the series introducing JSON indexes.

> +	return msg;
> +}
> +

Re: [tarantool-patches] Re: [PATCH v1 3/5] lib: introduce json_token_path_snprint

[Kirill Shcherbatov]

Hi! Thank you for your comments. A new version bellow:
=========================================

Implemented a helper function for getting a path to a json_token
in a json_tree. When routine is called with size=0, return value
(as always) as the number of characters that would have been
written in case the output string has been large enough.
We will use this function for reporting tuple format violations
in further patches.

Needed for #1012
---
 src/lib/json/json.c   | 71 +++++++++++++++++++++++++++++++++++++++++++
 src/lib/json/json.h   | 24 +++++++++++++++
 test/unit/json.c      | 67 +++++++++++++++++++++++++++++++++++++++-
 test/unit/json.result | 30 +++++++++++++++++-
 4 files changed, 190 insertions(+), 2 deletions(-)

diff --git a/src/lib/json/json.c b/src/lib/json/json.c
index c7909fde2..57449a3aa 100644
--- a/src/lib/json/json.c
+++ b/src/lib/json/json.c
@@ -317,6 +317,77 @@ json_path_validate(const char *path, int path_len, int index_base)
 	return rc;
 }
 
+/**
+ * Helper routine for json_tree_snprint_path to snprintf atomic
+ * token. Works the same way as json_tree_snprint_path, read it's
+ * comment for more details.
+ */
+static int
+json_token_snprint(char *buf, int size, const struct json_token *token,
+		   int index_base)
+{
+	enum json_token_type type = token->type;
+	assert(type == JSON_TOKEN_NUM || type == JSON_TOKEN_STR);
+	int len = 0;
+	if (type == JSON_TOKEN_NUM)
+		len = snprintf(buf, size, "[%d]", token->num + index_base);
+	else if (type == JSON_TOKEN_STR)
+		len = snprintf(buf, size, "[\"%.*s\"]", token->len, token->str);
+	return len;
+}
+
+int
+json_tree_snprint_path(char *buf, int size, const struct json_token *token,
+		       const struct json_token *root, int index_base)
+{
+	/*
+	 * Calculate JSON path string length passing 0-buffer in
+	 * json_token_snprint routine.
+	 */
+	int len = 0;
+	const struct json_token *ptr = token;
+	while (ptr != root && ptr->type != JSON_TOKEN_END) {
+		len += json_token_snprint(NULL, 0, ptr, index_base);
+		ptr = ptr->parent;
+	}
+	if (size == 0)
+		return len;
+
+	/*
+	 * Write to the memory buffer buf as many tokens,
+	 * starting with the root, as it can fit. If the fragment
+	 * of the token does not fit into the buffer, it would be
+	 * truncated.
+	 */
+	int pos = len;
+	char last = '\0';
+	ptr = token;
+	while (ptr != root && ptr->type != JSON_TOKEN_END) {
+		pos -= json_token_snprint(NULL, 0, ptr, index_base);
+		assert(pos >= 0);
+		if (pos + 1 < size) {
+			int rc = json_token_snprint(buf + pos, size - pos, ptr,
+						    index_base);
+			if (rc < 0)
+				return rc;
+			/*
+			 * The json_token_snprint writes a
+			 * '\0'-terminated string in memory
+			 * buffer. The first character in
+			 * token string representation would be
+			 * overwritten on next cycle iteration.
+			 * Let's keep it in 'last' variable to
+			 * restore next time.
+			 */
+			buf[pos + rc] = last;
+			last = buf[pos];
+		}
+		ptr = ptr->parent;
+	}
+	assert(buf[MIN(len, size - 1)] == '\0');
+	return len;
+}
+
 #define MH_SOURCE 1
 #define mh_name _json
 #define mh_key_t struct json_token *
diff --git a/src/lib/json/json.h b/src/lib/json/json.h
index 7ce10ce2c..bed20536d 100644
--- a/src/lib/json/json.h
+++ b/src/lib/json/json.h
@@ -253,6 +253,30 @@ json_path_cmp(const char *a, int a_len, const char *b, int b_len,
 int
 json_path_validate(const char *path, int path_len, int index_base);
 
+/**
+ * Write a JSON tree path path between root and token items to
+ * memory buffer buf, at most size bytes (including the null byte
+ * used to end output to strings).
+ * The parent argument may omitted via passing NULL value. It this
+ * case the whole JSON path would be printed.
+ * When routine is called with size=0, return value (as always)
+ * is the number of characters that would have been written in
+ * case the output string had been large enough.
+ *
+ * Upon successful return, routine returns the number of
+ * characters printed (excluding the null byte used to end output
+ * to strings). If the output was truncated due the size limit,
+ * then the return value is the number of characters (excluding
+ * the terminating null byte) which would have been written to
+ * the final string if enough space had been available.
+ * Thus, a return value of size or more means that the output
+ * was truncated.
+ * If an output error is encountered, a negative value is
+ * returned.
+ */
+int
+json_tree_snprint_path(char *buf, int size, const struct json_token *token,
+		       const struct json_token *root, int index_base);
 /**
  * Initialize a new empty JSON tree.
  *
diff --git a/test/unit/json.c b/test/unit/json.c
index e553ff23c..122e605cf 100644
--- a/test/unit/json.c
+++ b/test/unit/json.c
@@ -438,16 +438,81 @@ test_path_cmp()
 	footer();
 }
 
+void
+test_path_snprintf()
+{
+	header();
+	plan(24);
+
+	struct json_tree tree;
+	int rc = json_tree_create(&tree);
+	fail_if(rc != 0);
+	struct test_struct records[5];
+	const char *path = "[1][20][\"file\"][8]";
+	int path_len = strlen(path);
+
+	int records_idx = 0;
+	struct test_struct *node, *node_tmp;
+	node = test_add_path(&tree, path, path_len, records, &records_idx);
+	fail_if(&node->node != &records[3].node);
+
+	/* Test size estimation. */
+	char buff[64];
+	int len = json_tree_snprint_path(NULL, 0, &node->node, NULL, INDEX_BASE);
+	is(len, path_len, "estimate path length: have %d expected %d",
+	   len, path_len);
+	len = json_tree_snprint_path(NULL, 0, &node->node, &records[0].node,
+				     INDEX_BASE);
+	is(len, 15, "estimate path part length: have %d expected %d", len, 15);
+
+	len = json_tree_snprint_path(NULL, 0, &node->node, &records[1].node,
+				     INDEX_BASE);
+	is(len, 11, "estimate path part length: have %d expected %d", len, 11);
+	len = json_tree_snprint_path(NULL, 0, &node->node, &records[2].node,
+				     INDEX_BASE);
+	is(len, 3, "estimate path part length: have %d expected %d", len, 3);
+
+	/* Test truncate. */
+	for (int i = path_len + 1; i > strrchr(path, '[') - path - 2; i--) {
+		len = json_tree_snprint_path(buff, i + 1, &node->node, NULL,
+					     INDEX_BASE);
+		is(len, path_len, "correct char count: have %d expected %d",
+		   len, path_len);
+		len = MIN(len, i);
+		is(memcmp(buff, path, len), 0,
+		   "correct string fragment: have \"%s\", expected \"%.*s\"",
+		   buff, len, path);
+		is(buff[len], '\0', "terminating '\\0' at appropriate place");
+	}
+
+	/* Test path fragment snprintf. */
+	len = json_tree_snprint_path(buff, 16, &node->node, &records[0].node,
+				     INDEX_BASE);
+	is(memcmp(buff, path + 3, len), 0,
+	   "correct partial JSON path: have \"%s\" expected \"%s\"", buff,
+	   path + 3);
+	is(buff[len], '\0', "terminating '\\0' at appropriate place");
+
+	json_tree_foreach_entry_safe(node, &tree.root, struct test_struct,
+				     node, node_tmp)
+		json_tree_del(&tree, &node->node);
+	json_tree_destroy(&tree);
+
+	check_plan();
+	footer();
+}
+
 int
 main()
 {
 	header();
-	plan(4);
+	plan(5);
 
 	test_basic();
 	test_errors();
 	test_tree();
 	test_path_cmp();
+	test_path_snprintf();
 
 	int rc = check_plan();
 	footer();
diff --git a/test/unit/json.result b/test/unit/json.result
index a17451099..4e18c7ab7 100644
--- a/test/unit/json.result
+++ b/test/unit/json.result
@@ -1,5 +1,5 @@
 	*** main ***
-1..4
+1..5
 	*** test_basic ***
     1..71
     ok 1 - parse <[1]>
@@ -169,4 +169,32 @@ ok 3 - subtests
     ok 7 - path Data[[1]["FIO"].fname error pos 6 expected 6
 ok 4 - subtests
 	*** test_path_cmp: done ***
+	*** test_path_snprintf ***
+    1..24
+    ok 1 - estimate path length: have 18 expected 18
+    ok 2 - estimate path part length: have 15 expected 15
+    ok 3 - estimate path part length: have 11 expected 11
+    ok 4 - estimate path part length: have 3 expected 3
+    ok 5 - correct char count: have 18 expected 18
+    ok 6 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
+    ok 7 - terminating '\0' at appropriate place
+    ok 8 - correct char count: have 18 expected 18
+    ok 9 - correct string fragment: have "[1][20]["file"][8]", expected "[1][20]["file"][8]"
+    ok 10 - terminating '\0' at appropriate place
+    ok 11 - correct char count: have 18 expected 18
+    ok 12 - correct string fragment: have "[1][20]["file"][8", expected "[1][20]["file"][8"
+    ok 13 - terminating '\0' at appropriate place
+    ok 14 - correct char count: have 18 expected 18
+    ok 15 - correct string fragment: have "[1][20]["file"][", expected "[1][20]["file"]["
+    ok 16 - terminating '\0' at appropriate place
+    ok 17 - correct char count: have 18 expected 18
+    ok 18 - correct string fragment: have "[1][20]["file"]", expected "[1][20]["file"]"
+    ok 19 - terminating '\0' at appropriate place
+    ok 20 - correct char count: have 18 expected 18
+    ok 21 - correct string fragment: have "[1][20]["file"", expected "[1][20]["file""
+    ok 22 - terminating '\0' at appropriate place
+    ok 23 - correct partial JSON path: have "[20]["file"][8]" expected "[20]["file"][8]"
+    ok 24 - terminating '\0' at appropriate place
+ok 5 - subtests
+	*** test_path_snprintf: done ***
 	*** main: done ***
-- 
2.19.2

Re: [tarantool-patches] Re: [PATCH v1 4/5] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}

[Kirill Shcherbatov]

Reworked ER_FIELD_TYPE and ER_ACTION_MISMATCH error to pass path
string to field instead of field number.
This patch is required for further JSON patches, to give detailed
information about field on error.

Needed for #1012
---
 src/box/errcode.h      |  4 ++--
 src/box/memtx_rtree.c  |  2 +-
 src/box/tuple.h        | 15 +++++++++------
 src/box/tuple_format.c | 37 ++++++++++++++++++-------------------
 test/engine/ddl.result |  6 ++----
 5 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 9d3e89f85..94381f9f7 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -75,7 +75,7 @@ struct errcode_record {
 	/* 20 */_(ER_INVALID_MSGPACK,		"Invalid MsgPack - %s") \
 	/* 21 */_(ER_PROC_RET,			"msgpack.encode: can not encode Lua type '%s'") \
 	/* 22 */_(ER_TUPLE_NOT_ARRAY,		"Tuple/Key must be MsgPack array") \
-	/* 23 */_(ER_FIELD_TYPE,		"Tuple field %u type does not match one required by operation: expected %s") \
+	/* 23 */_(ER_FIELD_TYPE,		"Tuple field %s type does not match one required by operation: expected %s") \
 	/* 24 */_(ER_INDEX_PART_TYPE_MISMATCH,	"Field %s has type '%s' in one index, but type '%s' in another") \
 	/* 25 */_(ER_SPLICE,			"SPLICE error on field %u: %s") \
 	/* 26 */_(ER_UPDATE_ARG_TYPE,		"Argument type in operation '%c' on field %u does not match field type: expected %s") \
@@ -214,7 +214,7 @@ struct errcode_record {
 	/*159 */_(ER_SQL_EXECUTE,               "Failed to execute SQL statement: %s") \
 	/*160 */_(ER_SQL,			"SQL error: %s") \
 	/*161 */_(ER_SQL_BIND_NOT_FOUND,	"Parameter %s was not found in the statement") \
-	/*162 */_(ER_ACTION_MISMATCH,		"Field %d contains %s on conflict action, but %s in index parts") \
+	/*162 */_(ER_ACTION_MISMATCH,		"Field %s contains %s on conflict action, but %s in index parts") \
 	/*163 */_(ER_VIEW_MISSING_SQL,		"Space declared as a view must have SQL statement") \
 	/*164 */_(ER_FOREIGN_KEY_CONSTRAINT,	"Can not commit transaction: deferred foreign keys violations are not resolved") \
 	/*165 */_(ER_NO_SUCH_MODULE,		"Module '%s' does not exist") \
diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
index f2aa6c3e5..0f5e0ac53 100644
--- a/src/box/memtx_rtree.c
+++ b/src/box/memtx_rtree.c
@@ -48,7 +48,7 @@ mp_decode_num(const char **data, uint32_t fieldno, double *ret)
 {
 	if (mp_read_double(data, ret) != 0) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			 fieldno + TUPLE_INDEX_BASE,
+			 int2str(fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_NUMBER]);
 		return -1;
 	}
diff --git a/src/box/tuple.h b/src/box/tuple.h
index 3361b6757..83e5b7013 100644
--- a/src/box/tuple.h
+++ b/src/box/tuple.h
@@ -640,8 +640,8 @@ tuple_next_with_type(struct tuple_iterator *it, enum mp_type type)
 	}
 	if (mp_typeof(*field) != type) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			  fieldno + TUPLE_INDEX_BASE,
-			  mp_type_strs[type]);
+			 int2str(fieldno + TUPLE_INDEX_BASE),
+			 mp_type_strs[type]);
 		return NULL;
 	}
 	return field;
@@ -658,7 +658,7 @@ tuple_next_u32(struct tuple_iterator *it, uint32_t *out)
 	uint32_t val = mp_decode_uint(&field);
 	if (val > UINT32_MAX) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			 fieldno + TUPLE_INDEX_BASE,
+			 int2str(fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_UNSIGNED]);
 		return -1;
 	}
@@ -706,7 +706,8 @@ tuple_field_with_type(const struct tuple *tuple, uint32_t fieldno,
 	}
 	if (mp_typeof(*field) != type) {
 		diag_set(ClientError, ER_FIELD_TYPE,
-			 fieldno + TUPLE_INDEX_BASE, mp_type_strs[type]);
+			 int2str(fieldno + TUPLE_INDEX_BASE),
+			 mp_type_strs[type]);
 		return NULL;
 	}
 	return field;
@@ -751,7 +752,8 @@ tuple_field_i64(const struct tuple *tuple, uint32_t fieldno, int64_t *out)
 		}
 		FALLTHROUGH;
 	default:
-		diag_set(ClientError, ER_FIELD_TYPE, fieldno + TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE,
+			 int2str(fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_INTEGER]);
 		return -1;
 	}
@@ -784,7 +786,8 @@ tuple_field_u32(const struct tuple *tuple, uint32_t fieldno, uint32_t *out)
 		return -1;
 	*out = mp_decode_uint(&field);
 	if (*out > UINT32_MAX) {
-		diag_set(ClientError, ER_FIELD_TYPE, fieldno + TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE,
+			 int2str(fieldno + TUPLE_INDEX_BASE),
 			 field_type_strs[FIELD_TYPE_UNSIGNED]);
 		return -1;
 	}
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 1a21444ca..e66a5f521 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -61,9 +61,18 @@ tuple_field_delete(struct tuple_field *field)
 	free(field);
 }
 
+/** Format a field into path string using a static buffer. */
+static const char *
+tuple_field_path(const struct tuple_field *field)
+{
+	assert(field->token.parent != NULL);
+	assert(field->token.parent->parent == NULL);
+	assert(field->token.type == JSON_TOKEN_NUM);
+	return int2str(field->token.num + TUPLE_INDEX_BASE);
+}
+
 static int
-tuple_format_use_key_part(struct tuple_format *format,
-			  const struct field_def *fields, uint32_t field_count,
+tuple_format_use_key_part(struct tuple_format *format, uint32_t field_count,
 			  const struct key_part *part, bool is_sequential,
 			  int *current_slot)
 {
@@ -94,9 +103,9 @@ tuple_format_use_key_part(struct tuple_format *format,
 			field->nullable_action = part->nullable_action;
 	} else if (field->nullable_action != part->nullable_action) {
 		diag_set(ClientError, ER_ACTION_MISMATCH,
-				part->fieldno + TUPLE_INDEX_BASE,
-				on_conflict_action_strs[field->nullable_action],
-				on_conflict_action_strs[part->nullable_action]);
+			 tuple_field_path(field),
+			 on_conflict_action_strs[field->nullable_action],
+			 on_conflict_action_strs[part->nullable_action]);
 		return -1;
 	}
 
@@ -111,21 +120,12 @@ tuple_format_use_key_part(struct tuple_format *format,
 		field->type = part->type;
 	} else if (!field_type1_contains_type2(part->type,
 					       field->type)) {
-		const char *name;
-		int fieldno = part->fieldno + TUPLE_INDEX_BASE;
-		if (part->fieldno >= field_count) {
-			name = tt_sprintf("%d", fieldno);
-		} else {
-			const struct field_def *def =
-				&fields[part->fieldno];
-			name = tt_sprintf("'%s'", def->name);
-		}
 		int errcode;
 		if (!field->is_key_part)
 			errcode = ER_FORMAT_MISMATCH_INDEX_PART;
 		else
 			errcode = ER_INDEX_PART_TYPE_MISMATCH;
-		diag_set(ClientError, errcode, name,
+		diag_set(ClientError, errcode, tuple_field_path(field),
 			 field_type_strs[field->type],
 			 field_type_strs[part->type]);
 		return -1;
@@ -190,8 +190,7 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
 		const struct key_part *parts_end = part + key_def->part_count;
 
 		for (; part < parts_end; part++) {
-			if (tuple_format_use_key_part(format, fields,
-						      field_count, part,
+			if (tuple_format_use_key_part(format, field_count, part,
 						      is_sequential,
 						      &current_slot) != 0)
 				return -1;
@@ -453,7 +452,7 @@ tuple_init_field_map(struct tuple_format *format, uint32_t *field_map,
 	if (validate &&
 	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
 					 tuple_field_is_nullable(field))) {
-		diag_set(ClientError, ER_FIELD_TYPE, TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE, tuple_field_path(field),
 			 field_type_strs[field->type]);
 		return -1;
 	}
@@ -477,7 +476,7 @@ tuple_init_field_map(struct tuple_format *format, uint32_t *field_map,
 		    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
 						 tuple_field_is_nullable(field))) {
 			diag_set(ClientError, ER_FIELD_TYPE,
-				 i + TUPLE_INDEX_BASE,
+				 tuple_field_path(field),
 				 field_type_strs[field->type]);
 			return -1;
 		}
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index e3cff7f45..272ff7618 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -707,8 +707,7 @@ pk = s:create_index('pk')
 ...
 sk1 = s:create_index('sk1', { parts = { 2, 'unsigned' } })
 ---
-- error: Field 'field2' has type 'string' in space format, but type 'unsigned' in
-    index definition
+- error: Field 2 has type 'string' in space format, but type 'unsigned' in index definition
 ...
 -- Check space format conflicting with index parts.
 sk3 = s:create_index('sk3', { parts = { 2, 'string' } })
@@ -719,8 +718,7 @@ format[2].type = 'unsigned'
 ...
 s:format(format)
 ---
-- error: Field 'field2' has type 'unsigned' in space format, but type 'string' in
-    index definition
+- error: Field 2 has type 'unsigned' in space format, but type 'string' in index definition
 ...
 s:format()
 ---
-- 
2.19.2

Re: [tarantool-patches] Re: [PATCH v1 4/5] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}

[Vladimir Davydov]

Pushed to 2.1.

Re: [tarantool-patches] Re: [PATCH v1 3/5] lib: introduce json_token_path_snprint

[Vladimir Davydov]

On Tue, Dec 25, 2018 at 11:53:08AM +0300, Kirill Shcherbatov wrote:
> diff --git a/src/lib/json/json.c b/src/lib/json/json.c
> index c7909fde2..57449a3aa 100644
> --- a/src/lib/json/json.c
> +++ b/src/lib/json/json.c
> @@ -317,6 +317,77 @@ json_path_validate(const char *path, int path_len, int index_base)
>  	return rc;
>  }
>  
> +/**
> + * Helper routine for json_tree_snprint_path to snprintf atomic

atomic?

> + * token. Works the same way as json_tree_snprint_path, read it's
> + * comment for more details.

You could simply copy the description of any other snprint-like function
we have:

/**
 * An snprint-style helper to print an individual token key.
 */

Would be more concise and clear.

> + */
> +static int
> +json_token_snprint(char *buf, int size, const struct json_token *token,
> +		   int index_base)
> +{
> +	enum json_token_type type = token->type;
> +	assert(type == JSON_TOKEN_NUM || type == JSON_TOKEN_STR);
> +	int len = 0;
> +	if (type == JSON_TOKEN_NUM)
> +		len = snprintf(buf, size, "[%d]", token->num + index_base);
> +	else if (type == JSON_TOKEN_STR)
> +		len = snprintf(buf, size, "[\"%.*s\"]", token->len, token->str);
> +	return len;
> +}
> +
> +int
> +json_tree_snprint_path(char *buf, int size, const struct json_token *token,
> +		       const struct json_token *root, int index_base)

root should go first in the argument list, because it goes first in all
other json_tree methods.

> +{
> +	/*
> +	 * Calculate JSON path string length passing 0-buffer in
> +	 * json_token_snprint routine.
> +	 */
> +	int len = 0;
> +	const struct json_token *ptr = token;
> +	while (ptr != root && ptr->type != JSON_TOKEN_END) {
> +		len += json_token_snprint(NULL, 0, ptr, index_base);
> +		ptr = ptr->parent;
> +	}
> +	if (size == 0)
> +		return len;
> +
> +	/*
> +	 * Write to the memory buffer buf as many tokens,
> +	 * starting with the root, as it can fit. If the fragment
> +	 * of the token does not fit into the buffer, it would be
> +	 * truncated.
> +	 */
> +	int pos = len;
> +	char last = '\0';
> +	ptr = token;
> +	while (ptr != root && ptr->type != JSON_TOKEN_END) {
> +		pos -= json_token_snprint(NULL, 0, ptr, index_base);
> +		assert(pos >= 0);
> +		if (pos + 1 < size) {

Why pos + 1? Due to this extra 1, this function will crash if passed a
buffer of length 1.

> +			int rc = json_token_snprint(buf + pos, size - pos, ptr,
> +						    index_base);
> +			if (rc < 0)
> +				return rc;

How can it happen?

> +			/*
> +			 * The json_token_snprint writes a
> +			 * '\0'-terminated string in memory
> +			 * buffer. The first character in
> +			 * token string representation would be
> +			 * overwritten on next cycle iteration.
> +			 * Let's keep it in 'last' variable to
> +			 * restore next time.
> +			 */
> +			buf[pos + rc] = last;

rc can be greater than size-pos, in which case you'll write beyond the
supplied buffer.

> +			last = buf[pos];
> +		}
> +		ptr = ptr->parent;
> +	}
> +	assert(buf[MIN(len, size - 1)] == '\0');
> +	return len;
> +}
> +
>  #define MH_SOURCE 1
>  #define mh_name _json
>  #define mh_key_t struct json_token *
> diff --git a/src/lib/json/json.h b/src/lib/json/json.h
> index 7ce10ce2c..bed20536d 100644
> --- a/src/lib/json/json.h
> +++ b/src/lib/json/json.h
> @@ -253,6 +253,30 @@ json_path_cmp(const char *a, int a_len, const char *b, int b_len,
>  int
>  json_path_validate(const char *path, int path_len, int index_base);
>  
> +/**
> + * Write a JSON tree path path between root and token items to
> + * memory buffer buf, at most size bytes (including the null byte
> + * used to end output to strings).
> + * The parent argument may omitted via passing NULL value. It this

There's no 'parent' argument. You mean 'root'? But we always require the
caller to pass the root to all other JSON tree methods. Why make this
one different?

> + * case the whole JSON path would be printed.

> + * When routine is called with size=0, return value (as always)
> + * is the number of characters that would have been written in
> + * case the output string had been large enough.

This paragraph is pointless, because it directly follows from the next
paragraph.

> + *
> + * Upon successful return, routine returns the number of
> + * characters printed (excluding the null byte used to end output
> + * to strings). If the output was truncated due the size limit,
> + * then the return value is the number of characters (excluding
> + * the terminating null byte) which would have been written to
> + * the final string if enough space had been available.
> + * Thus, a return value of size or more means that the output
> + * was truncated.
> + * If an output error is encountered, a negative value is
> + * returned.

What error are you talking about? This function isn't supposed to fail.
Looks like you blindly copied snprintf() man page.

Instead you could simply write something like:

  An snprint-style function to print path to a token in a JSON tree.
  The root node doesn't necessarily have to be the tree root - it may be
  any ascendant of the given token, in which case a path from the given
  ascendant to the token will be printed.

Everyone who wants to know what snprintf() returns can read its manual
page.

> + */
> +int
> +json_tree_snprint_path(char *buf, int size, const struct json_token *token,
> +		       const struct json_token *root, int index_base);
>  /**
>   * Initialize a new empty JSON tree.
>   *
> diff --git a/test/unit/json.c b/test/unit/json.c
> index e553ff23c..122e605cf 100644
> --- a/test/unit/json.c
> +++ b/test/unit/json.c
> @@ -438,16 +438,81 @@ test_path_cmp()
>  	footer();
>  }
>  
> +void
> +test_path_snprintf()
> +{
> +	header();
> +	plan(24);
> +
> +	struct json_tree tree;
> +	int rc = json_tree_create(&tree);
> +	fail_if(rc != 0);
> +	struct test_struct records[5];
> +	const char *path = "[1][20][\"file\"][8]";
> +	int path_len = strlen(path);
> +
> +	int records_idx = 0;
> +	struct test_struct *node, *node_tmp;
> +	node = test_add_path(&tree, path, path_len, records, &records_idx);
> +	fail_if(&node->node != &records[3].node);
> +
> +	/* Test size estimation. */
> +	char buff[64];
> +	int len = json_tree_snprint_path(NULL, 0, &node->node, NULL, INDEX_BASE);
> +	is(len, path_len, "estimate path length: have %d expected %d",
> +	   len, path_len);
> +	len = json_tree_snprint_path(NULL, 0, &node->node, &records[0].node,
> +				     INDEX_BASE);
> +	is(len, 15, "estimate path part length: have %d expected %d", len, 15);
> +
> +	len = json_tree_snprint_path(NULL, 0, &node->node, &records[1].node,
> +				     INDEX_BASE);
> +	is(len, 11, "estimate path part length: have %d expected %d", len, 11);
> +	len = json_tree_snprint_path(NULL, 0, &node->node, &records[2].node,
> +				     INDEX_BASE);
> +	is(len, 3, "estimate path part length: have %d expected %d", len, 3);
> +
> +	/* Test truncate. */
> +	for (int i = path_len + 1; i > strrchr(path, '[') - path - 2; i--) {
> +		len = json_tree_snprint_path(buff, i + 1, &node->node, NULL,
> +					     INDEX_BASE);
> +		is(len, path_len, "correct char count: have %d expected %d",
> +		   len, path_len);
> +		len = MIN(len, i);
> +		is(memcmp(buff, path, len), 0,
> +		   "correct string fragment: have \"%s\", expected \"%.*s\"",
> +		   buff, len, path);
> +		is(buff[len], '\0', "terminating '\\0' at appropriate place");
> +	}

This is difficult for understanding. Why can't you simply choose a few
values of the buffer length and check that the function works for them
as expected? And I don't see any point special-casing NULL buffer here -
after all there's nothing special about it - it should execute the same
code as any other buffer length.

> +
> +	/* Test path fragment snprintf. */
> +	len = json_tree_snprint_path(buff, 16, &node->node, &records[0].node,
> +				     INDEX_BASE);
> +	is(memcmp(buff, path + 3, len), 0,
> +	   "correct partial JSON path: have \"%s\" expected \"%s\"", buff,
> +	   path + 3);
> +	is(buff[len], '\0', "terminating '\\0' at appropriate place");
> +
> +	json_tree_foreach_entry_safe(node, &tree.root, struct test_struct,
> +				     node, node_tmp)
> +		json_tree_del(&tree, &node->node);
> +	json_tree_destroy(&tree);
> +
> +	check_plan();
> +	footer();
> +}

Re: [PATCH v1 5/5] box: refactor tuple_init_field_map to use bitmap

[Vladimir Davydov]

On Sun, Dec 23, 2018 at 03:40:40PM +0300, Kirill Shcherbatov wrote:
> Refactored tuple_init_field_map to fill temporal bitmap and
> compare it with template req_fields_bitmap containing information
> about required fields. Each field is mapped to bitmap using
> unique field identifier.
> This approach to check the required fields will work even after
> the introduction of JSON paths, when the field tree becomes
> multilevel.
> 
> Needed for #1012
> ---
>  src/box/errcode.h                 |   2 +-
>  src/box/tuple_format.c            | 183 +++++++++++++++++++++++-------
>  src/box/tuple_format.h            |  23 ++++
>  test/box/alter_limits.result      |   8 +-
>  test/box/ddl.result               |  24 ++--
>  test/box/misc.result              |   2 +-
>  test/box/sql.result               |  12 +-
>  test/box/tree_pk_multipart.result |   8 +-
>  test/engine/ddl.result            |  28 ++---
>  test/engine/null.result           |  52 ++++-----
>  test/vinyl/constraint.result      |  12 +-
>  test/vinyl/errinj.result          |  12 +-
>  test/vinyl/savepoint.result       |   8 +-
>  13 files changed, 250 insertions(+), 124 deletions(-)
> 
> diff --git a/src/box/errcode.h b/src/box/errcode.h
> index 7d1f8ddc7..812e643d8 100644
> --- a/src/box/errcode.h
> +++ b/src/box/errcode.h
> @@ -91,7 +91,7 @@ struct errcode_record {
>  	/* 36 */_(ER_NO_SUCH_SPACE,		"Space '%s' does not exist") \
>  	/* 37 */_(ER_NO_SUCH_FIELD,		"Field %d was not found in the tuple") \
>  	/* 38 */_(ER_EXACT_FIELD_COUNT,		"Tuple field count %u does not match space field count %u") \
> -	/* 39 */_(ER_MIN_FIELD_COUNT,		"Tuple field count %u is less than required by space format or defined indexes (expected at least %u)") \
> +	/* 39 */_(ER_FIELD_STRUCTURE_MISMATCH,	"Tuple field %s does not math document structure defined by space format or index: %s") \
>  	/* 40 */_(ER_WAL_IO,			"Failed to write to disk") \
>  	/* 41 */_(ER_MORE_THAN_ONE_TUPLE,	"Get() doesn't support partial keys and non-unique indexes") \
>  	/* 42 */_(ER_ACCESS_DENIED,		"%s access to %s '%s' is denied for user '%s'") \
> diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
> index e29f84dc5..6e269fd77 100644
> --- a/src/box/tuple_format.c
> +++ b/src/box/tuple_format.c
> @@ -28,6 +28,8 @@
>   * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
>   * SUCH DAMAGE.
>   */
> +#include "bit/bit.h"
> +#include "fiber.h"
>  #include "json/json.h"
>  #include "tuple_format.h"
>  #include "coll_id_cache.h"
> @@ -206,6 +208,30 @@ tuple_format_create(struct tuple_format *format, struct key_def * const *keys,
>  		return -1;
>  	}
>  	format->field_map_size = field_map_size;
> +
> +	/* Allocate required fields bitmap. */
> +	uint32_t req_fields_bitmap_sz = (format->total_field_count +
> +					 CHAR_BIT * sizeof(unsigned long) - 1) /
> +					 CHAR_BIT * sizeof(unsigned long);

There's DIV_ROUND_UP helper for this. Anyway, I don't think this is
correct. Suppose there are 63 fields, then you will allocate just one
byte.

> +	format->req_fields_bitmap = calloc(1, req_fields_bitmap_sz);
> +	if (format->req_fields_bitmap == NULL) {
> +		diag_set(OutOfMemory, req_fields_bitmap_sz, "calloc",
> +			 "format->req_fields_bitmap");
> +		return -1;
> +	}
> +	struct tuple_field *field;
> +	struct json_token *root = (struct json_token *)&format->fields.root;

Please remove all pointless type conversions from this patch.

> +	json_tree_foreach_entry_preorder(field, root, struct tuple_field,
> +					 token) {
> +		/*
> +		 * Mark all leaf non-nullable fields as required
> +		 * setting corresponding bit 1 in format
> +		 * req_fields_bitmap.
> +		 */
> +		if (tuple_field_is_leaf(field) &&
> +		    !tuple_field_is_nullable(field))
> +			bit_set(format->req_fields_bitmap, field->id);
> +	}
>  	return 0;
>  }
>  
> @@ -304,6 +330,7 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
>  		struct tuple_field *field = tuple_field_new();
>  		if (field == NULL)
>  			goto error;
> +		field->id = fieldno;
>  		field->token.num = fieldno;
>  		field->token.type = JSON_TOKEN_NUM;
>  		if (json_tree_add(&format->fields, &format->fields.root,
> @@ -323,6 +350,8 @@ tuple_format_alloc(struct key_def * const *keys, uint16_t key_count,
>  		format->dict = dict;
>  		tuple_dictionary_ref(dict);
>  	}
> +	format->total_field_count = field_count;
> +	format->req_fields_bitmap = NULL;
>  	format->refs = 0;
>  	format->id = FORMAT_ID_NIL;
>  	format->index_field_count = index_field_count;
> @@ -339,6 +368,7 @@ error:
>  static inline void
>  tuple_format_destroy(struct tuple_format *format)
>  {
> +	free(format->req_fields_bitmap);
>  	tuple_format_destroy_fields(format);
>  	tuple_dictionary_unref(format->dict);
>  }
> @@ -422,6 +452,74 @@ tuple_format1_can_store_format2_tuples(struct tuple_format *format1,
>  	return true;
>  }
>  
> +/**
> + * Return meta information of a tuple field given a format
> + * and a unique field identifier.

The comment should also say why it's OK to implement this function in
such a suboptimal way.

> + */
> +static struct tuple_field *
> +tuple_format_field_by_id(const struct tuple_format *format, uint32_t id)
> +{
> +	struct tuple_field *field;
> +	struct json_token *root = (struct json_token *)&format->fields.root;
> +	json_tree_foreach_entry_preorder(field, root, struct tuple_field,
> +					 token) {
> +		if (field->id == id)
> +			return field;
> +	}
> +	return NULL;
> +}
> +
> +/**
> + * Analyze fields_bitmap to ensure that all required fields
> + * present in tuple. Routine relies on req_fields_bitmap is
> + * initialized on tuple_format_create and all required field's
> + * id bits are set 1.
> + */
> +static int
> +tuple_format_fields_bitmap_test(const struct tuple_format *format,
> +				const char *fields_bitmap)
> +{
> +	struct tuple_field *missed_field = NULL;
> +	const char *req_fields_bitmap = format->req_fields_bitmap;
> +	for (uint32_t i = 0; i < format->total_field_count; i++) {
> +		bool is_required = bit_test(req_fields_bitmap, i);
> +		if (is_required && is_required != bit_test(fields_bitmap, i)) {
> +			missed_field = tuple_format_field_by_id(format, i);
> +			assert(missed_field != NULL);
> +			break;
> +		}
> +	}

This path is hot. Testing bits one by one is too slow. You need to use
ffs or builtin. I think it's worth implementing it in the scope of the
bit library (bit_find or something).

> +	if (missed_field == NULL)
> +		return 0;
> +
> +	struct json_token *token = &missed_field->token;
> +	const char *err;
> +	if (missed_field->token.type == JSON_TOKEN_STR) {
> +		err = tt_sprintf("map does not contain a key \"%.*s\"",
> +				 token->len, token->str);
> +	} else if (missed_field->token.type == JSON_TOKEN_NUM) {
> +		struct json_token *parent = token->parent;
> +		/*
> +		 * Determine minimal field size looking for
> +		 * the greatest fieldno between non-nullable
> +		 * missed_field neighbors.
> +		 */
> +		uint32_t expected_size;
> +		for (int i = 0; i <= parent->max_child_idx; i++) {
> +			struct tuple_field *field =
> +				tuple_format_field((struct tuple_format *)format,
> +						   i);
> +			if (!tuple_field_is_nullable(field))
> +				expected_size = i + 1;
> +		}
> +		err = tt_sprintf("invalid array size %d (expected at least %d)",
> +				 token->num, expected_size);
> +	}
> +	diag_set(ClientError, ER_FIELD_STRUCTURE_MISMATCH,
> +		 tuple_field_path(missed_field), err);

Why not simply

  Field %s required by space format is missing

?

> +	return -1;
> +}
> +
>  /** @sa declaration for details. */
>  int
>  tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
> @@ -441,54 +539,59 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
>  			 (unsigned) format->exact_field_count);
>  		return -1;
>  	}
> -	if (validate && field_count < format->min_field_count) {
> -		diag_set(ClientError, ER_MIN_FIELD_COUNT,
> -			 (unsigned) field_count,
> -			 (unsigned) format->min_field_count);
> -		return -1;
> -	}
> -
> -	/* first field is simply accessible, so we do not store offset to it */
> -	const struct tuple_field *field =
> -		tuple_format_field((struct tuple_format *)format, 0);
> -	if (validate &&
> -	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
> -					 tuple_field_is_nullable(field))) {
> -		diag_set(ClientError, ER_FIELD_TYPE, tuple_field_path(field),
> -			 field_type_strs[field->type]);
> -		return -1;
> -	}
> -	mp_next(&pos);
> -	/* other fields...*/
> -	uint32_t i = 1;
>  	uint32_t defined_field_count = MIN(field_count, validate ?
>  					   tuple_format_field_count(format) :
>  					   format->index_field_count);
> -	if (field_count < format->index_field_count) {
> -		/*
> -		 * Nullify field map to be able to detect by 0,
> -		 * which key fields are absent in tuple_field().
> -		 */
> -		memset((char *)field_map - format->field_map_size, 0,
> -		       format->field_map_size);
> -	}
> -	for (; i < defined_field_count; ++i) {
> -		field = tuple_format_field((struct tuple_format *)format, i);
> -		if (validate &&
> -		    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
> -						 tuple_field_is_nullable(field))) {
> -			diag_set(ClientError, ER_FIELD_TYPE,
> -				 tuple_field_path(field),
> -				 field_type_strs[field->type]);

You don't need to change this code. Leave it as is, please.

> +	struct region *region = &fiber()->gc;
> +	char *fields_bitmap = NULL;
> +	if (validate) {
> +		uint32_t fields_bitmap_sz = (format->total_field_count +
> +					     CHAR_BIT *
> +					     sizeof(unsigned long) - 1) /
> +					     CHAR_BIT * sizeof(unsigned long);
> +		fields_bitmap = region_alloc(region, fields_bitmap_sz);
> +		if (fields_bitmap == NULL) {
> +			diag_set(OutOfMemory, fields_bitmap_sz, "calloc",
> +				"fields_bitmap");
>  			return -1;
>  		}
> -		if (field->offset_slot != TUPLE_OFFSET_SLOT_NIL) {
> -			field_map[field->offset_slot] =
> -				(uint32_t) (pos - tuple);
> +		memset(fields_bitmap, 0, fields_bitmap_sz);
> +	}
> +	memset((char *)field_map - format->field_map_size, 0,
> +	       format->field_map_size);
> +	struct json_tree *tree = (struct json_tree *)&format->fields;
> +	struct json_token *parent = &tree->root;
> +	struct json_token token;
> +	token.parent = NULL;
> +	token.type = JSON_TOKEN_NUM;
> +	token.num = 0;
> +	while ((uint32_t)token.num < defined_field_count) {
> +		struct tuple_field *field =
> +			json_tree_lookup_entry(tree, parent, &token,
> +					       struct tuple_field, token);
> +		if (field != NULL) {
> +			bool is_nullable = tuple_field_is_nullable(field);
> +			if (validate &&
> +			    !field_mp_type_is_compatible(field->type,
> +							 mp_typeof(*pos),
> +							 is_nullable) != 0) {
> +				diag_set(ClientError, ER_FIELD_TYPE,
> +					 tuple_field_path(field),
> +					 field_type_strs[field->type]);
> +				return -1;
> +			}
> +			if (field->offset_slot != TUPLE_OFFSET_SLOT_NIL) {
> +				field_map[field->offset_slot] =
> +					(uint32_t)(pos - tuple);
> +			}
> +			if (validate)
> +				bit_set(fields_bitmap, field->id);
>  		}
> +		token.num++;
>  		mp_next(&pos);
>  	}
> -	return 0;
> +	return validate ?
> +	       tuple_format_fields_bitmap_test(format, fields_bitmap) : 0;
>  }
>  
>  uint32_t
> diff --git a/src/box/tuple_format.h b/src/box/tuple_format.h
> index 949337807..947d0d8a5 100644
> --- a/src/box/tuple_format.h
> +++ b/src/box/tuple_format.h
> @@ -31,6 +31,7 @@
>   * SUCH DAMAGE.
>   */
>  
> +#include "bit/bit.h"
>  #include "key_def.h"
>  #include "field_def.h"
>  #include "errinj.h"
> @@ -114,6 +115,8 @@ struct tuple_field {
>  	struct coll *coll;
>  	/** Collation identifier. */
>  	uint32_t coll_id;
> +	/** Field unique identifier in tuple_format. */
> +	uint32_t id;
>  	/** Link in tuple_format::fields. */
>  	struct json_token token;
>  };
> @@ -173,6 +176,20 @@ struct tuple_format {
>  	 * Shared names storage used by all formats of a space.
>  	 */
>  	struct tuple_dictionary *dict;
> +	/**
> +	 * This bitmap of "required fields" contains information
> +	 * about fields that must present in tuple to be inserted.
> +	 * Look for tuple_format_fields_bitmap_test comment for
> +	 * more details.

And how fields are mapped to bits?

> +	 */
> +	char *req_fields_bitmap;

Why not call it simply required_fields?

> +	/**
> +	 * Total count of format fields in fields subtree.
> +	 * Required to allocate temporal objects containing

temporal != temporary. Far from it. I guess I'll just have to rewrite
all your comments, again...

> +	 * attributes for all fields. Particularly to allocate
> +	 * temporal req_fields_bitmap on region.
> +	 */
> +	uint32_t total_field_count;
>  	/**
>  	 * Fields comprising the format, organized in a tree.
>  	 * First level nodes correspond to tuple fields.
> @@ -194,6 +211,12 @@ tuple_format_field_count(const struct tuple_format *format)
>  	return root->children != NULL ? root->max_child_idx + 1 : 0;
>  }
>  
> +static inline bool
> +tuple_field_is_leaf(struct tuple_field *field)
> +{
> +	return field->token.max_child_idx == -1;

This check should live in json.h.