[PATCH] schema: rework index sequence API

[PATCH] schema: rework index sequence API

[Vladimir Davydov]

Rather than passing 'sequence_part' along with 'sequence' on index
create/alter, pass a table with the following fields:

 - id: sequence id or name
 - field: auto increment field id or name or path in case of json index

If id is omitted, the sequence will be auto-generated (equivalent to
'sequence = true'). If field is omitted, the first indexed field is
used. Old format, i.e. passing false/true or sequence name/id instead of
a table is still supported.

Follow-up #4009
---
https://github.com/tarantool/tarantool/commits/dv/rework-sequence-api

 src/box/lua/schema.lua     | 295 ++++++++++++++++++++++++++++-----------------
 test/box/sequence.result   |  81 ++++++++-----
 test/box/sequence.test.lua |  37 +++---
 3 files changed, 262 insertions(+), 151 deletions(-)

diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index dcaa1365..e69c3f2b 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -593,7 +593,7 @@ end
 -- field 1-based index or full JSON path. A particular case of a
 -- full JSON path is the format field name.
 --
-local function format_field_resolve(format, path, part_idx)
+local function format_field_resolve(format, path, what)
     assert(type(path) == 'number' or type(path) == 'string')
     local idx = nil
     local relative_path = nil
@@ -636,13 +636,12 @@ local function format_field_resolve(format, path, part_idx)
     end
     -- Can't resolve field index by path.
     assert(idx == nil)
