[PATCH v6 3/8] box: build path to field string uniformly on error

Kirill Shcherbatov kshcherbatov at tarantool.org
Mon Dec 17 09:52:47 MSK 2018


Introduced new tuple_field_json_path routine that constructs
path to field string containing fieldno or field name when
space format is set.
In subsequent patches it will be improved to support complex
JSON paths.

Needed for #1012
---
 src/box/tuple_format.c     | 50 +++++++++++++++++++++------------
 test/box/alter.result      |  3 +-
 test/box/blackhole.result  |  6 ++--
 test/box/ddl.result        |  6 ++--
 test/box/rtree_misc.result |  3 +-
 test/engine/ddl.result     | 57 +++++++++++++++++++++++---------------
 test/engine/null.result    | 24 ++++++++++------
 test/engine/update.result  |  6 ++--
 test/vinyl/errinj.result   |  3 +-
 9 files changed, 100 insertions(+), 58 deletions(-)

diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 903232c66..8338bba44 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -61,9 +61,28 @@ tuple_field_delete(struct tuple_field *field)
 	free(field);
 }
 
+/** Build the JSON path by field specified. */
+static const char *
+tuple_field_json_path(const struct tuple_format *format,
+		      struct tuple_field *field)
+{
+	struct json_token *token = &field->token;
+	const char *path;
+	if (token->parent == &format->fields.root &&
+		token->num < (int)format->dict->name_count) {
+		const char *field_name =
+			format->dict->names[token->num];
+		path = tt_sprintf("\"%s\"", field_name);
+	} else if (token->type == JSON_TOKEN_NUM) {
+		path = tt_sprintf("%u", token->num + TUPLE_INDEX_BASE);
+	} else {
+		unreachable();
+	}
+	return path;
+}
+
 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)
 {
@@ -93,8 +112,9 @@ tuple_format_use_key_part(struct tuple_format *format,
 		if (field->nullable_action == ON_CONFLICT_ACTION_NONE)
 			field->nullable_action = part->nullable_action;
 	} else if (field->nullable_action != part->nullable_action) {
-		diag_set(ClientError, ER_ACTION_MISMATCH,
-			 tt_sprintf("%u", part->fieldno + TUPLE_INDEX_BASE),
+		const char *path = tuple_field_json_path(format, field);
+		assert(path != NULL);
+		diag_set(ClientError, ER_ACTION_MISMATCH, path,
 			 on_conflict_action_strs[field->nullable_action],
 			 on_conflict_action_strs[part->nullable_action]);
 		return -1;
@@ -111,21 +131,14 @@ 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,
+		const char *path = tuple_field_json_path(format, field);
+		assert(path != NULL);
+		diag_set(ClientError, errcode, path,
 			 field_type_strs[field->type],
 			 field_type_strs[part->type]);
 		return -1;
@@ -190,8 +203,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;
@@ -547,8 +559,10 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 			if (validate &&
 			    !mp_type_is_compatible(type, field->type,
 						   is_nullable) != 0) {
-				diag_set(ClientError, ER_FIELD_TYPE,
-					 tt_sprintf("%u", token.num + 1),
+				const char *path =
+					tuple_field_json_path(format, field);
+				assert(path != NULL);
+				diag_set(ClientError, ER_FIELD_TYPE, path,
 					 field_type_strs[field->type]);
 				return -1;
 			}
diff --git a/test/box/alter.result b/test/box/alter.result
index 9a1086e0c..40c8f921a 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 "id" 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
diff --git a/test/box/blackhole.result b/test/box/blackhole.result
index 945b2755c..25c2eedfb 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 "value" 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 "key" 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..75155ed3f 100644
--- a/test/box/ddl.result
+++ b/test/box/ddl.result
@@ -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 "opts" 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 "opts" 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/rtree_misc.result b/test/box/rtree_misc.result
index 36d5b8f55..1b88203b9 100644
--- a/test/box/rtree_misc.result
+++ b/test/box/rtree_misc.result
@@ -554,7 +554,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 "opts" 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/engine/ddl.result b/test/engine/ddl.result
index 3c84e942d..219faa9dc 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -707,7 +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
+- error: Field "field2" has type 'string' in space format, but type 'unsigned' in
     index definition
 ...
 -- Check space format conflicting with index parts.
@@ -719,7 +719,7 @@ format[2].type = 'unsigned'
 ...
 s:format(format)
 ---
-- error: Field 'field2' has type 'unsigned' in space format, but type 'string' in
+- error: Field "field2" has type 'unsigned' in space format, but type 'string' in
     index definition
 ...
 s:format()
@@ -822,27 +822,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 "field2" 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 "field3" 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 "field4" 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 "field5" 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 "field6" 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 "field7" type does not match one required by operation: expected
+    map'
 ...
 s:replace{1, '2', {3, 3}, 4.4, -5, true, {value=7}}
 ---
@@ -1137,7 +1143,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 "field2" type does not match one required by operation: expected unsigned'
 ...
 -- unsigned -----> any
 ok_format_change(3, 'any')
@@ -1146,7 +1152,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 "field3" type does not match one required by operation: expected string'
 ...
 -- unsigned -----> number
 ok_format_change(3, 'number')
@@ -1163,7 +1169,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 "field3" type does not match one required by operation: expected map'
 ...
 -- string -----> any
 ok_format_change(4, 'any')
@@ -1176,7 +1182,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 "field4" type does not match one required by operation: expected boolean'
 ...
 -- number -----> any
 ok_format_change(5, 'any')
@@ -1189,7 +1195,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 "field5" type does not match one required by operation: expected integer'
 ...
 -- integer -----> any
 ok_format_change(6, 'any')
@@ -1206,7 +1212,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 "field6" type does not match one required by operation: expected unsigned'
 ...
 -- boolean -----> any
 ok_format_change(7, 'any')
@@ -1219,7 +1225,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 "field7" type does not match one required by operation: expected string'
 ...
 -- scalar -----> any
 ok_format_change(8, 'any')
@@ -1228,7 +1234,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 "field8" type does not match one required by operation: expected unsigned'
 ...
 -- array -----> any
 ok_format_change(9, 'any')
@@ -1237,7 +1243,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 "field9" type does not match one required by operation: expected scalar'
 ...
 -- map -----> any
 ok_format_change(10, 'any')
@@ -1246,7 +1252,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 "field10" type does not match one required by operation: expected scalar'
 ...
 s:drop()
 ---
@@ -1478,7 +1484,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 "field2" type does not match one required by operation: expected
+    unsigned'
 ...
 _ = s:delete(1)
 ---
@@ -1761,11 +1768,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 "field2" 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 "field3" type does not match one required by operation: expected
+    integer'
 ...
 s:replace{1, 100, -20}
 ---
@@ -1829,11 +1838,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 "field2" 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 "field3" type does not match one required by operation: expected
+    integer'
 ...
 s:replace{5, 5, 5}
 ---
diff --git a/test/engine/null.result b/test/engine/null.result
index 757e63185..ca05fccf9 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 "field2" type does not match one required by operation: expected
+    unsigned'
 ...
 format[2].is_nullable = true
 ---
@@ -1774,7 +1775,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 "second" 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 +1784,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 "second" type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5}
 ---
@@ -1801,7 +1804,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 "second" type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5}
 ---
@@ -1838,11 +1842,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 "second" 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 "second" type does not match one required by operation: expected
+    unsigned'
 ...
 _ = s:delete{5}
 ---
@@ -1869,7 +1875,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 "second" type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5} -- Fail.
 ---
@@ -1887,7 +1894,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 "second" type does not match one required by operation: expected
+    unsigned'
 ...
 s:insert{5} -- Fail.
 ---
diff --git a/test/engine/update.result b/test/engine/update.result
index b73037ebc..138859ac5 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 "VAL" 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 "VAL" type does not match one required by operation: expected
+    string'
 ...
 s:drop()
 ---
diff --git a/test/vinyl/errinj.result b/test/vinyl/errinj.result
index a081575be..1a68db015 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 "field2" type does not match one required by operation: expected
+    string'
 ...
 errinj.set("ERRINJ_VY_READ_PAGE_TIMEOUT", 0)
 ---
-- 
2.19.2




More information about the Tarantool-patches mailing list