diff --git a/changelogs/unreleased/gh-10095-vy-index-update-def-fix.md b/changelogs/unreleased/gh-10095-vy-index-update-def-fix.md new file mode 100644 index 0000000000000000000000000000000000000000..7a40071c99a8ee86d9cb4f4b56e20e50c6b60e31 --- /dev/null +++ b/changelogs/unreleased/gh-10095-vy-index-update-def-fix.md @@ -0,0 +1,4 @@ +## bugfix/vinyl + +* Fixed a bug when a DDL operation crashed in case of extending the key parts + of a secondary index with the primary index key parts (gh-10095). diff --git a/src/box/vinyl.c b/src/box/vinyl.c index cd76d065aa2711dfd95211ca9197ec2a9e4a82c6..5ac3d384d330ee643bc5e4759ec89dac0c5a05a8 100644 --- a/src/box/vinyl.c +++ b/src/box/vinyl.c @@ -929,6 +929,11 @@ vinyl_index_update_def(struct index *index) { struct vy_lsm *lsm = vy_lsm(index); lsm->opts = index->def->opts; + /* + * Sic: We copy key definitions in-place instead of reallocating them + * because they may be used by read iterators by pointer, for example, + * see vy_run_iterator. + */ key_def_copy(lsm->key_def, index->def->key_def); key_def_copy(lsm->cmp_def, index->def->cmp_def); } @@ -960,7 +965,9 @@ vinyl_index_def_change_requires_rebuild(struct index *index, return true; assert(index_depends_on_pk(index)); + const struct key_def *old_key_def = old_def->key_def; const struct key_def *old_cmp_def = old_def->cmp_def; + const struct key_def *new_key_def = new_def->key_def; const struct key_def *new_cmp_def = new_def->cmp_def; /* @@ -970,8 +977,15 @@ vinyl_index_def_change_requires_rebuild(struct index *index, * won't reveal such statements, but we may still need to * compare them to statements inserted after ALTER hence * we can't narrow field types without index rebuild. + * + * Sic: If secondary index key parts are extended with primary + * index key parts, cmp_def (hence the sorting order) will stay + * the same, but we still have to rebuild the index because + * the new key_def has more parts so we can't update it in-place, + * see vinyl_index_update_def(). */ - if (old_cmp_def->part_count != new_cmp_def->part_count) + if (old_key_def->part_count != new_key_def->part_count || + old_cmp_def->part_count != new_cmp_def->part_count) return true; for (uint32_t i = 0; i < new_cmp_def->part_count; i++) { diff --git a/test/vinyl-luatest/gh_10095_update_index_def_test.lua b/test/vinyl-luatest/gh_10095_update_index_def_test.lua new file mode 100644 index 0000000000000000000000000000000000000000..45c93483ae9af60330d74aa6910bf79887b0d9d3 --- /dev/null +++ b/test/vinyl-luatest/gh_10095_update_index_def_test.lua @@ -0,0 +1,35 @@ +local server = require('luatest.server') +local t = require('luatest') + +local g = t.group() + +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_update_index_def = function(cg) + cg.server:exec(function() + local s = box.schema.create_space('test', {engine = 'vinyl'}) + s:create_index('pk') + s:create_index('sk', {parts = {{2, 'unsigned'}}}) + s:insert({1, 10, 100}) + t.assert_equals(s.index.sk:get({10}), {1, 10, 100}) + s.index.sk:alter({parts = {{2, 'unsigned'}, {1, 'unsigned'}}}) + t.assert_equals(s.index.sk:get({10, 1}), {1, 10, 100}) + s.index.sk:alter({parts = {{2, 'unsigned'}, {3, 'unsigned'}}}) + t.assert_equals(s.index.sk:get({10, 100}), {1, 10, 100}) + end) +end