-    box.error(box.error.ILLEGAL_PARAMS, "options.parts[" .. part_idx .. "]: " ..
+    box.error(box.error.ILLEGAL_PARAMS, what .. ": " ..
               "field was not found by name '" .. path .. "'")
 
 ::done::
     if idx <= 0 then
-        box.error(box.error.ILLEGAL_PARAMS,
-                  "options.parts[" .. part_idx .. "]: " ..
+        box.error(box.error.ILLEGAL_PARAMS, what .. ": " ..
                   "field (number) must be one-based")
     end
     return idx - 1, relative_path
@@ -696,7 +695,8 @@ local function update_index_parts(format, parts)
             end
         end
         if type(part.field) == 'number' or type(part.field) == 'string' then
-            local idx, path = format_field_resolve(format, part.field, i)
+            local idx, path = format_field_resolve(format, part.field,
+                                                   "options.parts[" .. i .. "]")
             part.field = idx
             part.path = path or part.path
             parts_can_be_simplified = parts_can_be_simplified and part.path == nil
@@ -749,17 +749,180 @@ local function simplify_index_parts(parts)
     return new_parts
 end
 
-local function check_sequence_part(parts, sequence_part,
-                                   index_name, space_name)
-    if sequence_part <= 0 or sequence_part > #parts then
-        box.error(box.error.MODIFY_INDEX, index_name, space_name,
-                  "sequence part is out of bounds")
+--
+-- The first stage of a space sequence modification operation.
+-- Called before altering the space definition. Checks sequence
+-- options and detaches the sequence from the space if required.
+-- Returns a proxy object that is supposed to be passed to
+-- space_sequence_alter_commit() to complete the operation.
+--
+local function space_sequence_alter_prepare(format, parts, options,
+                                            space_id, index_id,
+                                            space_name, index_name)
+    local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
+
+    if options.sequence == nil then
+        -- No sequence option, nothing to do.
+        return nil
     end
-    sequence_part = parts[sequence_part]
-    local sequence_part_type = sequence_part.type or sequence_part[2]
-    if sequence_part_type ~= 'integer' and sequence_part_type ~= 'unsigned' then
+
+    -- A sequence can only be attached to a primary index.
+    if index_id ~= 0 then
+        -- Ignore 'sequence = false' for secondary indexes.
+        if options.sequence == false then
+            return nil
+        end
         box.error(box.error.MODIFY_INDEX, index_name, space_name,
-                  "sequence cannot be used with a non-integer key")
+                  "sequence cannot be used with a secondary key")
+    end
+
+    -- Convert the provided option to the table format.
+    local new_sequence
+    if type(options.sequence) == 'table' then
+        -- Sequence is given as a table, just copy it.
+        -- Silently ignore unknown fields.
+        new_sequence = {
+            id = options.sequence.id,
+            field = options.sequence.field,
+        }
+    elseif options.sequence == true then
+        -- Create an auto-generated sequence.
+        new_sequence = {}
+    elseif options.sequence == false then
+        -- Drop the currently attached sequence.
+        new_sequence = nil
+    else
+        -- Attach a sequence with the given id.
+        new_sequence = {id = options.sequence}
+    end
+
+    -- Resolve the sequence name.
+    if new_sequence ~= nil and new_sequence.id ~= nil then
+        local id = sequence_resolve(new_sequence.id)
+        if id == nil then
+            box.error(box.error.NO_SUCH_SEQUENCE, new_sequence.id)
+        end
+        new_sequence.id = id
+    end
+
+    -- Look up the index part corresponding to the given field.
+    -- Note, sequence.part is 0-base.
+    if new_sequence ~= nil and new_sequence.field ~= nil then
+        local field, path = format_field_resolve(format, new_sequence.field,
+                                                 "sequence field")
+        for i, part in ipairs(parts) do
+            local part_field = part.field or part[1]
+            local part_path = part.path
+            if field == part_field and path == part_path then
+                new_sequence.part = i - 1
+                break
+            end
+        end
+        if new_sequence.part == nil then
+            box.error(box.error.MODIFY_INDEX, index_name, space_name,
+                      "sequence field must be a part of the index")
+        end
+    end
+
+    -- Look up the currently attached sequence, if any.
+    local old_sequence
+    local sequence_tuple = _space_sequence:get(space_id)
+    if sequence_tuple ~= nil then
+        old_sequence = {
+            id = sequence_tuple.sequence_id,
+            part = sequence_tuple.sequence_part,
+            is_generated = sequence_tuple.is_generated,
+        }
+    else
+        old_sequence = nil
+    end
+
+    -- Inherit omitted options from the attached sequence.
+    if new_sequence ~= nil and old_sequence ~= nil then
+        if new_sequence.id == nil and old_sequence.is_generated then
+            new_sequence.id = old_sequence.id
+            new_sequence.is_generated = true
+        end
+        if new_sequence.part == nil then
+            new_sequence.part = old_sequence.part
+        end
+    end
+
+    -- If sequence field is omitted, use the first indexed field.
+    if new_sequence ~= nil and new_sequence.part == nil then
+        new_sequence.part = 0
+    end
+
+    -- Check the type of the auto-increment field.
+    if new_sequence ~= nil then
+        local part = parts[new_sequence.part + 1]
+        local part_type = part.type or part[2]
+        if part_type ~= 'integer' and part_type ~= 'unsigned' then
+            box.error(box.error.MODIFY_INDEX, index_name, space_name,
+                      "sequence cannot be used with a non-integer key")
+        end
+    end
+
+    -- If sequence id is omitted, we are supposed to create
+    -- a new auto-generated sequence for the given space.
+    if new_sequence ~= nil and new_sequence.id == nil then
+        local seq = box.schema.sequence.create(space_name .. '_seq')
+        new_sequence.id = seq.id
+        new_sequence.is_generated = true
+    end
+    if new_sequence ~= nil then
+        new_sequence.is_generated = new_sequence.is_generated or false
+    end
+
+    -- Detach the old sequence before altering the space.
+    if new_sequence == nil and old_sequence ~= nil then
+        _space_sequence:delete(space_id)
+    end
+
+    return {
+        space_id = space_id,
+        new_sequence = new_sequence,
+        old_sequence = old_sequence,
+    }
+end
+
+--
+-- The second stage of a space sequence modification operation.
+-- Called after altering the space definition. Attaches the sequence
+-- to the space and drops the old sequence if required. 'proxy' is
+-- an object returned by space_sequence_alter_prepare().
+--
+local function space_sequence_alter_commit(proxy)
+    local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
+
+    if proxy == nil then
+        -- No sequence option, nothing to do.
+        return
+    end
+
+    local space_id = proxy.space_id
+    local old_sequence = proxy.old_sequence
+    local new_sequence = proxy.new_sequence
+
+    if new_sequence ~= nil and old_sequence == nil then
+        -- Attach a new sequence.
+        _space_sequence:insert{space_id, new_sequence.id,
+                               new_sequence.is_generated,
+                               new_sequence.part}
+    elseif new_sequence ~= nil and old_sequence ~= nil then
+        -- Replace the old sequence.
+        if new_sequence.id ~= old_sequence.id or
+           new_sequence.part ~= old_sequence.part then
+            _space_sequence:replace{space_id, new_sequence.id,
+                                    new_sequence.is_generated,
+                                    new_sequence.part}
+        end
+    end
+
+    if old_sequence ~= nil and old_sequence.is_generated and
+       (new_sequence == nil or old_sequence.id ~= new_sequence.id) then
+        -- Drop automatically generated sequence.
+        box.schema.sequence.drop(old_sequence.id)
     end
 end
 
@@ -787,8 +950,7 @@ local alter_index_template = {
     name = 'string',
     type = 'string',
     parts = 'table',
-    sequence = 'boolean, number, string',
-    sequence_part = 'number',
+    sequence = 'boolean, number, string, table',
 }
 for k, v in pairs(index_options) do
     alter_index_template[k] = v
@@ -898,41 +1060,15 @@ box.schema.index.create = function(space_id, name, options)
                      "please use '%s' instead", field_type, part.type)
         end
     end
