tuple: fix non-informative update() error message

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, or poiniting out at invalid operation code. Fixes #3884
tarantool · Jan 13, 2020 · da926b2 · da926b2
1 parent 2bc4fe6
commit da926b2
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 21 deletions.
diff --git 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
@@ -177,8 +177,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
@@ -595,7 +595,7 @@ 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(char opcode, int op_num)
 {
 	switch (opcode) {
 	case '=':
@@ -614,9 +614,12 @@ xrow_update_op_by(char opcode)
 	case '!':
 		return &op_insert;
 	default:
-		diag_set(ClientError, ER_UNKNOWN_UPDATE_OP);
-		return NULL;
+		goto error;
 	}
+error:
+	diag_set(ClientError, ER_UNKNOWN_UPDATE_OP, op_num,
+		 tt_sprintf("\"%c\"", opcode));
+	return NULL;
 }
 
 int
@@ -637,7 +640,7 @@ xrow_update_op_next_token(struct xrow_update_op *op)
 }
 
 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) {
@@ -657,11 +660,14 @@ xrow_update_op_decode(struct xrow_update_op *op, int index_base,
 		return -1;
 	}
 	op->opcode = *mp_decode_str(expr, &len);
-	op->meta = xrow_update_op_by(op->opcode);
+	op->meta = xrow_update_op_by(op->opcode, 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;
 	}
 	/*

diff --git a/src/box/xrow_update_field.h b/src/box/xrow_update_field.h
@@ -223,6 +223,7 @@ xrow_update_op_next_token(struct xrow_update_op *op);
 /**
  * Decode an update operation from MessagePack.
  * @param[out] op Update operation.
+ * @param op_num Ordinal number of the operation.
  * @param index_base Field numbers base: 0 or 1.
  * @param dict Dictionary to lookup field number by a name.
  * @param expr MessagePack.
@@ -231,7 +232,7 @@ xrow_update_op_next_token(struct xrow_update_op *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);
 
 /**

diff --git a/test/box/gh-4511-access-settings-from-any-frontend.result b/test/box/gh-4511-access-settings-from-any-frontend.result
@@ -196,15 +196,18 @@ s:update('sql_defer_foreign_keys', {{}})
  | ...
 s:update('sql_defer_foreign_keys', {{'='}})
  | ---
- | - error: Unknown UPDATE operation
+ | - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
+ |     1'
  | ...
 s:update('sql_defer_foreign_keys', {{'=', 'value'}})
  | ---
- | - error: Unknown UPDATE operation
+ | - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
+ |     2'
  | ...
 s:update('sql_defer_foreign_keys', {{'=', 'value', true, 1}})
  | ---
- | - error: Unknown UPDATE operation
+ | - error: 'Unknown UPDATE operation #1: wrong number of arguments, expected 3, got
+ |     4'
  | ...
 
 s:update('sql_defer_foreign_keys', {{'+', 'value', 2}})

diff --git 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
@@ -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
@@ -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()
 ---