From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Mon, 2 Apr 2018 11:46:49 +0300 From: Vladimir Davydov Subject: Re: [PATCH 12/12] vinyl: allow to modify key definition if it does not require rebuild Message-ID: <20180402084649.dxtykpka5tylrgn2@esperanza> References: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: To: kostja@tarantool.org Cc: tarantool-patches@freelists.org List-ID: On Sun, Apr 01, 2018 at 12:05:39PM +0300, Vladimir Davydov wrote: > @@ -945,11 +973,19 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space) > */ > if (env->status != VINYL_ONLINE) > return 0; > - /* > - * Regardless of the space emptyness, key definition of an > - * existing index can not be changed, because key > - * definition is already in vylog. See #3169. > - */ > + /* The space is empty. Allow alter. */ > + if (pk->stat.disk.count.rows == 0 && > + pk->stat.memory.count.rows == 0) > + return 0; > + if (space_def_check_compatibility(old_space->def, new_space->def, > + false) != 0) > + return -1; > + if (old_space->index_count < new_space->index_count) { > + diag_set(ClientError, ER_UNSUPPORTED, "Vinyl", > + "adding an index to a non-empty space"); > + return -1; > + } > + These two checks got added by git-revert. They are pointless as we already have them in vinyl_space_prepare_alter(). I removed them on the branch. Here's the updated diff: diff --git a/src/box/key_def.cc b/src/box/key_def.cc index 2f157419..50786913 100644 --- a/src/box/key_def.cc +++ b/src/box/key_def.cc @@ -107,6 +107,15 @@ key_def_dup(const struct key_def *src) } void +key_def_swap(struct key_def *old_def, struct key_def *new_def) +{ + assert(old_def->part_count == new_def->part_count); + for (uint32_t i = 0; i < new_def->part_count; i++) + SWAP(old_def->parts[i], new_def->parts[i]); + SWAP(*old_def, *new_def); +} + +void key_def_delete(struct key_def *def) { free(def); diff --git a/src/box/key_def.h b/src/box/key_def.h index fe9ee56f..84e3e1e5 100644 --- a/src/box/key_def.h +++ b/src/box/key_def.h @@ -150,6 +150,13 @@ struct key_def * key_def_dup(const struct key_def *src); /** + * Swap content of two key definitions in memory. + * The two key definitions must have the same size. + */ +void +key_def_swap(struct key_def *old_def, struct key_def *new_def); + +/** * Delete @a key_def. * @param def Key_def to delete. */ diff --git a/src/box/vinyl.c b/src/box/vinyl.c index 145fde70..d503704e 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -803,7 +803,7 @@ vinyl_index_commit_create(struct index *index, int64_t lsn) * the index isn't in the recovery context and we * need to retry to log it now. */ - if (lsm->is_committed) { + if (lsm->commit_lsn >= 0) { vy_scheduler_add_lsm(&env->scheduler, lsm); return; } @@ -828,8 +828,8 @@ vinyl_index_commit_create(struct index *index, int64_t lsn) if (lsm->opts.lsn != 0) lsn = lsm->opts.lsn; - assert(!lsm->is_committed); - lsm->is_committed = true; + assert(lsm->commit_lsn < 0); + lsm->commit_lsn = lsn; assert(lsm->range_count == 1); struct vy_range *range = vy_range_tree_first(lsm->tree); @@ -854,6 +854,34 @@ vinyl_index_commit_create(struct index *index, int64_t lsn) vy_scheduler_add_lsm(&env->scheduler, lsm); } +static void +vinyl_index_commit_modify(struct index *index, int64_t lsn) +{ + struct vy_env *env = vy_env(index->engine); + struct vy_lsm *lsm = vy_lsm(index); + + (void)env; + assert(env->status == VINYL_ONLINE || + env->status == VINYL_FINAL_RECOVERY_LOCAL || + env->status == VINYL_FINAL_RECOVERY_REMOTE); + + if (lsn <= lsm->commit_lsn) { + /* + * This must be local recovery from WAL, when + * the operation has already been committed to + * vylog. + */ + assert(env->status == VINYL_FINAL_RECOVERY_LOCAL); + return; + } + + lsm->commit_lsn = lsn; + + vy_log_tx_begin(); + vy_log_modify_lsm(lsm->id, lsm->key_def, lsn); + vy_log_tx_try_commit(); +} + /* * Delete all runs, ranges, and slices of a given LSM tree * from the metadata log. @@ -945,11 +973,10 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space) */ if (env->status != VINYL_ONLINE) return 0; - /* - * Regardless of the space emptyness, key definition of an - * existing index can not be changed, because key - * definition is already in vylog. See #3169. - */ + /* The space is empty. Allow alter. */ + if (pk->stat.disk.count.rows == 0 && + pk->stat.memory.count.rows == 0) + return 0; if (old_space->index_count == new_space->index_count) { /* Check index_defs to be unchanged. */ for (uint32_t i = 0; i < old_space->index_count; ++i) { @@ -961,25 +988,14 @@ vinyl_space_prepare_alter(struct space *old_space, struct space *new_space) * vinyl yet. */ if (index_def_change_requires_rebuild(old_def, - new_def) || - key_part_cmp(old_def->key_def->parts, - old_def->key_def->part_count, - new_def->key_def->parts, - new_def->key_def->part_count) != 0) { + new_def)) { diag_set(ClientError, ER_UNSUPPORTED, "Vinyl", - "changing the definition of an index"); + "changing the definition of " + "a non-empty index"); return -1; } } } - if (pk->stat.disk.count.rows == 0 && - pk->stat.memory.count.rows == 0) - return 0; - /* - * Since space format is not persisted in vylog, it can be - * altered on non-empty space to some state, compatible - * with the old one. - */ if (space_def_check_compatibility(old_space->def, new_space->def, false) != 0) return -1; @@ -1083,6 +1099,8 @@ vinyl_space_swap_index(struct space *old_space, struct space *new_space, new_lsm->mem_format_with_colmask); SWAP(old_lsm->disk_format, new_lsm->disk_format); SWAP(old_lsm->opts, new_lsm->opts); + key_def_swap(old_lsm->key_def, new_lsm->key_def); + key_def_swap(old_lsm->cmp_def, new_lsm->cmp_def); } static int @@ -3931,7 +3949,7 @@ static const struct index_vtab vinyl_index_vtab = { /* .destroy = */ vinyl_index_destroy, /* .commit_create = */ vinyl_index_commit_create, /* .abort_create = */ generic_index_abort_create, - /* .commit_modify = */ generic_index_commit_modify, + /* .commit_modify = */ vinyl_index_commit_modify, /* .commit_drop = */ vinyl_index_commit_drop, /* .update_def = */ generic_index_update_def, /* .depends_on_pk = */ vinyl_index_depends_on_pk, diff --git a/src/box/vy_log.c b/src/box/vy_log.c index 5552065d..172a1b01 100644 --- a/src/box/vy_log.c +++ b/src/box/vy_log.c @@ -81,6 +81,7 @@ enum vy_log_key { VY_LOG_KEY_GC_LSN = 10, VY_LOG_KEY_TRUNCATE_COUNT = 11, VY_LOG_KEY_CREATE_LSN = 12, + VY_LOG_KEY_MODIFY_LSN = 13, }; /** vy_log_key -> human readable name. */ @@ -98,6 +99,7 @@ static const char *vy_log_key_name[] = { [VY_LOG_KEY_GC_LSN] = "gc_lsn", [VY_LOG_KEY_TRUNCATE_COUNT] = "truncate_count", [VY_LOG_KEY_CREATE_LSN] = "create_lsn", + [VY_LOG_KEY_MODIFY_LSN] = "modify_lsn", }; /** vy_log_type -> human readable name. */ @@ -115,6 +117,7 @@ static const char *vy_log_type_name[] = { [VY_LOG_DUMP_LSM] = "dump_lsm", [VY_LOG_SNAPSHOT] = "snapshot", [VY_LOG_TRUNCATE_LSM] = "truncate_lsm", + [VY_LOG_MODIFY_LSM] = "modify_lsm", }; /** Metadata log object. */ @@ -251,6 +254,10 @@ vy_log_record_snprint(char *buf, int size, const struct vy_log_record *record) SNPRINT(total, snprintf, buf, size, "%s=%"PRIi64", ", vy_log_key_name[VY_LOG_KEY_CREATE_LSN], record->create_lsn); + if (record->modify_lsn > 0) + SNPRINT(total, snprintf, buf, size, "%s=%"PRIi64", ", + vy_log_key_name[VY_LOG_KEY_MODIFY_LSN], + record->modify_lsn); if (record->dump_lsn > 0) SNPRINT(total, snprintf, buf, size, "%s=%"PRIi64", ", vy_log_key_name[VY_LOG_KEY_DUMP_LSN], @@ -360,6 +367,11 @@ vy_log_record_encode(const struct vy_log_record *record, size += mp_sizeof_uint(record->create_lsn); n_keys++; } + if (record->modify_lsn > 0) { + size += mp_sizeof_uint(VY_LOG_KEY_MODIFY_LSN); + size += mp_sizeof_uint(record->modify_lsn); + n_keys++; + } if (record->dump_lsn > 0) { size += mp_sizeof_uint(VY_LOG_KEY_DUMP_LSN); size += mp_sizeof_uint(record->dump_lsn); @@ -432,6 +444,10 @@ vy_log_record_encode(const struct vy_log_record *record, pos = mp_encode_uint(pos, VY_LOG_KEY_CREATE_LSN); pos = mp_encode_uint(pos, record->create_lsn); } + if (record->modify_lsn > 0) { + pos = mp_encode_uint(pos, VY_LOG_KEY_MODIFY_LSN); + pos = mp_encode_uint(pos, record->modify_lsn); + } if (record->dump_lsn > 0) { pos = mp_encode_uint(pos, VY_LOG_KEY_DUMP_LSN); pos = mp_encode_uint(pos, record->dump_lsn); @@ -552,6 +568,9 @@ vy_log_record_decode(struct vy_log_record *record, case VY_LOG_KEY_CREATE_LSN: record->create_lsn = mp_decode_uint(&pos); break; + case VY_LOG_KEY_MODIFY_LSN: + record->modify_lsn = mp_decode_uint(&pos); + break; case VY_LOG_KEY_DUMP_LSN: record->dump_lsn = mp_decode_uint(&pos); break; @@ -568,7 +587,7 @@ vy_log_record_decode(struct vy_log_record *record, goto fail; } } - if (record->type == VY_LOG_CREATE_LSM && record->create_lsn == 0) { + if (record->type == VY_LOG_CREATE_LSM) { /* * We used to use LSN as unique LSM tree identifier * and didn't store LSN separately so if there's @@ -577,7 +596,14 @@ vy_log_record_decode(struct vy_log_record *record, * fact the LSN of the WAL record that committed * the LSM tree. */ - record->create_lsn = record->lsm_id; + if (record->create_lsn == 0) + record->create_lsn = record->lsm_id; + /* + * If the LSM tree has never been modified, initialize + * 'modify_lsn' with 'create_lsn'. + */ + if (record->modify_lsn == 0) + record->modify_lsn = record->create_lsn; } return 0; fail: @@ -1172,7 +1198,7 @@ vy_recovery_create_lsm(struct vy_recovery *recovery, int64_t id, uint32_t space_id, uint32_t index_id, const struct key_part_def *key_parts, uint32_t key_part_count, int64_t create_lsn, - int64_t dump_lsn) + int64_t modify_lsn, int64_t dump_lsn) { struct vy_lsm_recovery_info *lsm; struct key_part_def *key_parts_copy; @@ -1259,6 +1285,7 @@ vy_recovery_create_lsm(struct vy_recovery *recovery, int64_t id, lsm->key_part_count = key_part_count; lsm->is_dropped = false; lsm->create_lsn = create_lsn; + lsm->modify_lsn = modify_lsn; lsm->dump_lsn = dump_lsn; /* @@ -1286,6 +1313,43 @@ vy_recovery_create_lsm(struct vy_recovery *recovery, int64_t id, } /** + * Handle a VY_LOG_MODIFY_LSM log record. + * This function updates key definition of the LSM tree with ID @id. + * Return 0 on success, -1 on failure. + */ +static int +vy_recovery_modify_lsm(struct vy_recovery *recovery, int64_t id, + const struct key_part_def *key_parts, + uint32_t key_part_count, int64_t modify_lsn) +{ + struct vy_lsm_recovery_info *lsm; + lsm = vy_recovery_lookup_lsm(recovery, id); + if (lsm == NULL) { + diag_set(ClientError, ER_INVALID_VYLOG_FILE, + tt_sprintf("Update of unregistered LSM tree %lld", + (long long)id)); + return -1; + } + if (lsm->is_dropped) { + diag_set(ClientError, ER_INVALID_VYLOG_FILE, + tt_sprintf("Update of deleted LSM tree %lld", + (long long)id)); + return -1; + } + free(lsm->key_parts); + lsm->key_parts = malloc(sizeof(*key_parts) * key_part_count); + if (lsm->key_parts == NULL) { + diag_set(OutOfMemory, sizeof(*key_parts) * key_part_count, + "malloc", "struct key_part_def"); + return -1; + } + memcpy(lsm->key_parts, key_parts, sizeof(*key_parts) * key_part_count); + lsm->key_part_count = key_part_count; + lsm->modify_lsn = modify_lsn; + return 0; +} + +/** * Handle a VY_LOG_DROP_LSM log record. * This function marks the LSM tree with ID @id as dropped. * All ranges and runs of the LSM tree must have been deleted by now. @@ -1757,7 +1821,13 @@ vy_recovery_process_record(struct vy_recovery *recovery, rc = vy_recovery_create_lsm(recovery, record->lsm_id, record->space_id, record->index_id, record->key_parts, record->key_part_count, - record->create_lsn, record->dump_lsn); + record->create_lsn, record->modify_lsn, + record->dump_lsn); + break; + case VY_LOG_MODIFY_LSM: + rc = vy_recovery_modify_lsm(recovery, record->lsm_id, + record->key_parts, record->key_part_count, + record->modify_lsn); break; case VY_LOG_DROP_LSM: rc = vy_recovery_drop_lsm(recovery, record->lsm_id); @@ -2007,6 +2077,7 @@ vy_log_append_lsm(struct xlog *xlog, struct vy_lsm_recovery_info *lsm) record.key_parts = lsm->key_parts; record.key_part_count = lsm->key_part_count; record.create_lsn = lsm->create_lsn; + record.modify_lsn = lsm->modify_lsn; record.dump_lsn = lsm->dump_lsn; if (vy_log_append_record(xlog, &record) != 0) return -1; diff --git a/src/box/vy_log.h b/src/box/vy_log.h index 702963d5..1b2b419f 100644 --- a/src/box/vy_log.h +++ b/src/box/vy_log.h @@ -164,6 +164,11 @@ enum vy_log_record_type { * WAL records written after truncation. */ VY_LOG_TRUNCATE_LSM = 12, + /** + * Modify key definition of an LSM tree. + * Requires vy_log_record::lsm_id, key_def, modify_lsn. + */ + VY_LOG_MODIFY_LSM = 13, vy_log_record_type_MAX }; @@ -202,6 +207,8 @@ struct vy_log_record { uint32_t key_part_count; /** LSN of the WAL row that created the LSM tree. */ int64_t create_lsn; + /** LSN of the WAL row that last modified the LSM tree. */ + int64_t modify_lsn; /** Max LSN stored on disk. */ int64_t dump_lsn; /** @@ -255,6 +262,8 @@ struct vy_lsm_recovery_info { bool is_dropped; /** LSN of the WAL row that created the LSM tree. */ int64_t create_lsn; + /** LSN of the WAL row that last modified the LSM tree. */ + int64_t modify_lsn; /** LSN of the last LSM tree dump. */ int64_t dump_lsn; /** @@ -516,6 +525,19 @@ vy_log_create_lsm(int64_t id, uint32_t space_id, uint32_t index_id, vy_log_write(&record); } +/** Helper to log a vinyl LSM tree modification. */ +static inline void +vy_log_modify_lsm(int64_t id, const struct key_def *key_def, int64_t modify_lsn) +{ + struct vy_log_record record; + vy_log_record_init(&record); + record.type = VY_LOG_MODIFY_LSM; + record.lsm_id = id; + record.key_def = key_def; + record.modify_lsn = modify_lsn; + vy_log_write(&record); +} + /** Helper to log a vinyl LSM tree drop. */ static inline void vy_log_drop_lsm(int64_t id) diff --git a/src/box/vy_lsm.c b/src/box/vy_lsm.c index af6dde50..a8ed39ad 100644 --- a/src/box/vy_lsm.c +++ b/src/box/vy_lsm.c @@ -182,6 +182,7 @@ vy_lsm_new(struct vy_lsm_env *lsm_env, struct vy_cache_env *cache_env, lsm->id = -1; lsm->refs = 1; lsm->dump_lsn = -1; + lsm->commit_lsn = -1; vy_cache_create(&lsm->cache, cache_env, cmp_def); rlist_create(&lsm->sealed); vy_range_tree_new(lsm->tree); @@ -479,7 +480,7 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery, bool is_checkpoint_recovery, bool force_recovery) { assert(lsm->id < 0); - assert(!lsm->is_committed); + assert(lsm->commit_lsn < 0); assert(lsm->range_count == 0); /* @@ -535,7 +536,7 @@ vy_lsm_recover(struct vy_lsm *lsm, struct vy_recovery *recovery, } lsm->id = lsm_info->id; - lsm->is_committed = true; + lsm->commit_lsn = lsm_info->modify_lsn; if (lsn < lsm_info->create_lsn || lsm_info->is_dropped) { /* diff --git a/src/box/vy_lsm.h b/src/box/vy_lsm.h index f5862a52..201b5a56 100644 --- a/src/box/vy_lsm.h +++ b/src/box/vy_lsm.h @@ -255,11 +255,13 @@ struct vy_lsm { * been dumped yet. */ int64_t dump_lsn; - /* - * This flag is set if the LSM tree creation was - * committed to the metadata log. + /** + * LSN of the WAL row that created or last modified + * this LSM tree. We store it in vylog so that during + * local recovery we can replay vylog records we failed + * to log before restart. */ - bool is_committed; + int64_t commit_lsn; /** * This flag is set if the LSM tree was dropped. * It is also set on local recovery if the LSM tree diff --git a/test/engine/ddl.result b/test/engine/ddl.result index c3cb1efc..308aefb0 100644 --- a/test/engine/ddl.result +++ b/test/engine/ddl.result @@ -391,9 +391,6 @@ s:drop() -- gh-3229: update optionality if a space format is changed too, -- not only when indexes are updated. -- -box.cfg{} ---- -... s = box.schema.create_space('test', {engine = engine}) --- ... @@ -466,3 +463,113 @@ sk:get({5}) s:drop() --- ... +-- +-- Modify key definition without index rebuild. +-- +s = box.schema.create_space('test', {engine = engine}) +--- +... +i1 = s:create_index('i1', {unique = true, parts = {1, 'unsigned'}}) +--- +... +i2 = s:create_index('i2', {unique = false, parts = {2, 'unsigned'}}) +--- +... +i3 = s:create_index('i3', {unique = true, parts = {3, 'unsigned'}}) +--- +... +_ = s:insert{1, 2, 3} +--- +... +box.snapshot() +--- +- ok +... +_ = s:insert{3, 2, 1} +--- +... +i1:alter{parts = {1, 'integer'}} +--- +... +_ = s:insert{-1, 2, 2} +--- +... +i1:select() +--- +- - [-1, 2, 2] + - [1, 2, 3] + - [3, 2, 1] +... +i2:select() +--- +- - [-1, 2, 2] + - [1, 2, 3] + - [3, 2, 1] +... +i3:select() +--- +- - [3, 2, 1] + - [-1, 2, 2] + - [1, 2, 3] +... +i2:alter{parts = {2, 'integer'}} +--- +... +i3:alter{parts = {3, 'integer'}} +--- +... +_ = s:replace{-1, -1, -1} +--- +... +i1:select() +--- +- - [-1, -1, -1] + - [1, 2, 3] + - [3, 2, 1] +... +i2:select() +--- +- - [-1, -1, -1] + - [1, 2, 3] + - [3, 2, 1] +... +i3:select() +--- +- - [-1, -1, -1] + - [3, 2, 1] + - [1, 2, 3] +... +box.snapshot() +--- +- ok +... +_ = s:replace{-1, -2, -3} +--- +... +_ = s:replace{-3, -2, -1} +--- +... +i1:select() +--- +- - [-3, -2, -1] + - [-1, -2, -3] + - [1, 2, 3] + - [3, 2, 1] +... +i2:select() +--- +- - [-3, -2, -1] + - [-1, -2, -3] + - [1, 2, 3] + - [3, 2, 1] +... +i3:select() +--- +- - [-1, -2, -3] + - [-3, -2, -1] + - [3, 2, 1] + - [1, 2, 3] +... +s:drop() +--- +... diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua index 25381a3c..019e18a1 100644 --- a/test/engine/ddl.test.lua +++ b/test/engine/ddl.test.lua @@ -141,7 +141,6 @@ s:drop() -- gh-3229: update optionality if a space format is changed too, -- not only when indexes are updated. -- -box.cfg{} s = box.schema.create_space('test', {engine = engine}) format = {} format[1] = {'field1', 'unsigned'} @@ -162,3 +161,37 @@ pk:get({4}) sk:select({box.NULL}) sk:get({5}) s:drop() + +-- +-- Modify key definition without index rebuild. +-- +s = box.schema.create_space('test', {engine = engine}) +i1 = s:create_index('i1', {unique = true, parts = {1, 'unsigned'}}) +i2 = s:create_index('i2', {unique = false, parts = {2, 'unsigned'}}) +i3 = s:create_index('i3', {unique = true, parts = {3, 'unsigned'}}) + +_ = s:insert{1, 2, 3} +box.snapshot() +_ = s:insert{3, 2, 1} + +i1:alter{parts = {1, 'integer'}} +_ = s:insert{-1, 2, 2} +i1:select() +i2:select() +i3:select() + +i2:alter{parts = {2, 'integer'}} +i3:alter{parts = {3, 'integer'}} +_ = s:replace{-1, -1, -1} +i1:select() +i2:select() +i3:select() + +box.snapshot() +_ = s:replace{-1, -2, -3} +_ = s:replace{-3, -2, -1} +i1:select() +i2:select() +i3:select() + +s:drop() diff --git a/test/engine/replica_join.result b/test/engine/replica_join.result index a003c0d8..39d857fe 100644 --- a/test/engine/replica_join.result +++ b/test/engine/replica_join.result @@ -293,6 +293,12 @@ for k = 8, 1, -1 do box.space.test:update(k, {{'-', 2, 8}}) end for k = 9, 16 do box.space.test:delete(k) end --- ... +box.space.test.index.primary:alter{parts = {1, 'integer'}} +--- +... +for k = -8, -1 do box.space.test:insert{k, 9 + k} end +--- +... _ = box.space.test2:upsert({1, 'test1', 11}, {{'+', 3, 1}}) --- ... @@ -333,7 +339,15 @@ test_run:cmd('switch replica') ... box.space.test:select() --- -- - [1, 8] +- - [-8, 1] + - [-7, 2] + - [-6, 3] + - [-5, 4] + - [-4, 5] + - [-3, 6] + - [-2, 7] + - [-1, 8] + - [1, 8] - [2, 7] - [3, 6] - [4, 5] @@ -344,13 +358,21 @@ box.space.test:select() ... box.space.test.index.secondary:select() --- -- - [8, 1] +- - [-8, 1] + - [8, 1] + - [-7, 2] - [7, 2] + - [-6, 3] - [6, 3] + - [-5, 4] - [5, 4] + - [-4, 5] - [4, 5] + - [-3, 6] - [3, 6] + - [-2, 7] - [2, 7] + - [-1, 8] - [1, 8] ... box.space.test2:select() diff --git a/test/engine/replica_join.test.lua b/test/engine/replica_join.test.lua index 5c6ed11a..1792281e 100644 --- a/test/engine/replica_join.test.lua +++ b/test/engine/replica_join.test.lua @@ -72,6 +72,8 @@ _ = test_run:cmd("cleanup server replica") -- new data for k = 8, 1, -1 do box.space.test:update(k, {{'-', 2, 8}}) end for k = 9, 16 do box.space.test:delete(k) end +box.space.test.index.primary:alter{parts = {1, 'integer'}} +for k = -8, -1 do box.space.test:insert{k, 9 + k} end _ = box.space.test2:upsert({1, 'test1', 11}, {{'+', 3, 1}}) _ = box.space.test2:update({2, 'test2'}, {{'+', 3, 2}}) _ = box.space.test2:delete{3, 'test3'} diff --git a/test/engine/truncate.result b/test/engine/truncate.result index b6e1a99a..2222804b 100644 --- a/test/engine/truncate.result +++ b/test/engine/truncate.result @@ -391,7 +391,7 @@ _ = s3:insert{789, 987} ... -- Check that index drop, create, and alter called after space -- truncate do not break recovery (gh-2615) -s4 = box.schema.create_space('test4', {engine = 'memtx'}) +s4 = box.schema.create_space('test4', {engine = engine}) --- ... _ = s4:create_index('i1', {parts = {1, 'string'}}) diff --git a/test/engine/truncate.test.lua b/test/engine/truncate.test.lua index 66cbd156..df2797a1 100644 --- a/test/engine/truncate.test.lua +++ b/test/engine/truncate.test.lua @@ -175,7 +175,7 @@ s3:truncate() _ = s3:insert{789, 987} -- Check that index drop, create, and alter called after space -- truncate do not break recovery (gh-2615) -s4 = box.schema.create_space('test4', {engine = 'memtx'}) +s4 = box.schema.create_space('test4', {engine = engine}) _ = s4:create_index('i1', {parts = {1, 'string'}}) _ = s4:create_index('i3', {parts = {3, 'string'}}) _ = s4:insert{'zzz', 111, 'yyy'} diff --git a/test/vinyl/ddl.result b/test/vinyl/ddl.result index 6f393c69..456977e5 100644 --- a/test/vinyl/ddl.result +++ b/test/vinyl/ddl.result @@ -87,7 +87,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) ... space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... #box.space._index:select({space.id}) --- @@ -116,7 +116,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) ... space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... #box.space._index:select({space.id}) --- @@ -145,7 +145,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) ... space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... #box.space._index:select({space.id}) --- @@ -165,7 +165,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) ... space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... box.snapshot() --- @@ -174,27 +174,33 @@ box.snapshot() while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end --- ... --- After a dump REPLACE + DELETE = nothing, so the space is empty --- but an index can not be altered. +-- after a dump REPLACE + DELETE = nothing, so the space is empty now and +-- can be altered. index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) --- ... space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index ... --- Space format still can be altered. -format = {} +#box.space._index:select({space.id}) --- +- 2 ... -format[1] = {name = 'field1', type = 'unsigned'} +box.space._index:get{space.id, 0}[6] --- +- [[0, 'unsigned'], [1, 'unsigned']] ... -format[2] = {name = 'field2', type = 'unsigned'} +space:insert({1, 2}) --- +- [1, 2] ... -space:format(format) +index:select{} +--- +- - [1, 2] +... +index2:select{} --- +- - [1, 2] ... space:drop() --- @@ -230,7 +236,7 @@ index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) ... space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... -- After compaction the REPLACE + DELETE + DELETE = nothing, so -- the space is now empty and can be altered. @@ -256,10 +262,8 @@ while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) --- ... --- Can not alter an index even if it becames empty after dump. space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index ... space:drop() --- @@ -287,7 +291,7 @@ space:auto_increment{3} ... box.space._index:replace{space.id, 0, 'pk', 'tree', {unique=true}, {{0, 'unsigned'}, {1, 'unsigned'}}} --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... space:select{} --- @@ -828,7 +832,7 @@ s.index.secondary.unique ... s.index.secondary:alter{unique = true} -- error --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... s.index.secondary.unique --- @@ -846,43 +850,3 @@ s.index.secondary:select(10) s:drop() --- ... --- --- gh-3169: vinyl index key definition can not be altered even if --- the index is empty. --- -s = box.schema.space.create('vinyl', {engine = 'vinyl'}) ---- -... -i = s:create_index('pk') ---- -... -i:alter{parts = {1, 'integer'}} ---- -- error: Vinyl does not support changing the definition of an index -... -_ = s:replace{-1} ---- -- error: 'Tuple field 1 type does not match one required by operation: expected unsigned' -... -_ = s:replace{1} ---- -... -_ = s:replace{-2} ---- -- error: 'Tuple field 1 type does not match one required by operation: expected unsigned' -... -_ = s:replace{3} ---- -... -_ = s:replace{-3} ---- -- error: 'Tuple field 1 type does not match one required by operation: expected unsigned' -... -s:select{} ---- -- - [1] - - [3] -... -s:drop() ---- -... diff --git a/test/vinyl/ddl.test.lua b/test/vinyl/ddl.test.lua index f050799e..8ce8b920 100644 --- a/test/vinyl/ddl.test.lua +++ b/test/vinyl/ddl.test.lua @@ -59,15 +59,15 @@ space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) box.snapshot() while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end --- After a dump REPLACE + DELETE = nothing, so the space is empty --- but an index can not be altered. +-- after a dump REPLACE + DELETE = nothing, so the space is empty now and +-- can be altered. index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) --- Space format still can be altered. -format = {} -format[1] = {name = 'field1', type = 'unsigned'} -format[2] = {name = 'field2', type = 'unsigned'} -space:format(format) +#box.space._index:select({space.id}) +box.space._index:get{space.id, 0}[6] +space:insert({1, 2}) +index:select{} +index2:select{} space:drop() space = box.schema.space.create('test', { engine = 'vinyl' }) @@ -91,7 +91,6 @@ box.snapshot() -- Wait until the dump is finished. while space.index.primary:info().rows ~= 0 do fiber.sleep(0.01) end index2 = space:create_index('secondary', { parts = {2, 'unsigned'} }) --- Can not alter an index even if it becames empty after dump. space.index.primary:alter({parts = {1, 'unsigned', 2, 'unsigned'}}) space:drop() @@ -305,18 +304,3 @@ s.index.secondary.unique s:insert{2, 10} s.index.secondary:select(10) s:drop() - --- --- gh-3169: vinyl index key definition can not be altered even if --- the index is empty. --- -s = box.schema.space.create('vinyl', {engine = 'vinyl'}) -i = s:create_index('pk') -i:alter{parts = {1, 'integer'}} -_ = s:replace{-1} -_ = s:replace{1} -_ = s:replace{-2} -_ = s:replace{3} -_ = s:replace{-3} -s:select{} -s:drop() diff --git a/test/vinyl/gh.result b/test/vinyl/gh.result index 32e4b6fb..9c72acc8 100644 --- a/test/vinyl/gh.result +++ b/test/vinyl/gh.result @@ -144,7 +144,7 @@ s:insert{5, 5} ... s.index.primary:alter({parts={2,'unsigned'}}) --- -- error: Vinyl does not support changing the definition of an index +- error: Vinyl does not support changing the definition of a non-empty index ... s:drop() --- diff --git a/test/vinyl/layout.result b/test/vinyl/layout.result index 4af2c024..e6294d3c 100644 --- a/test/vinyl/layout.result +++ b/test/vinyl/layout.result @@ -20,7 +20,7 @@ space = box.schema.space.create('test', {engine='vinyl'}) _ = space:create_index('pk', {parts = {{1, 'string', collation = 'unicode'}}, run_count_per_level=3}) --- ... -_ = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}, run_count_per_level=3}) +_ = space:create_index('sk', {parts = {{2, 'unsigned'}}, run_count_per_level=3}) --- ... -- Empty run @@ -35,6 +35,9 @@ box.snapshot() --- - ok ... +space.index.sk:alter{parts = {{2, 'unsigned', is_nullable = true}}} +--- +... space:replace{'ЭЭЭ', box.NULL} --- - ['ЭЭЭ', null] @@ -123,16 +126,16 @@ test_run:cmd("push filter 'offset: .*' to 'offset: '") ... result --- -- - - 00000000000000000009.vylog +- - - 00000000000000000010.vylog - - HEADER: type: INSERT BODY: - tuple: [0, {7: [{'field': 0, 'collation': 1, 'type': 'string'}], 9: 9, 12: 3, - 6: 512}] + tuple: [0, {6: 512, 7: [{'field': 0, 'collation': 1, 'type': 'string'}], + 9: 10, 12: 3, 13: 7}] - HEADER: type: INSERT BODY: - tuple: [5, {2: 8, 9: 9}] + tuple: [5, {2: 8, 9: 10}] - HEADER: type: INSERT BODY: @@ -153,11 +156,11 @@ result type: INSERT BODY: tuple: [0, {0: 2, 5: 1, 6: 512, 7: [{'field': 1, 'is_nullable': true, 'type': 'unsigned'}], - 9: 9, 12: 4}] + 9: 10, 12: 4, 13: 7}] - HEADER: type: INSERT BODY: - tuple: [5, {0: 2, 2: 6, 9: 9}] + tuple: [5, {0: 2, 2: 6, 9: 10}] - HEADER: type: INSERT BODY: @@ -197,7 +200,7 @@ result timestamp: type: INSERT BODY: - tuple: [5, {0: 2, 2: 10, 9: 12}] + tuple: [5, {0: 2, 2: 10, 9: 13}] - HEADER: timestamp: type: INSERT @@ -207,7 +210,7 @@ result timestamp: type: INSERT BODY: - tuple: [10, {0: 2, 9: 12}] + tuple: [10, {0: 2, 9: 13}] - HEADER: timestamp: type: INSERT @@ -217,7 +220,7 @@ result timestamp: type: INSERT BODY: - tuple: [5, {2: 12, 9: 12}] + tuple: [5, {2: 12, 9: 13}] - HEADER: timestamp: type: INSERT @@ -227,16 +230,16 @@ result timestamp: type: INSERT BODY: - tuple: [10, {9: 12}] + tuple: [10, {9: 13}] - - 00000000000000000008.index - - HEADER: type: RUNINFO BODY: - min_lsn: 7 + min_lsn: 8 max_key: ['ЭЭЭ'] page_count: 1 bloom_filter: - max_lsn: 9 + max_lsn: 10 min_key: ['ёёё'] - HEADER: type: PAGEINFO @@ -249,17 +252,17 @@ result min_key: ['ёёё'] - - 00000000000000000008.run - - HEADER: - lsn: 9 + lsn: 10 type: INSERT BODY: tuple: ['ёёё', null] - HEADER: - lsn: 8 + lsn: 9 type: INSERT BODY: tuple: ['эээ', null] - HEADER: - lsn: 7 + lsn: 8 type: INSERT BODY: tuple: ['ЭЭЭ', null] @@ -271,11 +274,11 @@ result - - HEADER: type: RUNINFO BODY: - min_lsn: 10 + min_lsn: 11 max_key: ['ЮЮЮ'] page_count: 1 bloom_filter: - max_lsn: 12 + max_lsn: 13 min_key: ['ёёё'] - HEADER: type: PAGEINFO @@ -288,17 +291,17 @@ result min_key: ['ёёё'] - - 00000000000000000012.run - - HEADER: - lsn: 10 + lsn: 11 type: REPLACE BODY: tuple: ['ёёё', 123] - HEADER: - lsn: 12 + lsn: 13 type: INSERT BODY: tuple: ['ююю', 789] - HEADER: - lsn: 11 + lsn: 12 type: INSERT BODY: tuple: ['ЮЮЮ', 456] @@ -310,11 +313,11 @@ result - - HEADER: type: RUNINFO BODY: - min_lsn: 7 + min_lsn: 8 max_key: [null, 'ЭЭЭ'] page_count: 1 bloom_filter: - max_lsn: 9 + max_lsn: 10 min_key: [null, 'ёёё'] - HEADER: type: PAGEINFO @@ -327,17 +330,17 @@ result min_key: [null, 'ёёё'] - - 00000000000000000006.run - - HEADER: - lsn: 9 + lsn: 10 type: INSERT BODY: tuple: [null, 'ёёё'] - HEADER: - lsn: 8 + lsn: 9 type: INSERT BODY: tuple: [null, 'эээ'] - HEADER: - lsn: 7 + lsn: 8 type: INSERT BODY: tuple: [null, 'ЭЭЭ'] @@ -349,11 +352,11 @@ result - - HEADER: type: RUNINFO BODY: - min_lsn: 10 + min_lsn: 11 max_key: [789, 'ююю'] page_count: 1 bloom_filter: - max_lsn: 12 + max_lsn: 13 min_key: [null, 'ёёё'] - HEADER: type: PAGEINFO @@ -366,22 +369,22 @@ result min_key: [null, 'ёёё'] - - 00000000000000000010.run - - HEADER: - lsn: 10 + lsn: 11 type: DELETE BODY: key: [null, 'ёёё'] - HEADER: - lsn: 10 + lsn: 11 type: REPLACE BODY: tuple: [123, 'ёёё'] - HEADER: - lsn: 11 + lsn: 12 type: INSERT BODY: tuple: [456, 'ЮЮЮ'] - HEADER: - lsn: 12 + lsn: 13 type: INSERT BODY: tuple: [789, 'ююю'] diff --git a/test/vinyl/layout.test.lua b/test/vinyl/layout.test.lua index fd5787b7..27bf5d40 100644 --- a/test/vinyl/layout.test.lua +++ b/test/vinyl/layout.test.lua @@ -8,13 +8,15 @@ fun = require 'fun' space = box.schema.space.create('test', {engine='vinyl'}) _ = space:create_index('pk', {parts = {{1, 'string', collation = 'unicode'}}, run_count_per_level=3}) -_ = space:create_index('sk', {parts = {{2, 'unsigned', is_nullable = true}}, run_count_per_level=3}) +_ = space:create_index('sk', {parts = {{2, 'unsigned'}}, run_count_per_level=3}) -- Empty run space:insert{'ЁЁЁ', 777} space:delete{'ЁЁЁ'} box.snapshot() +space.index.sk:alter{parts = {{2, 'unsigned', is_nullable = true}}} + space:replace{'ЭЭЭ', box.NULL} space:replace{'эээ', box.NULL} space:replace{'ёёё', box.NULL}