-    local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
-    local sequence_is_generated = false
-    local sequence = options.sequence or nil -- ignore sequence = false
-    local sequence_part = options.sequence_part
-    if sequence_part ~= nil and sequence == nil then
-        box.error(box.error.MODIFY_INDEX, options.name, space.name,
-                  "sequence part cannot be used without sequence")
-    end
-    if sequence ~= nil then
-        if iid ~= 0 then
-            box.error(box.error.MODIFY_INDEX, name, space.name,
-                      "sequence cannot be used with a secondary key")
-        end
-        sequence_part = sequence_part or 1
-        check_sequence_part(parts, sequence_part, name, space.name)
-        if sequence == true then
-            sequence = box.schema.sequence.create(space.name .. '_seq')
-            sequence = sequence.id
-            sequence_is_generated = true
-        else
-            sequence = sequence_resolve(sequence)
-            if sequence == nil then
-                box.error(box.error.NO_SUCH_SEQUENCE, options.sequence)
-            end
-        end
-    end
     -- save parts in old format if possible
     if parts_can_be_simplified then
         parts = simplify_index_parts(parts)
     end
+    local sequence_proxy = space_sequence_alter_prepare(format, parts, options,
+                                                        space_id, iid,
+                                                        space.name, name)
     _index:insert{space_id, iid, name, options.type, index_opts, parts}
-    if sequence ~= nil then
-        _space_sequence:insert{space_id, sequence, sequence_is_generated,
-                               sequence_part - 1}
-    end
+    space_sequence_alter_commit(sequence_proxy)
     return space.index[name]
 end
 
@@ -1044,71 +1180,12 @@ box.schema.index.alter = function(space_id, index_id, options)
             parts = simplify_index_parts(parts)
         end
     end
