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 B73A1430407 for ; Sat, 22 Aug 2020 01:46:04 +0300 (MSK) From: Vladislav Shpilevoy Date: Sat, 22 Aug 2020 00:46:02 +0200 Message-Id: <46c58e05328a4b9e85d6837fe922e308bd120ad4.1598049870.git.v.shpilevoy@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 1/1] box: introduce space:alter() List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com, sergepetrenko@tarantool.org There was no way to change certain space parameters without its recreation or manual update of internal system space _space. Even if some of them were legal to update: field_count, owner, flag of being temporary, is_sync flag. The patch introduces function space:alter(), which accepts a subset of parameters from box.schema.space.create which are mutable, and 'name' parameter. There is a method space:rename(), but still the parameter is added to space:alter() too, to be consistent with index:alter(), which also accepts a new name. Closes #5155 @TarantoolBot document Title: New function space:alter(options) Space objects in Lua (stored in `box.space` table) now have a new method: `space:alter(options)`. The method accepts a table with parameters `field_count`, `user`, `format`, `temporary`, `is_sync`, and `name`. All parameters have the same meaning as in `box.schema.space.create(name, options)`. Note, `name` parameter in `box.schema.space.create` is separated from `options` table. It is not so in `space:alter(options)` - here all parameters are specified in the `options` table. The function does not return anything in case of success, and throws an error when fails. >From 'Synchronous replication' page, from 'Limitations and known problems' it is necessary to delete the note about "no way to enable synchronous replication for existing spaces". Instead it is necessary to say, that it can be enabled using `space:alter({is_sync = true})`. And can be disabled by setting `is_sync = false`. https://www.tarantool.io/en/doc/2.5/book/replication/repl_sync/#limitations-and-known-problems The function will appear in >= 2.5.2. --- Branch: http://github.com/tarantool/tarantool/tree/gerold103/gh-5155-space-alter Issue: https://github.com/tarantool/tarantool/issues/5155 @ChangeLog * New function `space:alter(options)` to change some space settings without recreation nor touching `_space` space. src/box/lua/schema.lua | 63 +++++++ test/box/alter.result | 378 ++++++++++++++++++++++++++++++++++++++++ test/box/alter.test.lua | 142 +++++++++++++++ 3 files changed, 583 insertions(+) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 3c3fa3ef8..7fe1ea7a2 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -539,6 +539,64 @@ box.schema.space.rename = function(space_id, space_name) _space:update(space_id, {{"=", 3, space_name}}) end +local alter_space_template = { + field_count = 'number', + user = 'string, number', + format = 'table', + temporary = 'boolean', + is_sync = 'boolean', + name = 'string', +} + +box.schema.space.alter = function(space_id, options) + local space = box.space[space_id] + if not space then + box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id)) + end + check_param_table(options, alter_space_template) + + local _space = box.space._space + local tuple = _space:get({space.id}) + assert(tuple ~= nil) + + local owner + if options.user then + owner = user_or_role_resolve(options.user) + if not owner then + box.error(box.error.NO_SUCH_USER, options.user) + end + else + owner = tuple.owner + end + + local name = options.name or tuple.name + local field_count = options.field_count or tuple.field_count + local flags = tuple.flags + + if options.temporary ~= nil then + flags.temporary = options.temporary + end + + if options.is_sync ~= nil then + flags.is_sync = options.is_sync + end + + local format + if options.format ~= nil then + format = update_format(options.format) + else + format = tuple.format + end + + tuple = tuple:totable() + tuple[2] = owner + tuple[3] = name + tuple[5] = field_count + tuple[6] = flags + tuple[7] = format + _space:replace(tuple) +end + box.schema.index = {} local function update_index_parts_1_6_0(parts) @@ -1718,6 +1776,11 @@ space_mt.rename = function(space, name) check_space_exists(space) return box.schema.space.rename(space.id, name) end +space_mt.alter = function(space, options) + check_space_arg(space, 'alter') + check_space_exists(space) + return box.schema.space.alter(space.id, options) +end space_mt.create_index = function(space, name, options) check_space_arg(space, 'create_index') check_space_exists(space) diff --git a/test/box/alter.result b/test/box/alter.result index f150faac1..237c2d8aa 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -1181,3 +1181,381 @@ s:drop() box.internal.collation.drop('test') --- ... +-- +-- gh-5155: space:alter(), added in order to make it possible to turn on/off +-- is_sync, but it is handy for other options too. +-- +s = box.schema.create_space('test') +--- +... +pk = s:create_index('pk') +--- +... +-- Bad usage. +s:alter({unknown = true}) +--- +- error: Illegal parameters, unexpected option 'unknown' +... +ok, err = pcall(s.alter, {}) +--- +... +assert(err:match("Use space:alter") ~= nil) +--- +- true +... +-- Alter field_count. +s:replace{1} +--- +- [1] +... +-- Can't update on non-empty space. +s:alter({field_count = 2}) +--- +- error: Tuple field count 1 does not match space field count 2 +... +s:delete{1} +--- +- [1] +... +-- Can update on empty space. +s:alter({field_count = 2}) +--- +... +s:replace{1} +--- +- error: Tuple field count 1 does not match space field count 2 +... +s:replace{1, 1} +--- +- [1, 1] +... +-- When not specified or nil - ignored. +s:alter({field_count = nil}) +--- +... +s:replace{2} +--- +- error: Tuple field count 1 does not match space field count 2 +... +s:replace{2, 2} +--- +- [2, 2] +... +-- Set to 0 drops the restriction. +s:alter({field_count = 0}) +--- +... +s:truncate() +--- +... +s:replace{1} +--- +- [1] +... +s:delete{1} +--- +- [1] +... +-- Invalid values. +s:alter({field_count = box.NULL}) +--- +- error: Illegal parameters, options parameter 'field_count' should be of type number +... +s:alter({field_count = 'string'}) +--- +- error: Illegal parameters, options parameter 'field_count' should be of type number +... +-- Alter owner. +owner1 = box.space._space:get{s.id}.owner +--- +... +box.schema.user.create('test') +--- +... +-- When not specified or nil - ignored. +s:alter({user = nil}) +--- +... +owner2 = box.space._space:get{s.id}.owner +--- +... +assert(owner2 == owner1) +--- +- true +... +s:alter({user = 'test'}) +--- +... +owner2 = box.space._space:get{s.id}.owner +--- +... +assert(owner2 ~= owner1) +--- +- true +... +s:alter({user = owner1}) +--- +... +owner2 = box.space._space:get{s.id}.owner +--- +... +assert(owner2 == owner1) +--- +- true +... +box.schema.user.drop('test') +--- +... +-- Invalid values. +s:alter({user = box.NULL}) +--- +- error: 'Illegal parameters, options parameter ''user'' should be one of types: string, + number' +... +s:alter({user = true}) +--- +- error: 'Illegal parameters, options parameter ''user'' should be one of types: string, + number' +... +s:alter({user = 'not_existing'}) +--- +- error: User 'not_existing' is not found +... +-- Alter format. +format = {{name = 'field1', type = 'unsigned'}} +--- +... +s:alter({format = format}) +--- +... +s:format() +--- +- [{'name': 'field1', 'type': 'unsigned'}] +... +-- When not specified or nil - ignored. +s:alter({format = nil}) +--- +... +s:format() +--- +- [{'name': 'field1', 'type': 'unsigned'}] +... +t = s:replace{1} +--- +... +assert(t.field1 == 1) +--- +- true +... +s:alter({format = {}}) +--- +... +assert(t.field1 == nil) +--- +- true +... +s:delete{1} +--- +- [1] +... +-- Invalid values. +s:alter({format = box.NULL}) +--- +- error: Illegal parameters, options parameter 'format' should be of type table +... +s:alter({format = true}) +--- +- error: Illegal parameters, options parameter 'format' should be of type table +... +s:alter({format = {{{1, 2, 3, 4}}}}) +--- +- error: 'Illegal parameters, format[1]: name (string) is expected' +... +-- +-- Alter temporary. +-- +s:alter({temporary = true}) +--- +... +assert(s.temporary) +--- +- true +... +-- When not specified or nil - ignored. +s:alter({temporary = nil}) +--- +... +assert(s.temporary) +--- +- true +... +s:alter({temporary = false}) +--- +... +assert(not s.temporary) +--- +- true +... +-- Ensure absence is not treated like 'true'. +s:alter({temporary = nil}) +--- +... +assert(not s.temporary) +--- +- true +... +-- Invalid values. +s:alter({temporary = box.NULL}) +--- +- error: Illegal parameters, options parameter 'temporary' should be of type boolean +... +s:alter({temporary = 100}) +--- +- error: Illegal parameters, options parameter 'temporary' should be of type boolean +... +-- +-- Alter is_sync. +-- +old_synchro_quorum = box.cfg.replication_synchro_quorum +--- +... +old_synchro_timeout = box.cfg.replication_synchro_timeout +--- +... +box.cfg{ \ + replication_synchro_quorum = 2, \ + replication_synchro_timeout = 0.001, \ +} +--- +... +s:alter({is_sync = true}) +--- +... +assert(s.is_sync) +--- +- true +... +s:replace{1} +--- +- error: Quorum collection for a synchronous transaction is timed out +... +-- When not specified or nil - ignored. +s:alter({is_sync = nil}) +--- +... +assert(s.is_sync) +--- +- true +... +s:alter({is_sync = false}) +--- +... +assert(not s.is_sync) +--- +- true +... +-- Ensure absence is not treated like 'true'. +s:alter({is_sync = nil}) +--- +... +assert(not s.is_sync) +--- +- true +... +-- Invalid values. +s:alter({is_sync = box.NULL}) +--- +- error: Illegal parameters, options parameter 'is_sync' should be of type boolean +... +s:alter({is_sync = 100}) +--- +- error: Illegal parameters, options parameter 'is_sync' should be of type boolean +... +s:replace{1} +--- +- [1] +... +s:delete{1} +--- +- [1] +... +box.cfg{ \ + replication_synchro_quorum = old_synchro_quorum, \ + replication_synchro_timeout = old_synchro_timeout, \ +} +--- +... +-- Alter name. +s:alter({name = 'test2'}) +--- +... +assert(box.space.test2 ~= nil) +--- +- true +... +assert(box.space.test == nil) +--- +- true +... +assert(s.name == 'test2') +--- +- true +... +-- When not specified or nil - ignored. +s:alter({name = nil}) +--- +... +assert(box.space.test2 ~= nil) +--- +- true +... +assert(box.space.test == nil) +--- +- true +... +assert(s.name == 'test2') +--- +- true +... +s:alter({name = '_space'}) +--- +- error: Duplicate key exists in unique index 'name' in space '_space' +... +s:alter({name = 'test'}) +--- +... +assert(box.space.test ~= nil) +--- +- true +... +assert(box.space.test2 == nil) +--- +- true +... +assert(s.name == 'test') +--- +- true +... +-- Invalid values. +s:alter({name = box.NULL}) +--- +- error: Illegal parameters, options parameter 'name' should be of type string +... +s:alter({name = 100}) +--- +- error: Illegal parameters, options parameter 'name' should be of type string +... +s:drop() +--- +... +s:alter({}) +--- +- error: Space 'test' does not exist +... +ok, err = pcall(box.schema.space.alter, s.id, {}) +--- +... +assert(err:match('does not exist') ~= nil) +--- +- true +... diff --git a/test/box/alter.test.lua b/test/box/alter.test.lua index 3cb0c4f84..abd08e2fa 100644 --- a/test/box/alter.test.lua +++ b/test/box/alter.test.lua @@ -478,3 +478,145 @@ validate_indexes() s:drop() box.internal.collation.drop('test') + +-- +-- gh-5155: space:alter(), added in order to make it possible to turn on/off +-- is_sync, but it is handy for other options too. +-- +s = box.schema.create_space('test') +pk = s:create_index('pk') + +-- Bad usage. +s:alter({unknown = true}) +ok, err = pcall(s.alter, {}) +assert(err:match("Use space:alter") ~= nil) + +-- Alter field_count. +s:replace{1} +-- Can't update on non-empty space. +s:alter({field_count = 2}) +s:delete{1} +-- Can update on empty space. +s:alter({field_count = 2}) +s:replace{1} +s:replace{1, 1} +-- When not specified or nil - ignored. +s:alter({field_count = nil}) +s:replace{2} +s:replace{2, 2} +-- Set to 0 drops the restriction. +s:alter({field_count = 0}) +s:truncate() +s:replace{1} +s:delete{1} +-- Invalid values. +s:alter({field_count = box.NULL}) +s:alter({field_count = 'string'}) + +-- Alter owner. +owner1 = box.space._space:get{s.id}.owner +box.schema.user.create('test') +-- When not specified or nil - ignored. +s:alter({user = nil}) +owner2 = box.space._space:get{s.id}.owner +assert(owner2 == owner1) +s:alter({user = 'test'}) +owner2 = box.space._space:get{s.id}.owner +assert(owner2 ~= owner1) +s:alter({user = owner1}) +owner2 = box.space._space:get{s.id}.owner +assert(owner2 == owner1) +box.schema.user.drop('test') +-- Invalid values. +s:alter({user = box.NULL}) +s:alter({user = true}) +s:alter({user = 'not_existing'}) + +-- Alter format. +format = {{name = 'field1', type = 'unsigned'}} +s:alter({format = format}) +s:format() +-- When not specified or nil - ignored. +s:alter({format = nil}) +s:format() +t = s:replace{1} +assert(t.field1 == 1) +s:alter({format = {}}) +assert(t.field1 == nil) +s:delete{1} +-- Invalid values. +s:alter({format = box.NULL}) +s:alter({format = true}) +s:alter({format = {{{1, 2, 3, 4}}}}) + +-- +-- Alter temporary. +-- +s:alter({temporary = true}) +assert(s.temporary) +-- When not specified or nil - ignored. +s:alter({temporary = nil}) +assert(s.temporary) +s:alter({temporary = false}) +assert(not s.temporary) +-- Ensure absence is not treated like 'true'. +s:alter({temporary = nil}) +assert(not s.temporary) +-- Invalid values. +s:alter({temporary = box.NULL}) +s:alter({temporary = 100}) + +-- +-- Alter is_sync. +-- +old_synchro_quorum = box.cfg.replication_synchro_quorum +old_synchro_timeout = box.cfg.replication_synchro_timeout +box.cfg{ \ + replication_synchro_quorum = 2, \ + replication_synchro_timeout = 0.001, \ +} +s:alter({is_sync = true}) +assert(s.is_sync) +s:replace{1} +-- When not specified or nil - ignored. +s:alter({is_sync = nil}) +assert(s.is_sync) +s:alter({is_sync = false}) +assert(not s.is_sync) +-- Ensure absence is not treated like 'true'. +s:alter({is_sync = nil}) +assert(not s.is_sync) +-- Invalid values. +s:alter({is_sync = box.NULL}) +s:alter({is_sync = 100}) +s:replace{1} +s:delete{1} +box.cfg{ \ + replication_synchro_quorum = old_synchro_quorum, \ + replication_synchro_timeout = old_synchro_timeout, \ +} + +-- Alter name. +s:alter({name = 'test2'}) +assert(box.space.test2 ~= nil) +assert(box.space.test == nil) +assert(s.name == 'test2') +-- When not specified or nil - ignored. +s:alter({name = nil}) +assert(box.space.test2 ~= nil) +assert(box.space.test == nil) +assert(s.name == 'test2') +s:alter({name = '_space'}) +s:alter({name = 'test'}) +assert(box.space.test ~= nil) +assert(box.space.test2 == nil) +assert(s.name == 'test') +-- Invalid values. +s:alter({name = box.NULL}) +s:alter({name = 100}) + +s:drop() + +s:alter({}) +ok, err = pcall(box.schema.space.alter, s.id, {}) +assert(err:match('does not exist') ~= nil) -- 2.21.1 (Apple Git-122.3)