[tarantool-patches] Re: [PATCH v1 4/5] box: refactor ER_{FIELD_TYPE, ACTION_MISMATCH}

Kirill Shcherbatov kshcherbatov at tarantool.org
Tue Dec 25 11:53:22 MSK 2018


Reworked ER_FIELD_TYPE and ER_ACTION_MISMATCH error to pass path
string to field 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 | 37 ++++++++++++++++++-------------------
 test/engine/ddl.result |  6 ++----
 5 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index 9d3e89f85..94381f9f7 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..0f5e0ac53 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,
+			 int2str(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..83e5b7013 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]);
+			 int2str(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,
+			 int2str(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]);
+			 int2str(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,
+			 int2str(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,
+			 int2str(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 1a21444ca..e66a5f521 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -61,9 +61,18 @@ tuple_field_delete(struct tuple_field *field)
 	free(field);
 }
 
+/** Format a field into path string using a static buffer. */
+static const char *
+tuple_field_path(const struct tuple_field *field)
+{
+	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);
+}
+
 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)
 {
@@ -94,9 +103,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]);
+			 tuple_field_path(field),
+			 on_conflict_action_strs[field->nullable_action],
+			 on_conflict_action_strs[part->nullable_action]);
 		return -1;
 	}
 
@@ -111,21 +120,12 @@ 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,
+		diag_set(ClientError, errcode, tuple_field_path(field),
 			 field_type_strs[field->type],
 			 field_type_strs[part->type]);
 		return -1;
@@ -190,8 +190,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;
@@ -453,7 +452,7 @@ tuple_init_field_map(struct tuple_format *format, uint32_t *field_map,
 	if (validate &&
 	    !field_mp_type_is_compatible(field->type, mp_typeof(*pos),
 					 tuple_field_is_nullable(field))) {
-		diag_set(ClientError, ER_FIELD_TYPE, TUPLE_INDEX_BASE,
+		diag_set(ClientError, ER_FIELD_TYPE, tuple_field_path(field),
 			 field_type_strs[field->type]);
 		return -1;
 	}
@@ -477,7 +476,7 @@ tuple_init_field_map(struct tuple_format *format, uint32_t *field_map,
 		    !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,
+				 tuple_field_path(field),
 				 field_type_strs[field->type]);
 			return -1;
 		}
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index e3cff7f45..272ff7618 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -707,8 +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
-    index definition
+- error: Field 2 has type 'string' in space format, but type 'unsigned' in index definition
 ...
 -- Check space format conflicting with index parts.
 sk3 = s:create_index('sk3', { parts = { 2, 'string' } })
@@ -719,8 +718,7 @@ format[2].type = 'unsigned'
 ...
 s:format(format)
 ---
-- error: Field 'field2' has type 'unsigned' in space format, but type 'string' in
-    index definition
+- error: Field 2 has type 'unsigned' in space format, but type 'string' in index definition
 ...
 s:format()
 ---
-- 
2.19.2





More information about the Tarantool-patches mailing list