-    local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
-    local sequence_is_generated = false
-    local sequence = options.sequence
-    local sequence_part = options.sequence_part
-    local sequence_tuple
-    if index_id ~= 0 then
-        if sequence or sequence_part ~= nil then
-            box.error(box.error.MODIFY_INDEX, options.name, space.name,
-                      "sequence cannot be used with a secondary key")
-        end
-        -- ignore 'sequence = false' for secondary indexes
-        sequence = nil
-    end
-    if sequence ~= nil or sequence_part ~= nil then
-        sequence_tuple = _space_sequence:get(space_id)
-        if sequence_tuple ~= nil then
-            -- Inherit omitted options from the attached sequence.
-            if sequence == nil then
-                sequence = sequence_tuple.sequence_id
-                sequence_is_generated = sequence_tuple.is_generated
-            end
-            if sequence and sequence_part == nil then
-                sequence_part = sequence_tuple.sequence_part
-            end
-        end
-    end
-    if sequence then
-        sequence_part = sequence_part or 1
-        check_sequence_part(parts, sequence_part, options.name, space.name)
-    elseif sequence_part ~= nil then
-        box.error(box.error.MODIFY_INDEX, options.name, space.name,
-                  "sequence part cannot be used without sequence")
-    end
-    if sequence == true then
-        if sequence_tuple == nil or sequence_tuple.is_generated == false then
-            sequence = box.schema.sequence.create(space.name .. '_seq')
-            sequence = sequence.id
-        else
-            -- Space already has an automatically generated sequence.
-            sequence = sequence_tuple.sequence_id
-        end
-        sequence_is_generated = true
-    elseif sequence then
-        sequence = sequence_resolve(sequence)
-        if sequence == nil then
-            box.error(box.error.NO_SUCH_SEQUENCE, options.sequence)
-        end
-    end
-    if sequence == false then
-        _space_sequence:delete(space_id)
-    end
+    local sequence_proxy = space_sequence_alter_prepare(format, parts, options,
+                                                        space_id, index_id,
+                                                        space.name, options.name)
     _index:replace{space_id, index_id, options.name, options.type,
                    index_opts, parts}
-    if sequence and (sequence_tuple == nil or
-                     sequence_tuple.sequence_id ~= sequence or
-                     sequence_tuple.sequence_part ~= sequence_part) then
-        _space_sequence:replace{space_id, sequence, sequence_is_generated,
-                                sequence_part - 1}
-    end
-    if sequence ~= nil and sequence_tuple ~= nil and
-       sequence_tuple.is_generated == true and
-       sequence_tuple.sequence_id ~= sequence then
-        -- Delete automatically generated sequence.
-        box.schema.sequence.drop(sequence_tuple.sequence_id)
-    end
+    space_sequence_alter_commit(sequence_proxy)
 end
 
 -- a static box_tuple_t ** instance for calling box_index_* API
diff --git a/test/box/sequence.result b/test/box/sequence.result
index 3cac4dc9..90bbafac 100644
--- a/test/box/sequence.result
+++ b/test/box/sequence.result
@@ -590,17 +590,20 @@ s:create_index('pk', {parts = {1, 'number'}, sequence = 'test'}) -- error
 - error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
     be used with a non-integer key'
 ...
-s:create_index('pk', {sequence_part = 1}) -- error
+s:create_index('pk', {sequence = {id = 'no_such_sequence'}}) -- error
 ---
-- error: 'Can''t create or modify index ''nil'' in space ''test'': sequence part cannot
-    be used without sequence'
+- error: Sequence 'no_such_sequence' does not exist
 ...
-s:create_index('pk', {sequence = true, sequence_part = 2}) -- error
+s:create_index('pk', {sequence = {field = 2}}) -- error
 ---
-- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence part is
-    out of bounds'
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence field must
+    be a part of the index'
 ...
-s:create_index('pk', {parts = {1, 'unsigned', 2, 'string'}, sequence = true, sequence_part = 2}) -- error
+s:create_index('pk', {sequence = {field = 'no.such.field'}}) -- error
+---
+- error: 'Illegal parameters, sequence field: field was not found by name ''no.such.field'''
+...
+s:create_index('pk', {parts = {1, 'unsigned', 2, 'string'}, sequence = {field = 2}}) -- error
 ---
 - error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
     be used with a non-integer key'
@@ -608,23 +611,33 @@ s:create_index('pk', {parts = {1, 'unsigned', 2, 'string'}, sequence = true, seq
 pk = s:create_index('pk', {parts = {1, 'string', 2, 'unsigned'}}) -- ok
 ---
 ...
-pk:alter{sequence_part = 1} -- error
+pk:alter{sequence = {id = 'no_such_sequence', field = 2}} -- error
 ---
-- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence part cannot
-    be used without sequence'
+- error: Sequence 'no_such_sequence' does not exist
 ...
-pk:alter{sequence = true, sequence_part = 1} -- error
+pk:alter{sequence = {id = 'test', field = 2}} -- ok
+---
+...
+pk:alter{sequence = {id = 'test', field = 1}} -- error
+---
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a non-integer key'
+...
+pk:alter{sequence = false} -- ok
+---
+...
+pk:alter{sequence = {field = 1}} -- error
 ---
 - error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
     be used with a non-integer key'
 ...
-pk:alter{sequence = true, sequence_part = 2} -- ok
+pk:alter{sequence = {field = 2}} -- ok
 ---
 ...
-pk:alter{sequence = false, sequence_part = 2} -- error
+pk:alter{sequence = {field = 1}} -- error
 ---
-- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence part cannot
-    be used without sequence'
+- error: 'Can''t create or modify index ''pk'' in space ''test'': sequence cannot
+    be used with a non-integer key'
 ...
 pk:alter{sequence = false} -- ok
 ---
@@ -657,10 +670,10 @@ s:create_index('secondary', {parts = {2, 'unsigned'}, sequence = true}) -- error
 - error: 'Can''t create or modify index ''secondary'' in space ''test'': sequence
     cannot be used with a secondary key'
 ...
-s:create_index('secondary', {parts = {2, 'unsigned'}, sequence_part = 1}) -- error
+s:create_index('secondary', {parts = {2, 'unsigned'}, sequence = {field = 2}}) -- error
 ---
-- error: 'Can''t create or modify index ''nil'' in space ''test'': sequence part cannot
-    be used without sequence'
+- error: 'Can''t create or modify index ''secondary'' in space ''test'': sequence
+    cannot be used with a secondary key'
 ...
 sk = s:create_index('secondary', {parts = {2, 'unsigned'}}) -- ok
 ---
@@ -767,10 +780,11 @@ box.space._space_sequence:insert{s.id, sq.id, false, 0} -- error
 ---
 - error: 'No index #0 is defined in space ''test'''
 ...
-s:create_index('pk', {sequence = {}}) -- error
+pk = s:create_index('pk', {sequence = {}}) -- ok
+---
+...
+pk:drop()
 ---
-- error: 'Illegal parameters, options parameter ''sequence'' should be one of types:
-    boolean, number, string'
 ...
 s:create_index('pk', {sequence = 'abc'}) -- error
 ---
@@ -807,10 +821,8 @@ s.index.pk.sequence_id == nil
 ---
 - true
 ...
-pk:alter{sequence = {}} -- error
+pk:alter{sequence = {}} -- ok
 ---
-- error: 'Illegal parameters, options parameter ''sequence'' should be one of types:
-    boolean, number, string'
 ...
 pk:alter{sequence = 'abc'} -- error
 ---
@@ -1962,7 +1974,7 @@ s:drop()
 s = box.schema.space.create('test')
 ---
 ...
-_ = s:create_index('pk', {parts = {1, 'string', 2, 'unsigned', 3, 'unsigned'}, sequence = true, sequence_part = 2})
+_ = s:create_index('pk', {parts = {1, 'string', 2, 'unsigned', 3, 'unsigned'}, sequence = {field = 2}})
 ---
 ...
 sequence_id = s.index.pk.sequence_id
@@ -1992,7 +2004,7 @@ s:insert{'b', box.NULL, 11}
 ---
 - ['b', 11, 11]
 ...
-s.index.pk:alter{sequence_part = 3}
+s.index.pk:alter{sequence = {field = 3}}
 ---
 ...
 s.index.pk.sequence_part == 3
@@ -2011,7 +2023,7 @@ s:insert{'c', 101, box.NULL}
 ---
 - ['c', 101, 101]
 ...
-s.index.pk:alter{sequence = true, sequence_part = 2}
+s.index.pk:alter{sequence = {field = 2}}
 ---
 ...
 s.index.pk.sequence_part == 2
@@ -2039,7 +2051,10 @@ s:drop()
 s = box.schema.space.create('test')
 ---
 ...
-_ = s:create_index('pk', {parts = {{'[1].a.b[1]', 'unsigned'}}, sequence = true})
+s:format{{'x', 'map'}}
+---
+...
+_ = s:create_index('pk', {parts = {{'x.a.b[1]', 'unsigned'}}, sequence = {field = 'x.a.b[1]'}})
 ---
 ...
 s:replace{} -- error
@@ -2062,6 +2077,16 @@ s:replace{{a = {b = {box.NULL}}}} -- ok
 ---
 - [{'a': {'b': [1]}}]
 ...
+s.index.pk:alter{sequence = false}
+---
+...
+s.index.pk:alter{sequence = {field = 'x.a.b[1]'}}
+---
+...
+s:replace{{a = {b = {box.NULL}}}} -- ok
+---
+- [{'a': {'b': [1]}}]
+...
 s:drop()
 ---
 ...
diff --git a/test/box/sequence.test.lua b/test/box/sequence.test.lua
index c39f1d41..ec5d011c 100644
--- a/test/box/sequence.test.lua
+++ b/test/box/sequence.test.lua
@@ -196,15 +196,19 @@ s:create_index('pk', {parts = {1, 'string'}, sequence = 'test'}) -- error
 s:create_index('pk', {parts = {1, 'scalar'}, sequence = 'test'}) -- error
 s:create_index('pk', {parts = {1, 'number'}, sequence = 'test'}) -- error
 
-s:create_index('pk', {sequence_part = 1}) -- error
-s:create_index('pk', {sequence = true, sequence_part = 2}) -- error
-s:create_index('pk', {parts = {1, 'unsigned', 2, 'string'}, sequence = true, sequence_part = 2}) -- error
+s:create_index('pk', {sequence = {id = 'no_such_sequence'}}) -- error
+s:create_index('pk', {sequence = {field = 2}}) -- error
+s:create_index('pk', {sequence = {field = 'no.such.field'}}) -- error
+s:create_index('pk', {parts = {1, 'unsigned', 2, 'string'}, sequence = {field = 2}}) -- error
 
 pk = s:create_index('pk', {parts = {1, 'string', 2, 'unsigned'}}) -- ok
-pk:alter{sequence_part = 1} -- error
-pk:alter{sequence = true, sequence_part = 1} -- error
-pk:alter{sequence = true, sequence_part = 2} -- ok
-pk:alter{sequence = false, sequence_part = 2} -- error
+pk:alter{sequence = {id = 'no_such_sequence', field = 2}} -- error
+pk:alter{sequence = {id = 'test', field = 2}} -- ok
+pk:alter{sequence = {id = 'test', field = 1}} -- error
+pk:alter{sequence = false} -- ok
+pk:alter{sequence = {field = 1}} -- error
+pk:alter{sequence = {field = 2}} -- ok
+pk:alter{sequence = {field = 1}} -- error
 pk:alter{sequence = false} -- ok
 pk:drop()
 
@@ -216,7 +220,7 @@ pk:drop()
 pk = s:create_index('pk') -- ok
 s:create_index('secondary', {parts = {2, 'unsigned'}, sequence = 'test'}) -- error
 s:create_index('secondary', {parts = {2, 'unsigned'}, sequence = true}) -- error
-s:create_index('secondary', {parts = {2, 'unsigned'}, sequence_part = 1}) -- error
+s:create_index('secondary', {parts = {2, 'unsigned'}, sequence = {field = 2}}) -- error
 sk = s:create_index('secondary', {parts = {2, 'unsigned'}}) -- ok
 sk:alter{sequence = 'test'} -- error
 sk:alter{sequence = true} -- error
@@ -246,7 +250,8 @@ sk:drop()
 pk:drop()
 box.space._space_sequence:insert{s.id, sq.id, false, 0} -- error
 
-s:create_index('pk', {sequence = {}}) -- error
+pk = s:create_index('pk', {sequence = {}}) -- ok
+pk:drop()
 s:create_index('pk', {sequence = 'abc'}) -- error
 s:create_index('pk', {sequence = 12345}) -- error
 pk = s:create_index('pk', {sequence = 'test'}) -- ok
@@ -257,7 +262,7 @@ s.index.pk.sequence_id == sq.id
 pk:drop()
 pk = s:create_index('pk', {sequence = false}) -- ok
 s.index.pk.sequence_id == nil
-pk:alter{sequence = {}} -- error
+pk:alter{sequence = {}} -- ok
 pk:alter{sequence = 'abc'} -- error
 pk:alter{sequence = 12345} -- error
 pk:alter{sequence = 'test'} -- ok
@@ -666,7 +671,7 @@ s:drop()
 -- gh-4009: setting sequence for an index part other than the first.
 --
 s = box.schema.space.create('test')
-_ = s:create_index('pk', {parts = {1, 'string', 2, 'unsigned', 3, 'unsigned'}, sequence = true, sequence_part = 2})
+_ = s:create_index('pk', {parts = {1, 'string', 2, 'unsigned', 3, 'unsigned'}, sequence = {field = 2}})
 sequence_id = s.index.pk.sequence_id
 sequence_id ~= nil
 s.index.pk.sequence_part == 2
@@ -674,12 +679,12 @@ s:insert{'a', box.NULL, 1}
 s:insert{'a', box.NULL, 2}
 s:insert{'b', 10, 10}
 s:insert{'b', box.NULL, 11}
-s.index.pk:alter{sequence_part = 3}
+s.index.pk:alter{sequence = {field = 3}}
 s.index.pk.sequence_part == 3
 s.index.pk.sequence_id == sequence_id
 s:insert{'c', 100, 100}
 s:insert{'c', 101, box.NULL}
-s.index.pk:alter{sequence = true, sequence_part = 2}
+s.index.pk:alter{sequence = {field = 2}}
 s.index.pk.sequence_part == 2
 s.index.pk.sequence_id == sequence_id
 s:insert{'d', 1000, 1000}
@@ -690,10 +695,14 @@ s:drop()
 -- gh-4210: using sequence with a json path key part.
 --
 s = box.schema.space.create('test')
-_ = s:create_index('pk', {parts = {{'[1].a.b[1]', 'unsigned'}}, sequence = true})
+s:format{{'x', 'map'}}
+_ = s:create_index('pk', {parts = {{'x.a.b[1]', 'unsigned'}}, sequence = {field = 'x.a.b[1]'}})
 s:replace{} -- error
 s:replace{{c = {}}} -- error
 s:replace{{a = {c = {}}}} -- error
 s:replace{{a = {b = {}}}} -- error
 s:replace{{a = {b = {box.NULL}}}} -- ok
+s.index.pk:alter{sequence = false}
+s.index.pk:alter{sequence = {field = 'x.a.b[1]'}}
+s:replace{{a = {b = {box.NULL}}}} -- ok
 s:drop()
-- 
2.11.0

[tarantool-patches] Re: [PATCH] schema: rework index sequence API

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [19/05/27 22:25]:
> Rather than passing 'sequence_part' along with 'sequence' on index
> create/alter, pass a table with the following fields:
> 
>  - id: sequence id or name
>  - field: auto increment field id or name or path in case of json index
> 
> If id is omitted, the sequence will be auto-generated (equivalent to
> 'sequence = true'). If field is omitted, the first indexed field is
> used. Old format, i.e. passing false/true or sequence name/id instead of
> a table is still supported.

Hi,

Thank you for the patch.

I certainly do not appreciate lack of changes in _space_sequence
definition.

You do hope, it seems, that mentioning field name in the sequence
name is a stupid idea, and will be reverted some day.
Meanwhile all front ends will have to resolve the field to path to
build a proper statement for _space_sequence.

> +s.index.pk:alter{sequence = false}
> +s.index.pk:alter{sequence = {field = 'x.a.b[1]'}}
> +s:replace{{a = {b = {box.NULL}}}} -- ok

What happens when I rename the field using space::format()?
Does sequence continue to work? Which field is it using then? 

What happens when if I alter the index and modify its part names, order
or count? The answer to this question depends on whether
_sequence_space stores the path or the part.

If you think using a field is a stupid idea and the core should
continue using part no, please, test the consequences of this at
least.

-- 
Konstantin Osipov, Moscow, Russia

Re: [tarantool-patches] Re: [PATCH] schema: rework index sequence API

[Vladimir Davydov]

On Mon, May 27, 2019 at 10:38:46PM +0300, Konstantin Osipov wrote:
> * Vladimir Davydov <vdavydov.dev@gmail.com> [19/05/27 22:25]:
> > Rather than passing 'sequence_part' along with 'sequence' on index
> > create/alter, pass a table with the following fields:
> > 
> >  - id: sequence id or name
> >  - field: auto increment field id or name or path in case of json index
> > 
> > If id is omitted, the sequence will be auto-generated (equivalent to
> > 'sequence = true'). If field is omitted, the first indexed field is
> > used. Old format, i.e. passing false/true or sequence name/id instead of
> > a table is still supported.
> 
> Hi,
> 
> Thank you for the patch.
> 
> I certainly do not appreciate lack of changes in _space_sequence
> definition.
> 
> You do hope, it seems, that mentioning field name in the sequence
> name is a stupid idea, and will be reverted some day.
> Meanwhile all front ends will have to resolve the field to path to
> build a proper statement for _space_sequence.

Ok, I'll try to rework that as well to store fieldno + path.

> 
> > +s.index.pk:alter{sequence = false}
> > +s.index.pk:alter{sequence = {field = 'x.a.b[1]'}}
> > +s:replace{{a = {b = {box.NULL}}}} -- ok
> 
> What happens when I rename the field using space::format()?
> Does sequence continue to work? Which field is it using then? 

I guess, just like indexes, sequences should be attached not to a field
name, but to a field no so nothing will happen when a field is renamed.

> 
> What happens when if I alter the index and modify its part names, order
> or count? The answer to this question depends on whether
> _sequence_space stores the path or the part.

If we store fieldno+path, there will be an error altering the index
unless sequence is modified as well. I guess that's okay.

> 
> If you think using a field is a stupid idea and the core should
> continue using part no, please, test the consequences of this at
> least.

Re: [tarantool-patches] Re: [PATCH] schema: rework index sequence API

[Konstantin Osipov]

* Vladimir Davydov <vdavydov.dev@gmail.com> [19/05/28 10:42]:
> > What happens when I rename the field using space::format()?
> > Does sequence continue to work? Which field is it using then? 
> 
> I guess, just like indexes, sequences should be attached not to a field
> name, but to a field no so nothing will happen when a field is renamed.
> 
> > 
> > What happens when if I alter the index and modify its part names, order
> > or count? The answer to this question depends on whether
> > _sequence_space stores the path or the part.
> 
> If we store fieldno+path, there will be an error altering the index
> unless sequence is modified as well. I guess that's okay.
> 
> > 
> > If you think using a field is a stupid idea and the core should
> > continue using part no, please, test the consequences of this at
> > least.

Thanks. The above questions were suggestions for test cases,
actually.

-- 
Konstantin Osipov, Moscow, Russia