Tarantool development patches archive
 help / color / mirror / Atom feed
From: Kirill Shcherbatov <kshcherbatov@tarantool.org>
To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com
Cc: kostja@tarantool.org, Kirill Shcherbatov <kshcherbatov@tarantool.org>
Subject: [PATCH v6 3/8] box: build path to field string uniformly on error
Date: Mon, 17 Dec 2018 09:52:47 +0300	[thread overview]
Message-ID: <a5b95c1ac98be7364fe0f0052e72b1c24880fe30.1544995259.git.kshcherbatov@tarantool.org> (raw)
In-Reply-To: <cover.1544995259.git.kshcherbatov@tarantool.org>

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

  parent reply	other threads:[~2018-12-17  6:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-17  6:52 [PATCH v6 0/8] box: Indexes by JSON path Kirill Shcherbatov
2018-12-17  6:52 ` [PATCH v6 1/8] box: refactor tuple_validate_raw Kirill Shcherbatov
2018-12-17  6:52 ` [PATCH v6 2/8] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH} Kirill Shcherbatov
2018-12-17  6:52 ` Kirill Shcherbatov [this message]
2018-12-17  6:52 ` [PATCH v6 4/8] box: introduce JSON Indexes Kirill Shcherbatov
2018-12-17  6:52 ` [PATCH v6 5/8] box: introduce has_json_paths flag in templates Kirill Shcherbatov
2018-12-27 11:51   ` [tarantool-patches] " Konstantin Osipov
2018-12-27 11:52     ` Konstantin Osipov
2018-12-27 11:57       ` [tarantool-patches] " Konstantin Osipov
2018-12-17  6:52 ` [PATCH v6 6/8] box: tune tuple_field_raw_by_path for indexed data Kirill Shcherbatov
2018-12-17  6:52 ` [PATCH v6 7/8] box: introduce offset_slot cache in key_part Kirill Shcherbatov
2018-12-17  6:52 ` [PATCH v6 8/8] box: specify indexes in user-friendly form Kirill Shcherbatov
2018-12-18 20:58   ` Vladimir Davydov
2018-12-18 20:46 ` [PATCH v6 0/8] box: Indexes by JSON path Vladimir Davydov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5b95c1ac98be7364fe0f0052e72b1c24880fe30.1544995259.git.kshcherbatov@tarantool.org \
    --to=kshcherbatov@tarantool.org \
    --cc=kostja@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH v6 3/8] box: build path to field string uniformly on error' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox