[PATCH v1 2/5] box: refactor field type and nullability checks

Kirill Shcherbatov kshcherbatov at tarantool.org
Sun Dec 23 15:40:37 MSK 2018


Reworked field types and nullability checks to set error message
in tuple_init_field_map manually. We need to specify full JSON
path to field sting in further patches.
- introduced field_mp_type_is_compatible routine making
  field_type and mp_type compatibility test taking into account
  field nullability
- refactored key_part_validate to pass const char * key argument
  and to reuse field_mp_type_is_compatible code

Needed for #1012
---
 src/box/index.cc       | 22 +++++++---------------
 src/box/key_def.c      | 12 +++++-------
 src/box/key_def.h      | 30 ++++++++++++++++++++----------
 src/box/tuple_format.c | 18 +++++++++++-------
 4 files changed, 43 insertions(+), 39 deletions(-)

diff --git a/src/box/index.cc b/src/box/index.cc
index cf81eca49..1dbf364a8 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -89,9 +89,7 @@ key_validate(const struct index_def *index_def, enum iterator_type type,
 			return -1;
 		}
 		if (part_count == 1) {
-			enum mp_type mp_type = mp_typeof(*key);
-			if (key_mp_type_validate(FIELD_TYPE_ARRAY, mp_type,
-						 ER_KEY_PART_TYPE, 0, false))
+			if (key_part_validate(FIELD_TYPE_ARRAY, key, 0, false))
 				return -1;
 			uint32_t array_size = mp_decode_array(&key);
 			if (array_size != d && array_size != d * 2) {
@@ -100,23 +98,17 @@ key_validate(const struct index_def *index_def, enum iterator_type type,
 				return -1;
 			}
 			for (uint32_t part = 0; part < array_size; part++) {
-				enum mp_type mp_type = mp_typeof(*key);
-				mp_next(&key);
-				if (key_mp_type_validate(FIELD_TYPE_NUMBER,
-							 mp_type,
-							 ER_KEY_PART_TYPE, 0,
-							 false))
+				if (key_part_validate(FIELD_TYPE_NUMBER, key,
+						      0, false))
 					return -1;
+				mp_next(&key);
 			}
 		} else {
 			for (uint32_t part = 0; part < part_count; part++) {
-				enum mp_type mp_type = mp_typeof(*key);
-				mp_next(&key);
-				if (key_mp_type_validate(FIELD_TYPE_NUMBER,
-							 mp_type,
-							 ER_KEY_PART_TYPE,
-							 part, false))
+				if (key_part_validate(FIELD_TYPE_NUMBER, key,
+						      part, false))
 					return -1;
+				mp_next(&key);
 			}
 		}
 	} else {
diff --git a/src/box/key_def.c b/src/box/key_def.c
index 2119ca389..4cc06de47 100644
--- a/src/box/key_def.c
+++ b/src/box/key_def.c
@@ -88,7 +88,7 @@ const char *mp_type_strs[] = {
 	/* .MP_EXT    = */ "extension",
 };
 
-const uint32_t key_mp_type[] = {
+const uint32_t field_mp_type[] = {
 	/* [FIELD_TYPE_ANY]      =  */ UINT32_MAX,
 	/* [FIELD_TYPE_UNSIGNED] =  */ 1U << MP_UINT,
 	/* [FIELD_TYPE_STRING]   =  */ 1U << MP_STR,
@@ -608,14 +608,12 @@ key_validate_parts(const struct key_def *key_def, const char *key,
 		   uint32_t part_count, bool allow_nullable)
 {
 	for (uint32_t i = 0; i < part_count; i++) {
-		enum mp_type mp_type = mp_typeof(*key);
 		const struct key_part *part = &key_def->parts[i];
-		mp_next(&key);
-
-		if (key_mp_type_validate(part->type, mp_type, ER_KEY_PART_TYPE,
-					 i, key_part_is_nullable(part)
-					 && allow_nullable))
+		if (key_part_validate(part->type, key, i,
+				      key_part_is_nullable(part) &&
+				      allow_nullable))
 			return -1;
+		mp_next(&key);
 	}
 	return 0;
 }
diff --git a/src/box/key_def.h b/src/box/key_def.h
index d4da6c5a1..2d9a3a7c2 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -392,27 +392,37 @@ key_def_has_collation(const struct key_def *key_def)
 	return false;
 }
 
-/** A helper table for key_mp_type_validate */
-extern const uint32_t key_mp_type[];
+/** A helper table for field_mp_type_is_compatible */
+extern const uint32_t field_mp_type[];
+
+/** Checks if mp_type (MsgPack) is compatible with field type. */
+static inline bool
+field_mp_type_is_compatible(enum field_type type, enum mp_type mp_type,
+			    bool is_nullable)
+{
+	assert(type < field_type_MAX);
+	assert((size_t)mp_type < CHAR_BIT * sizeof(*field_mp_type));
+	uint32_t mask = field_mp_type[type] | (is_nullable * (1U << MP_NIL));
+	return (mask & (1U << mp_type)) != 0;
+}
 
 /**
  * @brief Checks if \a field_type (MsgPack) is compatible \a type (KeyDef).
  * @param type KeyDef type
- * @param field_type MsgPack type
+ * @param key Pointer to MsgPack field to be tested.
  * @param field_no - a field number (is used to store an error message)
  *
  * @retval 0  mp_type is valid.
  * @retval -1 mp_type is invalid.
  */
 static inline int
-key_mp_type_validate(enum field_type key_type, enum mp_type mp_type,
-		     int err, uint32_t field_no, bool is_nullable)
+key_part_validate(enum field_type key_type, const char *key,
+		  uint32_t field_no, bool is_nullable)
 {
-	assert(key_type < field_type_MAX);
-	assert((size_t) mp_type < CHAR_BIT * sizeof(*key_mp_type));
-	uint32_t mask = key_mp_type[key_type] | (is_nullable * (1U << MP_NIL));
-	if (unlikely((mask & (1U << mp_type)) == 0)) {
-		diag_set(ClientError, err, field_no, field_type_strs[key_type]);
+	if (unlikely(!field_mp_type_is_compatible(key_type, mp_typeof(*key),
+						  is_nullable))) {
+		diag_set(ClientError, ER_KEY_PART_TYPE, field_no,
+			 field_type_strs[key_type]);
 		return -1;
 	}
 	return 0;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 3af39a37e..04343fd53 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -451,13 +451,15 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 	}
 
 	/* first field is simply accessible, so we do not store offset to it */
-	enum mp_type mp_type = mp_typeof(*pos);
 	const struct tuple_field *field =
 		tuple_format_field((struct tuple_format *)format, 0);
 	if (validate &&
-	    key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE,
-				 TUPLE_INDEX_BASE, tuple_field_is_nullable(field)))
+	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
+					 tuple_field_is_nullable(field))) {
+		diag_set(ClientError, ER_FIELD_TYPE, TUPLE_INDEX_BASE,
+			 field_type_strs[field->type]);
 		return -1;
+	}
 	mp_next(&pos);
 	/* other fields...*/
 	uint32_t i = 1;
@@ -474,12 +476,14 @@ tuple_init_field_map(const struct tuple_format *format, uint32_t *field_map,
 	}
 	for (; i < defined_field_count; ++i) {
 		field = tuple_format_field((struct tuple_format *)format, i);
-		mp_type = mp_typeof(*pos);
 		if (validate &&
-		    key_mp_type_validate(field->type, mp_type, ER_FIELD_TYPE,
-					 i + TUPLE_INDEX_BASE,
-					 tuple_field_is_nullable(field)))
+		    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
+						 tuple_field_is_nullable(field))) {
+			diag_set(ClientError, ER_FIELD_TYPE,
+				 i + TUPLE_INDEX_BASE,
+				 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




More information about the Tarantool-patches mailing list