[PATCH 2/2] alter: do not require index rebuild to clear uniqueness
Vladimir Davydov
vdavydov.dev at gmail.com
Mon Feb 5 16:25:41 MSK 2018
Obviously, there's no point in rebuilding an index if all we do is
relaxing the uniqueness property. This will also allow us to clear
the uniqueness flag for vinyl indexes, which do not support rebuild.
Note, a memtx tree index stores a pointer to either cmp_def or key_def
depending on whether the index is unique. Hence to clear the uniqueness
flag without rebuilding the index, we need to update this pointer. To do
that, we add a new index virtual method, update_def.
Closes #2449
---
src/box/alter.cc | 6 ++++--
src/box/index.cc | 5 +++++
src/box/index.h | 12 ++++++++++++
src/box/index_def.c | 16 +---------------
src/box/index_def.h | 3 ---
src/box/key_def.cc | 17 -----------------
src/box/key_def.h | 4 ----
src/box/memtx_bitset.c | 1 +
src/box/memtx_hash.c | 1 +
src/box/memtx_rtree.c | 1 +
src/box/memtx_tree.c | 8 ++++++++
src/box/sysview_index.c | 1 +
src/box/vinyl.c | 1 +
test/vinyl/ddl.result | 41 +++++++++++++++++++++++++++++++++++++++++
test/vinyl/ddl.test.lua | 13 +++++++++++++
15 files changed, 89 insertions(+), 41 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 095131e2..c48e89f9 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1113,7 +1113,8 @@ ModifyIndex::alter(struct alter_space *alter)
struct index *new_index = space_index(alter->new_space,
new_index_def->iid);
assert(new_index != NULL);
- index_def_swap(old_index->def, new_index->def);
+ SWAP(old_index->def, new_index->def);
+ index_update_def(new_index);
}
void
@@ -1131,7 +1132,8 @@ ModifyIndex::rollback(struct alter_space *alter)
struct index *new_index = space_index(alter->new_space,
new_index_def->iid);
assert(new_index != NULL);
- index_def_swap(old_index->def, new_index->def);
+ SWAP(old_index->def, new_index->def);
+ index_update_def(old_index);
}
ModifyIndex::~ModifyIndex()
diff --git a/src/box/index.cc b/src/box/index.cc
index d1245836..69fc7611 100644
--- a/src/box/index.cc
+++ b/src/box/index.cc
@@ -545,6 +545,11 @@ generic_index_commit_drop(struct index *)
{
}
+void
+generic_index_update_def(struct index *)
+{
+}
+
ssize_t
generic_index_size(struct index *index)
{
diff --git a/src/box/index.h b/src/box/index.h
index 54a27f6f..74b1c9ae 100644
--- a/src/box/index.h
+++ b/src/box/index.h
@@ -341,6 +341,11 @@ struct index_vtab {
* Must not fail.
*/
void (*commit_drop)(struct index *);
+ /**
+ * Called after index definition update that did not
+ * require index rebuild.
+ */
+ void (*update_def)(struct index *);
ssize_t (*size)(struct index *);
ssize_t (*bsize)(struct index *);
@@ -453,6 +458,12 @@ index_commit_drop(struct index *index)
index->vtab->commit_drop(index);
}
+static inline void
+index_update_def(struct index *index)
+{
+ index->vtab->update_def(index);
+}
+
static inline ssize_t
index_size(struct index *index)
{
@@ -555,6 +566,7 @@ index_end_build(struct index *index)
*/
void generic_index_commit_create(struct index *, int64_t);
void generic_index_commit_drop(struct index *);
+void generic_index_update_def(struct index *);
ssize_t generic_index_size(struct index *);
int generic_index_min(struct index *, const char *, uint32_t, struct tuple **);
int generic_index_max(struct index *, const char *, uint32_t, struct tuple **);
diff --git a/src/box/index_def.c b/src/box/index_def.c
index 2c857295..c503c787 100644
--- a/src/box/index_def.c
+++ b/src/box/index_def.c
@@ -136,20 +136,6 @@ index_def_dup(const struct index_def *def)
return dup;
}
-void
-index_def_swap(struct index_def *def1, struct index_def *def2)
-{
- /*
- * Swap const-size items and name. Keep the original key
- * definitions, they are used in the engines.
- */
- struct index_def tmp_def = *def1;
- memcpy(def1, def2, offsetof(struct index_def, key_def));
- memcpy(def2, &tmp_def, offsetof(struct index_def, key_def));
- key_def_swap(def1->key_def, def2->key_def);
- key_def_swap(def1->cmp_def, def2->cmp_def);
-}
-
/** Free a key definition. */
void
index_def_delete(struct index_def *index_def)
@@ -176,7 +162,7 @@ index_def_change_requires_rebuild(const struct index_def *old_index_def,
{
if (old_index_def->iid != new_index_def->iid ||
old_index_def->type != new_index_def->type ||
- old_index_def->opts.is_unique != new_index_def->opts.is_unique ||
+ (!old_index_def->opts.is_unique && new_index_def->opts.is_unique) ||
!key_part_check_compatibility(old_index_def->key_def->parts,
old_index_def->key_def->part_count,
new_index_def->key_def->parts,
diff --git a/src/box/index_def.h b/src/box/index_def.h
index cb191e7a..18f94136 100644
--- a/src/box/index_def.h
+++ b/src/box/index_def.h
@@ -159,9 +159,6 @@ struct index_def {
struct index_def *
index_def_dup(const struct index_def *def);
-void
-index_def_swap(struct index_def *def1, struct index_def *def2);
-
/* Destroy and free an index_def. */
void
index_def_delete(struct index_def *def);
diff --git a/src/box/key_def.cc b/src/box/key_def.cc
index 3aad8bf0..f7f6c753 100644
--- a/src/box/key_def.cc
+++ b/src/box/key_def.cc
@@ -528,23 +528,6 @@ key_def_merge(const struct key_def *first, const struct key_def *second)
return new_def;
}
-void
-key_def_swap(struct key_def *a, struct key_def *b)
-{
- assert(a->part_count == b->part_count);
- char buf[sizeof(*a)];
- /* Swap fixed members. */
- memcpy(buf, a, sizeof(*a));
- memcpy(a, b, sizeof(*a));
- memcpy(b, buf, sizeof(*a));
- /* Swap parts. */
- for (uint32_t i = 0; i < a->part_count; ++i) {
- struct key_part tmp = a->parts[i];
- a->parts[i] = b->parts[i];
- b->parts[i] = tmp;
- }
-}
-
int
key_validate_parts(const struct key_def *key_def, const char *key,
uint32_t part_count, bool allow_nullable)
diff --git a/src/box/key_def.h b/src/box/key_def.h
index 83af70df..201863f5 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -271,10 +271,6 @@ key_def_find(const struct key_def *key_def, uint32_t fieldno);
struct key_def *
key_def_merge(const struct key_def *first, const struct key_def *second);
-/** Swap content of key defs. */
-void
-key_def_swap(struct key_def *a, struct key_def *b);
-
/*
* Check that parts of the key match with the key definition.
* @param key_def Key definition.
diff --git a/src/box/memtx_bitset.c b/src/box/memtx_bitset.c
index db7fa1f0..9216ed86 100644
--- a/src/box/memtx_bitset.c
+++ b/src/box/memtx_bitset.c
@@ -458,6 +458,7 @@ static const struct index_vtab memtx_bitset_index_vtab = {
/* .destroy = */ memtx_bitset_index_destroy,
/* .commit_create = */ generic_index_commit_create,
/* .commit_drop = */ generic_index_commit_drop,
+ /* .update_def = */ generic_index_update_def,
/* .size = */ memtx_bitset_index_size,
/* .bsize = */ memtx_bitset_index_bsize,
/* .min = */ generic_index_min,
diff --git a/src/box/memtx_hash.c b/src/box/memtx_hash.c
index 3a37293e..78f55eb7 100644
--- a/src/box/memtx_hash.c
+++ b/src/box/memtx_hash.c
@@ -376,6 +376,7 @@ static const struct index_vtab memtx_hash_index_vtab = {
/* .destroy = */ memtx_hash_index_destroy,
/* .commit_create = */ generic_index_commit_create,
/* .commit_drop = */ generic_index_commit_drop,
+ /* .update_def = */ generic_index_update_def,
/* .size = */ memtx_hash_index_size,
/* .bsize = */ memtx_hash_index_bsize,
/* .min = */ generic_index_min,
diff --git a/src/box/memtx_rtree.c b/src/box/memtx_rtree.c
index d52bd4d7..74c1c370 100644
--- a/src/box/memtx_rtree.c
+++ b/src/box/memtx_rtree.c
@@ -296,6 +296,7 @@ static const struct index_vtab memtx_rtree_index_vtab = {
/* .destroy = */ memtx_rtree_index_destroy,
/* .commit_create = */ generic_index_commit_create,
/* .commit_drop = */ generic_index_commit_drop,
+ /* .update_def = */ generic_index_update_def,
/* .size = */ memtx_rtree_index_size,
/* .bsize = */ memtx_rtree_index_bsize,
/* .min = */ generic_index_min,
diff --git a/src/box/memtx_tree.c b/src/box/memtx_tree.c
index 196a5364..6ebdc90f 100644
--- a/src/box/memtx_tree.c
+++ b/src/box/memtx_tree.c
@@ -308,6 +308,13 @@ memtx_tree_index_destroy(struct index *base)
free(index);
}
+static void
+memtx_tree_index_update_def(struct index *base)
+{
+ struct memtx_tree_index *index = (struct memtx_tree_index *)base;
+ index->tree.arg = memtx_tree_index_cmp_def(index);
+}
+
static ssize_t
memtx_tree_index_size(struct index *base)
{
@@ -575,6 +582,7 @@ static const struct index_vtab memtx_tree_index_vtab = {
/* .destroy = */ memtx_tree_index_destroy,
/* .commit_create = */ generic_index_commit_create,
/* .commit_drop = */ generic_index_commit_drop,
+ /* .update_def = */ memtx_tree_index_update_def,
/* .size = */ memtx_tree_index_size,
/* .bsize = */ memtx_tree_index_bsize,
/* .min = */ generic_index_min,
diff --git a/src/box/sysview_index.c b/src/box/sysview_index.c
index a76a67e4..0bec3028 100644
--- a/src/box/sysview_index.c
+++ b/src/box/sysview_index.c
@@ -162,6 +162,7 @@ static const struct index_vtab sysview_index_vtab = {
/* .destroy = */ sysview_index_destroy,
/* .commit_create = */ generic_index_commit_create,
/* .commit_drop = */ generic_index_commit_drop,
+ /* .update_def = */ generic_index_update_def,
/* .size = */ generic_index_size,
/* .bsize = */ sysview_index_bsize,
/* .min = */ generic_index_min,
diff --git a/src/box/vinyl.c b/src/box/vinyl.c
index b2331081..e5fc9337 100644
--- a/src/box/vinyl.c
+++ b/src/box/vinyl.c
@@ -3993,6 +3993,7 @@ static const struct index_vtab vinyl_index_vtab = {
/* .destroy = */ vinyl_index_destroy,
/* .commit_create = */ vinyl_index_commit_create,
/* .commit_drop = */ vinyl_index_commit_drop,
+ /* .update_def = */ generic_index_update_def,
/* .size = */ generic_index_size,
/* .bsize = */ vinyl_index_bsize,
/* .min = */ generic_index_min,
diff --git a/test/vinyl/ddl.result b/test/vinyl/ddl.result
index a0c41a1d..9fa3504c 100644
--- a/test/vinyl/ddl.result
+++ b/test/vinyl/ddl.result
@@ -716,3 +716,44 @@ box.space.test.index.pk
box.space.test:drop()
---
...
+-- gh-2449 change 'unique' index property from true to false
+s = box.schema.space.create('test', { engine = 'vinyl' })
+---
+...
+_ = s:create_index('primary')
+---
+...
+_ = s:create_index('secondary', {unique = true, parts = {2, 'unsigned'}})
+---
+...
+s:insert{1, 10}
+---
+- [1, 10]
+...
+s.index.secondary:alter{unique = false} -- ok
+---
+...
+s.index.secondary.unique
+---
+- false
+...
+s.index.secondary:alter{unique = true} -- error
+---
+- error: Vinyl does not support changing the definition of a non-empty index
+...
+s.index.secondary.unique
+---
+- false
+...
+s:insert{2, 10}
+---
+- [2, 10]
+...
+s.index.secondary:select(10)
+---
+- - [1, 10]
+ - [2, 10]
+...
+s:drop()
+---
+...
diff --git a/test/vinyl/ddl.test.lua b/test/vinyl/ddl.test.lua
index 1761c430..bb9e5596 100644
--- a/test/vinyl/ddl.test.lua
+++ b/test/vinyl/ddl.test.lua
@@ -269,3 +269,16 @@ box.space._space:insert{512, 1, 'test', 'vinyl', 0, setmetatable({}, {__serializ
box.space._index:insert{512, 0, 'pk', 'tree', {unique = true}, {{0, 'unsigned'}}}
box.space.test.index.pk
box.space.test:drop()
+
+-- gh-2449 change 'unique' index property from true to false
+s = box.schema.space.create('test', { engine = 'vinyl' })
+_ = s:create_index('primary')
+_ = s:create_index('secondary', {unique = true, parts = {2, 'unsigned'}})
+s:insert{1, 10}
+s.index.secondary:alter{unique = false} -- ok
+s.index.secondary.unique
+s.index.secondary:alter{unique = true} -- error
+s.index.secondary.unique
+s:insert{2, 10}
+s.index.secondary:select(10)
+s:drop()
--
2.11.0
More information about the Tarantool-patches
mailing list