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 2/8] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}
Date: Mon, 17 Dec 2018 09:52:46 +0300	[thread overview]
Message-ID: <4d35379b0e230811688a41018f23644dac4c7212.1544995259.git.kshcherbatov@tarantool.org> (raw)
In-Reply-To: <cover.1544995259.git.kshcherbatov@tarantool.org>

Reworked ER_FIELD_TYPE and ER_ACTION_MISMATCH error to pass
string instead of field number.

Refactored tuple_init_field_map to set this ER_FIELD_TYPE
directly when new routine mp_type_is_compatible test is failed.
This code is hot, so preparing fieldno string for modified
key_mp_type_validate each time were not acceptable solution.

This patch is required for further JSON patches, to give detailed
information about field for ER_FIELD_TYPE error type.

Needed for #1012
---
 src/box/errcode.h      |  4 ++--
 src/box/memtx_rtree.c  |  2 +-
 src/box/tuple.h        | 15 +++++++++------
 src/box/tuple_format.c | 26 ++++++++++++++++++++------
 4 files changed, 32 insertions(+), 15 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 1ed3656e4..903232c66 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -94,9 +94,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]);
+			 tt_sprintf("%u", part->fieldno + TUPLE_INDEX_BASE),
+			 on_conflict_action_strs[field->nullable_action],
+			 on_conflict_action_strs[part->nullable_action]);
 		return -1;
 	}
 
@@ -481,6 +481,17 @@ tuple_field_map_validate(const struct tuple_format *format, uint32_t *field_map)
 	return 0;
 }
 
+/** Checks if mp_type (MsgPack) is compatible with field type. */
+static inline bool
+mp_type_is_compatible(enum mp_type mp_type, enum field_type type,
+		      bool is_nullable)
+{
+	assert(type < field_type_MAX);
+	assert((size_t) mp_type < CHAR_BIT * sizeof(*key_mp_type));
+	uint32_t mask = key_mp_type[type] | (is_nullable * (1U << MP_NIL));
+	return (mask & (1U << mp_type)) != 0;
+}
+
 /** @sa declaration for details. */
 int
 tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
@@ -534,10 +545,13 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 		if (field != NULL) {
 			bool is_nullable = tuple_field_is_nullable(field);
 			if (validate &&
-			    key_mp_type_validate(field->type, type,
-						 ER_FIELD_TYPE, token.num + 1,
-						 is_nullable) != 0)
+			    !mp_type_is_compatible(type, field->type,
+						   is_nullable) != 0) {
+				diag_set(ClientError, ER_FIELD_TYPE,
+					 tt_sprintf("%u", token.num + 1),
+					 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

  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 ` Kirill Shcherbatov [this message]
2018-12-17  6:52 ` [PATCH v6 3/8] box: build path to field string uniformly on error Kirill Shcherbatov
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=4d35379b0e230811688a41018f23644dac4c7212.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 2/8] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}' \
    /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