Skip to content

Commit

Permalink
tuple: fix non-informative update() error message
Browse files Browse the repository at this point in the history
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
  • Loading branch information
slumber committed Nov 26, 2019
1 parent 5d2105b commit 208f24d
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 19 deletions.
2 changes: 1 addition & 1 deletion src/box/errcode.h
Expand Up @@ -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)") \
Expand Down
4 changes: 2 additions & 2 deletions src/box/xrow_update.c
Expand Up @@ -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;
/*
Expand Down
23 changes: 16 additions & 7 deletions src/box/xrow_update_field.c
Expand Up @@ -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 '+':
Expand All @@ -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) {
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/box/xrow_update_field.h
Expand Up @@ -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 */
Expand Down
6 changes: 4 additions & 2 deletions test/box/update.result
Expand Up @@ -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}, {{'+', '+', '+'}})
---
Expand Down
13 changes: 8 additions & 5 deletions test/engine/upsert.result
Expand Up @@ -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
---
Expand Down Expand Up @@ -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{})
---
Expand Down
3 changes: 2 additions & 1 deletion test/vinyl/gh.result
Expand Up @@ -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()
---
Expand Down

0 comments on commit 208f24d

Please sign in to comment.