* [PATCH] schema: rework index sequence API
@ 2019-05-27 16:00 Vladimir Davydov
2019-05-27 19:38 ` [tarantool-patches] " Konstantin Osipov
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Davydov @ 2019-05-27 16:00 UTC (permalink / raw)
To: tarantool-patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [tarantool-patches] Re: [PATCH] schema: rework index sequence API
2019-05-27 16:00 [PATCH] schema: rework index sequence API Vladimir Davydov
@ 2019-05-27 19:38 ` Konstantin Osipov
2019-05-28 7:41 ` Vladimir Davydov
0 siblings, 1 reply; 4+ messages in thread
From: Konstantin Osipov @ 2019-05-27 19:38 UTC (permalink / raw)
To: tarantool-patches
* 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tarantool-patches] Re: [PATCH] schema: rework index sequence API
2019-05-27 19:38 ` [tarantool-patches] " Konstantin Osipov
@ 2019-05-28 7:41 ` Vladimir Davydov
2019-05-28 9:38 ` Konstantin Osipov
0 siblings, 1 reply; 4+ messages in thread
From: Vladimir Davydov @ 2019-05-28 7:41 UTC (permalink / raw)
To: Konstantin Osipov; +Cc: tarantool-patches
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [tarantool-patches] Re: [PATCH] schema: rework index sequence API
2019-05-28 7:41 ` Vladimir Davydov
@ 2019-05-28 9:38 ` Konstantin Osipov
0 siblings, 0 replies; 4+ messages in thread
From: Konstantin Osipov @ 2019-05-28 9:38 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: tarantool-patches
* 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-05-28 9:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-27 16:00 [PATCH] schema: rework index sequence API Vladimir Davydov
2019-05-27 19:38 ` [tarantool-patches] " Konstantin Osipov
2019-05-28 7:41 ` Vladimir Davydov
2019-05-28 9:38 ` Konstantin Osipov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox