[Tarantool-patches] [PATCH] box: don't allow referenced collation to be dropped
Nikita Pettik
korablev at tarantool.org
Mon Nov 11 16:52:46 MSK 2019
This patch fixes one debt related to collations: collation is not allowed
to be dropped until at least one space format or index references to it.
Otherwise, we may face situation when collation doesn't exist anymore,
but space has references to it (in-memory DD objects are not updated on
collation alter). To achieve this we simply iterate over all spaces and
check their formats and indexes. Despite it takes
O(space_count * (field_count + index_count)) complexity, it is not a big
deal: drop of collation is considered to be rare operation. So instead
of maintaining reference counters we'd better once spend a bit more
time during drop.
Closes #4544
---
Branch: https://github.com/tarantool/tarantool/commits/np/gh-4544-drop-collation
Issue: https://github.com/tarantool/tarantool/issues/4544
src/box/alter.cc | 48 +++++++++++++++++++++++++++++++++++++++++++--
test/box/net.box.result | 4 ++--
test/box/net.box.test.lua | 2 +-
test/sql/collation.result | 33 ++++++++++++++++++++++++++++++-
test/sql/collation.test.lua | 11 +++++++++++
5 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 697b7dbf3..f4232ce34 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -3228,6 +3228,48 @@ on_drop_collation_rollback(struct trigger *trigger, void *event)
return 0;
}
+/**
+ * Check if collation is referenced by any space. If so return -1.
+ * If collation is mentioned in no one space or index definition,
+ * return 0.
+ */
+static int
+space_check_coll_entry(struct space *space, void *data)
+{
+ uint32_t coll_id = *(uint32_t *) data;
+ for (uint32_t i = 0; i < space->index_count; ++i) {
+ struct index *idx = space->index[i];
+ for (uint32_t j = 0; j < idx->def->key_def->part_count; ++j) {
+ struct key_part kp = idx->def->key_def->parts[j];
+ if (kp.coll_id == coll_id) {
+ char *id_str = tt_static_buf();
+ sprintf(id_str, "%u", coll_id);
+ char *msg = tt_static_buf();
+ sprintf(msg, "index %d of space %s has "
+ "references to collation",
+ idx->def->iid, space->def->name);
+ diag_set(ClientError, ER_DROP_COLLATION, id_str,
+ msg);
+ return -1;
+ }
+ }
+ for (uint32_t j = 0; j < space->def->field_count; ++j) {
+ if (space->def->fields[j].coll_id == coll_id) {
+ char *id_str = tt_static_buf();
+ sprintf(id_str, "%u", coll_id);
+ char *msg = tt_static_buf();
+ sprintf(msg, "format of space %s has "
+ "references to collation",
+ space->def->name);
+ diag_set(ClientError, ER_DROP_COLLATION, id_str,
+ msg);
+ return -1;
+ }
+ }
+ }
+ return 0;
+}
+
/**
* A trigger invoked on replace in a space containing
* collations that a user defined.
@@ -3248,11 +3290,13 @@ on_replace_dd_collation(struct trigger * /* trigger */, void *event)
if (on_commit == NULL || on_rollback == NULL)
return -1;
/*
- * TODO: Check that no index uses the collation
- * identifier.
+ * Check that no index or space format uses the
+ * collation identifier.
*/
int32_t old_id = tuple_field_u32_xc(old_tuple,
BOX_COLLATION_FIELD_ID);
+ if (space_foreach(space_check_coll_entry, &old_id) != 0)
+ return -1;
/*
* Don't allow user to drop "none" collation
* since it is very special and vastly used
diff --git a/test/box/net.box.result b/test/box/net.box.result
index e3dabf7d9..dcb7e03f4 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -2944,10 +2944,10 @@ end
c:close()
---
...
-box.internal.collation.drop('test')
+space:drop()
---
...
-space:drop()
+box.internal.collation.drop('test')
---
...
c.state
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 8e65ff470..d4fe32e17 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1209,8 +1209,8 @@ else \
return parts[1].collation_id == collation_id \
end
c:close()
-box.internal.collation.drop('test')
space:drop()
+box.internal.collation.drop('test')
c.state
c = nil
diff --git a/test/sql/collation.result b/test/sql/collation.result
index 11962ef47..e77712636 100644
--- a/test/sql/collation.result
+++ b/test/sql/collation.result
@@ -222,7 +222,7 @@ box.space._collation:select{0}
...
box.space._collation:delete{0}
---
-- error: 'Can''t drop collation none : system collation'
+- error: 'Can''t drop collation 0 : index 0 of space _schema has references to collation'
...
-- gh-3185: collations of LHS and RHS must be compatible.
--
@@ -1236,3 +1236,34 @@ s:select{}
s:drop()
---
...
+-- gh-4544: make sure that collation can't be dropped until it
+-- is referenced by space or index.
+--
+box.internal.collation.create('c', 'ICU', 'unicode')
+---
+...
+coll_id = box.space._collation.index.name:get({'c'})[1]
+---
+...
+box.execute([[CREATE TABLE things (id STRING COLLATE "unicode" PRIMARY KEY, a STRING COLLATE "c");]])
+---
+- row_count: 1
+...
+box.space._collation:delete(1)
+---
+- error: 'Can''t drop collation 1 : index 0 of space THINGS has references to collation'
+...
+box.space._collation:delete(coll_id)
+---
+- error: 'Can''t drop collation 277 : format of space THINGS has references to collation'
+...
+box.internal.collation.drop('c')
+---
+- error: 'Can''t drop collation 277 : format of space THINGS has references to collation'
+...
+box.space.THINGS:drop()
+---
+...
+box.internal.collation.drop('c')
+---
+...
diff --git a/test/sql/collation.test.lua b/test/sql/collation.test.lua
index 1be28b3ff..70b0c9622 100644
--- a/test/sql/collation.test.lua
+++ b/test/sql/collation.test.lua
@@ -343,3 +343,14 @@ s:insert{'ё'}
s:select{}
s:drop()
+-- gh-4544: make sure that collation can't be dropped until it
+-- is referenced by space or index.
+--
+box.internal.collation.create('c', 'ICU', 'unicode')
+coll_id = box.space._collation.index.name:get({'c'})[1]
+box.execute([[CREATE TABLE things (id STRING COLLATE "unicode" PRIMARY KEY, a STRING COLLATE "c");]])
+box.space._collation:delete(1)
+box.space._collation:delete(coll_id)
+box.internal.collation.drop('c')
+box.space.THINGS:drop()
+box.internal.collation.drop('c')
--
2.15.1
More information about the Tarantool-patches
mailing list