From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladimir Davydov Subject: [PATCH] schema: rework index sequence API Date: Mon, 27 May 2019 19:00:48 +0300 Message-Id: <5533af44673160684c608bec811109721bef5d36.1558972156.git.vdavydov.dev@gmail.com> To: tarantool-patches@freelists.org List-ID: 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