[Tarantool-patches] [PATCH v2] tuple: fix non-informative update() error message

Chris Sosnin k.sosnin at tarantool.org
Wed Nov 27 11:53:25 MSK 2019


Thank you for the review!
See second version below.
branch: https://github.com/tarantool/tarantool/tree/ksosnin/gh-3884-update-error
issue: https://github.com/tarantool/tarantool/issues/3884

Calling tuple_object:update() with invalid argument number
yields 'Unknown UPDATE operation' error. Instead, we replace this
error with explicit "wrong argument number", mentioning which operation
failed. Also add checking for correct opcode.

Fixes #3884
---
 src/box/errcode.h           |  2 +-
 src/box/xrow_update.c       |  4 ++--
 src/box/xrow_update_field.c | 23 ++++++++++++++++-------
 src/box/xrow_update_field.h |  2 +-
 test/box/update.result      |  6 ++++--
 test/engine/upsert.result   | 13 ++++++++-----
 test/vinyl/gh.result        |  3 ++-
 7 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/box/errcode.h b/src/box/errcode.h
index c660b1c70..97603259b 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -80,7 +80,7 @@ struct errcode_record {
 	/* 25 */_(ER_UPDATE_SPLICE,		"SPLICE error on field %s: %s") \
 	/* 26 */_(ER_UPDATE_ARG_TYPE,		"Argument type in operation '%c' on field %s does not match field type: expected %s") \
 	/* 27 */_(ER_FORMAT_MISMATCH_INDEX_PART, "Field %s has type '%s' in space format, but type '%s' in index definition") \
-	/* 28 */_(ER_UNKNOWN_UPDATE_OP,		"Unknown UPDATE operation") \
+	/* 28 */_(ER_UNKNOWN_UPDATE_OP,		"Unknown UPDATE operation #%d: %s") \
 	/* 29 */_(ER_UPDATE_FIELD,		"Field %s UPDATE error: %s") \
 	/* 30 */_(ER_FUNCTION_TX_ACTIVE,	"Transaction is active at return from function") \
 	/* 31 */_(ER_KEY_PART_COUNT,		"Invalid key part count (expected [0..%u], got %u)") \
diff --git a/src/box/xrow_update.c b/src/box/xrow_update.c
index 123db081a..db215aada 100644
--- a/src/box/xrow_update.c
+++ b/src/box/xrow_update.c
@@ -172,8 +172,8 @@ xrow_update_read_ops(struct xrow_update *update, const char *expr,
 	}
 	struct xrow_update_op *op = update->ops;
 	struct xrow_update_op *ops_end = op + update->op_count;
-	for (; op < ops_end; op++) {
-		if (xrow_update_op_decode(op, update->index_base, dict,
+	for (int i = 1; op < ops_end; op++, i++) {
+		if (xrow_update_op_decode(op, i, update->index_base, dict,
 					  &expr) != 0)
 			return -1;
 		/*
diff --git a/src/box/xrow_update_field.c b/src/box/xrow_update_field.c
index c694e17e9..47c263945 100644
--- a/src/box/xrow_update_field.c
+++ b/src/box/xrow_update_field.c
@@ -576,9 +576,14 @@ static const struct xrow_update_op_meta op_delete = {
 };
 
 static inline const struct xrow_update_op_meta *
-xrow_update_op_by(char opcode)
+xrow_update_op_by(const char *opcode, uint32_t len, int op_num)
 {
-	switch (opcode) {
+	if (len != 1) {
+		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP, op_num,
+			 tt_sprintf("\"%.*s\"", len, opcode));
+		return NULL;
+	}
+	switch (*opcode) {
 	case '=':
 		return &op_set;
 	case '+':
@@ -595,13 +600,14 @@ xrow_update_op_by(char opcode)
 	case '!':
 		return &op_insert;
 	default:
-		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP);
+		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP, op_num,
+			 tt_sprintf("\"%.*s\"", len, opcode));
 		return NULL;
 	}
 }
 
 int
-xrow_update_op_decode(struct xrow_update_op *op, int index_base,
+xrow_update_op_decode(struct xrow_update_op *op, int op_num, int index_base,
 		      struct tuple_dictionary *dict, const char **expr)
 {
 	if (mp_typeof(**expr) != MP_ARRAY) {
@@ -620,12 +626,15 @@ xrow_update_op_decode(struct xrow_update_op *op, int index_base,
 			 "update operation name must be a string");
 		return -1;
 	}
-	op->opcode = *mp_decode_str(expr, &len);
-	op->meta = xrow_update_op_by(op->opcode);
+	const char *opcode = mp_decode_str(expr, &len);
+	op->opcode = *opcode;
+	op->meta = xrow_update_op_by(opcode, len, op_num);
 	if (op->meta == NULL)
 		return -1;
 	if (arg_count != op->meta->arg_count) {
-		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP);
+		const char *str = tt_sprintf("wrong number of arguments, "\
+			 "expected %u, got %u", op->meta->arg_count, arg_count);
+		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP, op_num, str);
 		return -1;
 	}
 	int32_t field_no = 0;
diff --git a/src/box/xrow_update_field.h b/src/box/xrow_update_field.h
index e90095b9e..23a4ae517 100644
--- a/src/box/xrow_update_field.h
+++ b/src/box/xrow_update_field.h
@@ -197,7 +197,7 @@ struct xrow_update_op {
  * @retval -1 Client error.
  */
 int
-xrow_update_op_decode(struct xrow_update_op *op, int index_base,
+xrow_update_op_decode(struct xrow_update_op *op, int op_num,  int index_base,
 		      struct tuple_dictionary *dict, const char **expr);
 
 /* }}} xrow_update_op */
diff --git a/test/box/update.result b/test/box/update.result
index c6a5a25a7..317a51aca 100644
--- a/test/box/update.result
+++ b/test/box/update.result
@@ -826,11 +826,13 @@ s:update({0}, {{}})
 ...
 s:update({0}, {{'+'}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
+    1'
 ...
 s:update({0}, {{'+', 0}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
+    2'
 ...
 s:update({0}, {{'+', '+', '+'}})
 ---
diff --git a/test/engine/upsert.result b/test/engine/upsert.result
index 47da307fa..5610c675d 100644
--- a/test/engine/upsert.result
+++ b/test/engine/upsert.result
@@ -1809,7 +1809,7 @@ space:upsert({1}, {{'!', 2, 100}}) -- must fail on checking tuple
 ...
 space:upsert({'a'}, {{'a', 2, 100}}) -- must fail on checking ops
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: "a"'
 ...
 space:upsert({'a'}, {{'!', 2, 'ups1'}}) -- 'fast' upsert via insert in one index
 ---
@@ -2117,19 +2117,22 @@ test(t, {{'+', 3, 3}, {'&', 3, 'a'}}, {{1, '1', 1, '1'}})
 ...
 test(t, {{'+', 3}, {'&', 3, 'a'}}, {{1, '1', 1, '1'}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
+    2'
 ...
 test(t, {{':', 3, 3}}, {{1, '1', 1, '1'}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 5, got
+    3'
 ...
 test(t, {{':', 3, 3, 3}}, {{1, '1', 1, '1'}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 5, got
+    4'
 ...
 test(t, {{'?', 3, 3}}, {{1, '1', 1, '1'}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #1: "?"'
 ...
 'dump ' .. anything_to_string(box.space.s:select{})
 ---
diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result
index 78cb2a28d..59578ebdd 100644
--- a/test/vinyl/gh.result
+++ b/test/vinyl/gh.result
@@ -369,7 +369,8 @@ s:select()
 ...
 s:upsert({1, 'test', 'failed'}, {{'=', 3, 33}, {'=', 4, nil}})
 ---
-- error: Unknown UPDATE operation
+- error: 'Unknown UPDATE operation #2: wrong number of arguments, expected 3, got
+    2'
 ...
 s:select()
 ---
-- 
2.21.0 (Apple Git-122.2)



More information about the Tarantool-patches mailing list