diff --git a/changelogs/unreleased/gh-5975-vy-duplicate-key-error-fix.md b/changelogs/unreleased/gh-5975-vy-duplicate-key-error-fix.md new file mode 100644 index 000000000000..c13ca7a1b18d --- /dev/null +++ b/changelogs/unreleased/gh-5975-vy-duplicate-key-error-fix.md @@ -0,0 +1,3 @@ +## bugfix/vinyl + +* Fixed a bug when a duplicate key error referred to a wrong index (gh-5975). diff --git a/src/box/space.c b/src/box/space.c index 4a2a51b7ddef..3da0c0d104ec 100644 --- a/src/box/space.c +++ b/src/box/space.c @@ -516,15 +516,6 @@ space_index_def(struct space *space, int n) return space->index[n]->def; } -const char * -index_name_by_id(struct space *space, uint32_t id) -{ - struct index *index = space_index(space, id); - if (index != NULL) - return index->def->name; - return NULL; -} - /** * Run BEFORE triggers and foreign key constraint checks registered for a space. * If a trigger changes the current statement, this function updates the diff --git a/src/box/space.h b/src/box/space.h index 930184cf329d..8941a4694976 100644 --- a/src/box/space.h +++ b/src/box/space.h @@ -460,18 +460,6 @@ space_bsize(struct space *space); struct index_def * space_index_def(struct space *space, int n); -/** - * Get name of the index by its identifier and parent space. - * - * @param space Parent space. - * @param id Index identifier. - * - * @retval not NULL Index name. - * @retval NULL No index with the specified identifier. - */ -const char * -index_name_by_id(struct space *space, uint32_t id); - /** * Check whether or not the current user can be granted * the requested access to the space. diff --git a/src/box/vinyl.c b/src/box/vinyl.c index e1c8b9e388d2..f163ff660dcb 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -1510,8 +1510,6 @@ vy_get_by_raw_key(struct vy_lsm *lsm, struct vy_tx *tx, * of the primary index. * @param tx Current transaction. * @param rv Read view. - * @param space_name Space name. - * @param index_name Index name. * @param lsm LSM tree corresponding to the index. * @param stmt New tuple. * @@ -1520,7 +1518,6 @@ vy_get_by_raw_key(struct vy_lsm *lsm, struct vy_tx *tx, */ static inline int vy_check_is_unique_primary(struct vy_tx *tx, const struct vy_read_view **rv, - const char *space_name, const char *index_name, struct vy_lsm *lsm, struct tuple *stmt) { assert(lsm->index_id == 0); @@ -1531,8 +1528,10 @@ vy_check_is_unique_primary(struct vy_tx *tx, const struct vy_read_view **rv, if (vy_get(lsm, tx, rv, stmt, &found)) return -1; if (found != NULL) { - diag_set(ClientError, ER_TUPLE_FOUND, index_name, space_name, - tuple_str(found), tuple_str(stmt)); + struct index *index = &lsm->base; + struct space *space = space_by_id(index->def->space_id); + diag_set(ClientError, ER_TUPLE_FOUND, index->def->name, + space_name(space), tuple_str(found), tuple_str(stmt)); tuple_unref(found); return -1; } @@ -1541,7 +1540,6 @@ vy_check_is_unique_primary(struct vy_tx *tx, const struct vy_read_view **rv, static int vy_check_is_unique_secondary_one(struct vy_tx *tx, const struct vy_read_view **rv, - const char *space_name, const char *index_name, struct vy_lsm *lsm, struct tuple *stmt, int multikey_idx) { @@ -1579,8 +1577,10 @@ vy_check_is_unique_secondary_one(struct vy_tx *tx, const struct vy_read_view **r return 0; } if (found != NULL) { - diag_set(ClientError, ER_TUPLE_FOUND, index_name, space_name, - tuple_str(found), tuple_str(stmt)); + struct index *index = &lsm->base; + struct space *space = space_by_id(index->def->space_id); + diag_set(ClientError, ER_TUPLE_FOUND, index->def->name, + space_name(space), tuple_str(found), tuple_str(stmt)); tuple_unref(found); return -1; } @@ -1592,8 +1592,6 @@ vy_check_is_unique_secondary_one(struct vy_tx *tx, const struct vy_read_view **r * of a secondary index. * @param tx Current transaction. * @param rv Read view. - * @param space_name Space name. - * @param index_name Index name. * @param lsm LSM tree corresponding to the index. * @param stmt New tuple. * @@ -1602,19 +1600,16 @@ vy_check_is_unique_secondary_one(struct vy_tx *tx, const struct vy_read_view **r */ static int vy_check_is_unique_secondary(struct vy_tx *tx, const struct vy_read_view **rv, - const char *space_name, const char *index_name, struct vy_lsm *lsm, struct tuple *stmt) { assert(lsm->opts.is_unique); if (!lsm->cmp_def->is_multikey) { - return vy_check_is_unique_secondary_one(tx, rv, - space_name, index_name, lsm, stmt, - MULTIKEY_NONE); + return vy_check_is_unique_secondary_one(tx, rv, lsm, stmt, + MULTIKEY_NONE); } int count = tuple_multikey_count(stmt, lsm->cmp_def); for (int i = 0; i < count; ++i) { - if (vy_check_is_unique_secondary_one(tx, rv, - space_name, index_name, lsm, stmt, i) != 0) + if (vy_check_is_unique_secondary_one(tx, rv, lsm, stmt, i) != 0) return -1; } return 0; @@ -1660,9 +1655,7 @@ vy_check_is_unique(struct vy_env *env, struct vy_tx *tx, if (space_needs_check_unique_constraint(space, 0) && vy_stmt_type(stmt) == IPROTO_INSERT) { struct vy_lsm *lsm = vy_lsm(space->index[0]); - if (vy_check_is_unique_primary(tx, rv, space_name(space), - index_name_by_id(space, 0), - lsm, stmt) != 0) + if (vy_check_is_unique_primary(tx, rv, lsm, stmt) != 0) return -1; } @@ -1677,9 +1670,7 @@ vy_check_is_unique(struct vy_env *env, struct vy_tx *tx, if (key_update_can_be_skipped(lsm->key_def->column_mask, column_mask)) continue; - if (vy_check_is_unique_secondary(tx, rv, space_name(space), - index_name_by_id(space, i), - lsm, stmt) != 0) + if (vy_check_is_unique_secondary(tx, rv, lsm, stmt) != 0) return -1; } return 0; @@ -3899,12 +3890,6 @@ struct vy_build_ctx { * if this flag is set. */ bool check_unique_constraint; - /** - * Names of the altered space and the new index. - * Used for error reporting. - */ - const char *space_name; - const char *index_name; /** Set in case a build error occurred. */ bool is_failed; /** Container for storing errors. */ @@ -3936,7 +3921,6 @@ vy_build_on_replace(struct trigger *trigger, void *event) /* Check key uniqueness if necessary. */ if (ctx->check_unique_constraint && stmt->new_tuple != NULL && vy_check_is_unique_secondary(tx, vy_tx_read_view(tx), - ctx->space_name, ctx->index_name, lsm, stmt->new_tuple) != 0) goto err; @@ -4009,7 +3993,6 @@ vy_build_insert_stmt(struct vy_lsm *lsm, struct vy_mem *mem, */ static int vy_build_insert_tuple(struct vy_env *env, struct vy_lsm *lsm, - const char *space_name, const char *index_name, struct tuple_format *new_format, bool check_unique_constraint, struct tuple *tuple) { @@ -4050,9 +4033,8 @@ vy_build_insert_tuple(struct vy_env *env, struct vy_lsm *lsm, */ if (check_unique_constraint) { vy_mem_pin(mem); - rc = vy_check_is_unique_secondary(NULL, - &env->xm->p_committed_read_view, - space_name, index_name, lsm, stmt); + rc = vy_check_is_unique_secondary( + NULL, &env->xm->p_committed_read_view, lsm, stmt); vy_mem_unpin(mem); if (rc != 0) { tuple_unref(stmt); @@ -4298,8 +4280,6 @@ vinyl_space_build_index(struct space *src_space, struct index *new_index, ctx.lsm = new_lsm; ctx.format = new_format; ctx.check_unique_constraint = check_unique_constraint; - ctx.space_name = space_name(src_space); - ctx.index_name = new_index->def->name; ctx.is_failed = false; diag_create(&ctx.diag); trigger_create(&on_replace, vy_build_on_replace, &ctx, NULL); @@ -4341,10 +4321,7 @@ vinyl_space_build_index(struct space *src_space, struct index *new_index, * in which case we would insert an outdated tuple. */ if (vy_stmt_lsn(tuple) <= build_lsn) { - rc = vy_build_insert_tuple(env, new_lsm, - space_name(src_space), - new_index->def->name, - new_format, + rc = vy_build_insert_tuple(env, new_lsm, new_format, check_unique_constraint, tuple); if (rc != 0) diff --git a/test/engine-luatest/gh_5975_duplicate_key_error_test.lua b/test/engine-luatest/gh_5975_duplicate_key_error_test.lua new file mode 100644 index 000000000000..f64b6c80e9c5 --- /dev/null +++ b/test/engine-luatest/gh_5975_duplicate_key_error_test.lua @@ -0,0 +1,60 @@ +local t = require('luatest') + +local server = require('luatest.server') + +local g = t.group(nil, t.helpers.matrix{engine = {'memtx', 'vinyl'}}) + +g.before_all(function(cg) + cg.server = server:new() + cg.server:start() +end) + +g.after_all(function(cg) + cg.server:drop() +end) + +g.after_each(function(cg) + cg.server:exec(function() + if box.space.test ~= nil then + box.space.test:drop() + end + end) +end) + +g.test_duplicate_key_error = function(cg) + cg.server:exec(function(engine) + local function check_err(space_name, index_name, old_tuple, new_tuple) + local space = box.space[space_name] + local errmsg = string.format( + 'Duplicate key exists in unique index "%s" in space ' .. + '"%s" with old tuple - %s and new tuple - %s', + index_name, space_name, + box.tuple.new(old_tuple), box.tuple.new(new_tuple)) + t.assert_error_msg_equals(errmsg, space.insert, space, new_tuple) + end + + local s = box.schema.space.create('test', { + engine = engine, + format = { + {name = 'a', type = 'unsigned'}, + {name = 'b', type = 'unsigned'}, + }, + }) + s:create_index('pk', {parts = {'a'}}) + s:create_index('i1', {parts = {'b'}, unique = true}) + s:create_index('i2', {parts = {'b'}, unique = true}) + s:insert({1, 1}) + + check_err('test', 'pk', {1, 1}, {1, 2}) + check_err('test', 'i1', {1, 1}, {2, 1}) + t.assert_equals(s:select({}, {fullscan = true}), {{1, 1}}) + s.index.i1:drop() + check_err('test', 'pk', {1, 1}, {1, 2}) + check_err('test', 'i2', {1, 1}, {2, 1}) + t.assert_equals(s:select({}, {fullscan = true}), {{1, 1}}) + s.index.i2:drop() + check_err('test', 'pk', {1, 1}, {1, 2}) + s:insert({2, 1}) + t.assert_equals(s:select({}, {fullscan = true}), {{1, 1}, {2, 1}}) + end, {cg.params.engine}) +end