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