[Tarantool-patches] [PATCH 1/1] box: introduce space:alter()

[Vladislav Shpilevoy <v.shpilevoy@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)