From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id BC428452566 for ; Mon, 11 Nov 2019 16:52:54 +0300 (MSK) From: Nikita Pettik Date: Mon, 11 Nov 2019 16:52:46 +0300 Message-Id: <20191111135246.79818-1-korablev@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] box: don't allow referenced collation to be dropped List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Cc: v.shpilevoy@tarantool.org 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