From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 24 Dec 2018 22:36:56 +0300 From: Vladimir Davydov Subject: Re: [PATCH v1 4/5] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH} Message-ID: <20181224193655.txnce6qnoqfr2fcw@esperanza> References: <0a1994b6576ea4779abb9e0dca85a494df2584a4.1545567929.git.kshcherbatov@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <0a1994b6576ea4779abb9e0dca85a494df2584a4.1545567929.git.kshcherbatov@tarantool.org> To: Kirill Shcherbatov Cc: tarantool-patches@freelists.org, kostja@tarantool.org List-ID: 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; > +} > +