From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Kirill Shcherbatov Subject: [PATCH v6 3/8] box: build path to field string uniformly on error Date: Mon, 17 Dec 2018 09:52:47 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit To: tarantool-patches@freelists.org, vdavydov.dev@gmail.com Cc: kostja@tarantool.org, Kirill Shcherbatov List-ID: 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, ¤t_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