diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua index 39e0de7ddafd117750e569f85891410d98297424..7aa58e49697f5b5bb66058867af2e8e73e55014f 100644 --- a/src/box/lua/load_cfg.lua +++ b/src/box/lua/load_cfg.lua @@ -56,6 +56,38 @@ local default_cfg = { snapshot_count = 6, } +-- types of available options +-- could be comma separated lua types or 'any' if any type is allowed +local template_cfg = { + listen = 'string, number', + slab_alloc_arena = 'number', + slab_alloc_minimal = 'number', + slab_alloc_factor = 'number', + work_dir = 'string', + snap_dir = 'string', + wal_dir = 'string', + logger = 'string', + logger_nonblock = 'boolean', + log_level = 'number', + io_collect_interval = 'number', + readahead = 'number', + snap_io_rate_limit = 'number', + too_long_threshold = 'number', + wal_mode = 'string', + rows_per_wal = 'number', + wal_dir_rescan_delay= 'number', + panic_on_snap_error = 'boolean', + panic_on_wal_error = 'boolean', + replication_source = 'string', + custom_proc_title = 'string', + pid_file = 'string', + background = 'boolean', + username = 'string', + coredump = 'boolean', + snapshot_period = 'number', + snapshot_count = 'number', +} + -- dynamically settable options local dynamic_cfg = { wal_mode = ffi.C.box_set_wal_mode, @@ -106,15 +138,53 @@ setmetatable(box, { end }) -function box.cfg(cfg) - if cfg == nil then - cfg = {} +local function check_param_table(table, template) + if table == nil then + return + end + if type(table) ~= 'table' then + error("Error: cfg should be a table") + end + -- just pass {.. dont_check = true, ..} to disable check below + if table.dont_check then + return + end + for k,v in pairs(table) do + if template[k] == nil then + error("Error: cfg parameter '" .. k .. "' is unexpected") + elseif template[k] == 'any' then + -- any type is ok + elseif (string.find(template[k], ',') == nil) then + -- one type + if type(v) ~= template[k] then + error("Error: cfg parameter '" .. k .. "' should be of type " .. template[k]) + end + else + local good_types = string.gsub(template[k], ' ', ''); + if (string.find(',' .. good_types .. ',', ',' .. type(v) .. ',') == nil) then + good_types = string.gsub(good_types, ',', ', '); + error("Error: cfg parameter '" .. k .. "' should be one of types: " .. template[k]) + end + end end - for k,v in pairs(default_cfg) do - if cfg[k] == nil then - cfg[k] = v +end + + +local function update_param_table(table, defaults) + if table == nil then + table = {} + end + for k,v in pairs(defaults) do + if table[k] == nil then + table[k] = v end end + return table +end + +function box.cfg(cfg) + check_param_table(cfg, template_cfg) + cfg = update_param_table(cfg, default_cfg) for k,v in pairs(wrapper_cfg) do -- options that can be number or string diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index b4002c7a8ed9f743f34fdeca5a902391634e5cc3..f594eaf0e8f6a3dbc2d71760c970ee76eaf45f49 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -74,22 +74,122 @@ local function user_resolve(user) return tuple[1] end +--[[ + @brief Common function to check table with parameters (like options) + @param table - table with parameters + @param template - table with expected types of expected parameters + type could be comma separated string with lua types (number, string etc), + or 'any' if any type is allowed + The function checks following: + 1)that parameters table is a table (or nil) + 2)all keys in parameters are present in template + 3)type of every parameter fits (one of) types described in template + Check (2) and (3) could be disabled by adding {, dont_check = <smth is true>, } + into parameters table + The functions calls box.error(box.error.ILLEGAL_PARAMS, ..) on error + @example + check_param_table(options, { user = 'string', + port = 'string, number', + data = 'any' } ) +--]] +local function check_param_table(table, template) + if table == nil then + return + end + if type(table) ~= 'table' then + box.error(box.error.ILLEGAL_PARAMS, + "options should be a table") + end + -- just pass {.. dont_check = true, ..} to disable checks below + if table.dont_check then + return + end + for k,v in pairs(table) do + if template[k] == nil then + box.error(box.error.ILLEGAL_PARAMS, + "options parameter '" .. k .. "' is unexpected") + elseif template[k] == 'any' then + -- any type is ok + elseif (string.find(template[k], ',') == nil) then + -- one type + if type(v) ~= template[k] then + box.error(box.error.ILLEGAL_PARAMS, + "options parameter '" .. k .. + "' should be of type " .. template[k]) + end + else + local good_types = string.gsub(template[k], ' ', '') + local haystack = ',' .. good_types .. ',' + local needle = ',' .. type(v) .. ',' + if (string.find(haystack, needle) == nil) then + good_types = string.gsub(good_types, ',', ', ') + box.error(box.error.ILLEGAL_PARAMS, + "options parameter '" .. k .. + "' should be one of types: " .. template[k]) + end + end + end +end + +--[[ + @brief Common function to check type parameter (of function) + Calls box.error(box.error.ILLEGAL_PARAMS, ) on error + @example: check_param(user, 'user', 'string') +--]] +local function check_param(param, name, should_be_type) + if type(param) ~= should_be_type then + box.error(box.error.ILLEGAL_PARAMS, + name .. " should be a " .. should_be_type) + end +end + +--[[ + Adds to a table key-value pairs from defaults table + that is not present in original table. + Returns updated table. + If nil is passed instead of table, it's treated as empty table {} + For example update_param_table({ type = 'hash', temporary = true }, + { type = 'tree', unique = true }) + will return table { type = 'hash', temporary = true, unique = true } +--]] +local function update_param_table(table, defaults) + if table == nil then + table = {} + end + if (defaults == nil) then + return table + end + for k,v in pairs(defaults) do + if table[k] == nil then + table[k] = v + end + end + return table +end + box.begin = function() if ffi.C.boxffi_txn_begin() == -1 then box.error() end end box.commit = function() if ffi.C.boxffi_txn_commit() == -1 then box.error() end end box.rollback = ffi.C.boxffi_txn_rollback; box.schema.space = {} box.schema.space.create = function(name, options) + check_param(name, 'name', 'string') + local options_template = { + if_not_exists = 'boolean', + temporary = 'boolean', + engine = 'string', + id = 'number', + field_count = 'number', + user = 'user', + } + local options_defaults = { + engine = 'memtx', + field_count = 0, + } + check_param_table(options, options_template) + options = update_param_table(options, options_defaults) + local _space = box.space[box.schema.SPACE_ID] - if options == nil then - options = {} - end - local if_not_exists = options.if_not_exists - local temporary = options.temporary and "temporary" or "" - local engine = "memtx" - if options.engine then - engine = options.engine - end if box.space[name] then if options.if_not_exists then return box.space[name], "not created" @@ -97,10 +197,8 @@ box.schema.space.create = function(name, options) box.error(box.error.SPACE_EXISTS, name) end end - local id - if options.id then - id = options.id - else + local id = options.id + if not id then id = _space.index[0]:max()[1] if id < box.schema.SYSTEM_ID_MAX then id = box.schema.SYSTEM_ID_MAX + 1 @@ -108,9 +206,6 @@ box.schema.space.create = function(name, options) id = id + 1 end end - if options.field_count == nil then - options.field_count = 0 - end local uid = nil if options.user then uid = user_resolve(options.user) @@ -118,11 +213,16 @@ box.schema.space.create = function(name, options) if uid == nil then uid = session.uid() end - _space:insert{id, uid, name, engine, options.field_count, temporary} + local temporary = options.temporary and "temporary" or "" + _space:insert{id, uid, name, options.engine, options.field_count, temporary} return box.space[id], "created" end + box.schema.create_space = box.schema.space.create + box.schema.space.drop = function(space_id) + check_param(space_id, 'space_id', 'number') + local _space = box.space[box.schema.SPACE_ID] local _index = box.space[box.schema.INDEX_ID] local _priv = box.space[box.schema.PRIV_ID] @@ -141,35 +241,73 @@ box.schema.space.drop = function(space_id) box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id)) end end + box.schema.space.rename = function(space_id, space_name) + check_param(space_id, 'space_id', 'number') + check_param(space_name, 'space_name', 'string') + local _space = box.space[box.schema.SPACE_ID] _space:update(space_id, {{"=", 3, space_name}}) end box.schema.index = {} -box.schema.index.create = function(space_id, name, options) - local _index = box.space[box.schema.INDEX_ID] - if options == nil then - options = {} +local function check_index_parts(parts) + if type(parts) ~= "table" then + box.error(box.error.ILLEGAL_PARAMS, + "options.parts parameter should be a table") end - if options.type == nil then - options.type = "tree" + if #parts % 2 ~= 0 then + box.error(box.error.ILLEGAL_PARAMS, + "options.parts: expected filed_no (number), type (string) pairs") end - if options.parts == nil then - options.parts = { 0, "num" } - else - for i=1,#options.parts,2 do + for i=1,#parts,2 do + if type(parts[i]) ~= "number" then + box.error(box.error.ILLEGAL_PARAMS, + "options.parts: expected filed_no (number), type (string) pairs") + elseif parts[i] == 0 then -- Lua uses one-based field numbers but _space is zero-based - if type(options.parts[i]) == "number" then - options.parts[i] = options.parts[i] - 1 - end + box.error(box.error.ILLEGAL_PARAMS, + "invalid index parts: field_no must be one-based") end end - if options.unique == nil then - options.unique = true + for i=2,#parts,2 do + if type(parts[i]) ~= "string" then + box.error(box.error.ILLEGAL_PARAMS, + "options.parts: expected filed_no (number), type (string) pairs") + end end - local index_type = options.type +end + +local function update_index_parts(parts) + for i=1,#parts,2 do + -- Lua uses one-based field numbers but _space is zero-based + parts[i] = parts[i] - 1 + end + return parts +end + +box.schema.index.create = function(space_id, name, options) + check_param(space_id, 'space_id', 'number') + check_param(name, 'name', 'string') + local options_template = { + type = 'string', + parts = 'table', + unique = 'boolean', + id = 'number', + } + local options_defaults = { + type = 'tree', + parts = { 1, "num" }, + unique = true, + } + check_param_table(options, options_template) + options = update_param_table(options, options_defaults) + check_index_parts(options.parts) + options.parts = update_index_parts(options.parts) + + local _index = box.space[box.schema.INDEX_ID] + local unique = options.unique and 1 or 0 local part_count = bit.rshift(#options.parts, 1) local parts = options.parts @@ -186,16 +324,27 @@ box.schema.index.create = function(space_id, name, options) if options.id then iid = options.id end - _index:insert{space_id, iid, name, index_type, unique, part_count, unpack(parts)} + _index:insert{space_id, iid, name, options.type, + unique, part_count, unpack(options.parts)} end + box.schema.index.drop = function(space_id, index_id) + check_param(space_id, 'space_id', 'number') + check_param(index_id, 'index_id', 'number') + local _index = box.space[box.schema.INDEX_ID] _index:delete{space_id, index_id} end + box.schema.index.rename = function(space_id, index_id, name) + check_param(space_id, 'space_id', 'number') + check_param(index_id, 'index_id', 'number') + check_param(name, 'name', 'string') + local _index = box.space[box.schema.INDEX_ID] _index:update({space_id, index_id}, {{"=", 3, name}}) end + box.schema.index.alter = function(space_id, index_id, options) if box.space[space_id] == nil then box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id)) @@ -206,10 +355,20 @@ box.schema.index.alter = function(space_id, index_id, options) if options == nil then return end - if type(space_id) == "string" then + + local options_template = { + type = 'string', + name = 'string', + parts = 'table', + unique = 'boolean', + id = 'number', + } + check_param_table(options, options_template) + + if type(space_id) ~= "number" then space_id = box.space[space_id].id end - if type(index_id) == "string" then + if type(index_id) ~= "number" then index_id = box.space[space_id].index[index_id].id end local _index = box.space[box.schema.INDEX_ID] @@ -247,12 +406,8 @@ box.schema.index.alter = function(space_id, index_id, options) if options.parts == nil then options.parts = {tuple:slice(6)} -- not part count else - for i=1,#options.parts,2 do - -- Lua uses one-based field numbers but _space is zero-based - if type(options.parts[i]) == "number" then - options.parts[i] = options.parts[i] - 1 - end - end + check_index_parts(options.parts) + options.parts = update_index_parts(options.parts) end _index:replace{space_id, index_id, options.name, options.type, options.unique, #options.parts/2, unpack(options.parts)} diff --git a/test/app/cfg.result b/test/app/cfg.result index 17f172e65116d1b808545b324c2852e5dffe3dc7..5ded3d0573975846c06847d6428e2e2ed3fafa9f 100644 --- a/test/app/cfg.result +++ b/test/app/cfg.result @@ -1,2 +1,2 @@ -false [string "-- load_cfg.lua - internal file..."]:105: Please call box.cfg{} first +false [string "-- load_cfg.lua - internal file..."]:137: Please call box.cfg{} first true table diff --git a/test/app/float_value.result b/test/app/float_value.result index a590ec2707af2d28e27b03505d839c13886c13ac..6e1c60c628d9128b0b8284cc430deed47da8e0af 100644 --- a/test/app/float_value.result +++ b/test/app/float_value.result @@ -1,21 +1,21 @@ box.cfg 1 snapshot_count:6 -2 too_long_threshold:0.01 +2 pid_file:box.pid 3 slab_alloc_factor:2 4 rows_per_wal:50 5 background:false 6 logger:tarantool.log 7 slab_alloc_arena:0.1 8 log_level:5 -9 logger_nonblock:true -10 snap_dir:. -11 coredump:false -12 wal_mode:write -13 pid_file:box.pid +9 listen:3313 +10 logger_nonblock:true +11 snap_dir:. +12 coredump:false +13 wal_mode:write 14 panic_on_snap_error:true 15 panic_on_wal_error:false -16 readahead:16320 -17 admin:3313 +16 too_long_threshold:0.01 +17 readahead:16320 18 wal_dir:. 19 snapshot_period:14400 20 slab_alloc_minimal:64 diff --git a/test/app/float_value.test.lua b/test/app/float_value.test.lua index bd39a55f03723e32792a80b1d89c7195f50bd0df..a9a0d2ae1490c33385ab90c8dcefb257c2059596 100755 --- a/test/app/float_value.test.lua +++ b/test/app/float_value.test.lua @@ -3,7 +3,7 @@ -- Test floating points values (too_long_treshold) with fractional part -- box.cfg{ - admin = 3313, + listen = 3313, slab_alloc_arena = 0.1, pid_file = "box.pid", rows_per_wal = 50, diff --git a/test/app/init_script.result b/test/app/init_script.result index 54e60c1cf385cc8f3fbf63ce550a390baef5c085..421e682bb48eee0f691c352a577f8757fc03f960 100644 --- a/test/app/init_script.result +++ b/test/app/init_script.result @@ -16,15 +16,14 @@ box.cfg 11 snap_dir:. 12 coredump:false 13 wal_mode:write -14 too_long_threshold:0.5 -15 panic_on_snap_error:true -16 panic_on_wal_error:false +14 panic_on_snap_error:true +15 panic_on_wal_error:false +16 too_long_threshold:0.5 17 readahead:16320 -18 admin:3313 -19 rows_per_wal:500000 -20 wal_dir:. -21 snapshot_period:14400 -22 wal_dir_rescan_delay:0.1 +18 rows_per_wal:500000 +19 wal_dir:. +20 snapshot_period:14400 +21 wal_dir_rescan_delay:0.1 -- -- Test insert from detached fiber -- diff --git a/test/app/init_script.test.lua b/test/app/init_script.test.lua index 4bc28a7a002db9a84f0b6ff5bc2764cb27b29036..2bb0552cfebe6f5c202013c4edb35720cfd96b76 100755 --- a/test/app/init_script.test.lua +++ b/test/app/init_script.test.lua @@ -3,7 +3,6 @@ -- Testing init script -- box.cfg{ - admin = 3313, listen = 3314, pid_file = "box.pid", logger="tarantool.log" diff --git a/test/box/alter.result b/test/box/alter.result index 76edd8110395f9d10bcd836728b90823b9c05f24..2a8dee1478e6580871159c213b2d6e9689649f47 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -327,7 +327,7 @@ auto2 = box.schema.create_space('auto', {id = auto.id}) ... box.schema.space.drop('auto') --- -- error: 'Supplied key type of part 0 does not match index part type: expected NUM' +- error: Illegal parameters, space_id should be a number ... auto2 --- @@ -371,3 +371,37 @@ box.space.space:select{} s:drop() --- ... +-- ------------------------------------------------------------------ +-- gh-362 Appropriate error messages in create_index +-- ------------------------------------------------------------------ +s = box.schema.create_space(42) +--- +- error: Illegal parameters, name should be a string +... +s = box.schema.create_space("test", "bug") +--- +- error: Illegal parameters, options should be a table +... +s = box.schema.create_space("test", {unknown = 'param'}) +--- +- error: Illegal parameters, options parameter 'unknown' is unexpected +... +s = box.schema.create_space("test") +--- +... +s:create_index('primary', {unique = true, parts = {0, 'NUM', 1, 'STR'}}) +--- +- error: 'Illegal parameters, invalid index parts: field_no must be one-based' +... +s:create_index('primary', {unique = true, parts = {'NUM', 1, 'STR', 2}}) +--- +- error: 'Illegal parameters, options.parts: expected filed_no (number), type (string) + pairs' +... +s:create_index('primary', {unique = true, parts = 'bug'}) +--- +- error: Illegal parameters, options parameter 'parts' should be of type table +... +s:drop() +--- +... diff --git a/test/box/alter.test.lua b/test/box/alter.test.lua index 5a2345ed07d9e26d1e7ba94c07cfa175930d8a9c..7c2c45d36e70d5470bbfd9ae94bd3466637a0e1c 100644 --- a/test/box/alter.test.lua +++ b/test/box/alter.test.lua @@ -139,3 +139,15 @@ box.space.space:replace{1,'The rain in Spain'} box.space.space:delete{1,'The rain in Spain'} box.space.space:select{} s:drop() + +-- ------------------------------------------------------------------ +-- gh-362 Appropriate error messages in create_index +-- ------------------------------------------------------------------ +s = box.schema.create_space(42) +s = box.schema.create_space("test", "bug") +s = box.schema.create_space("test", {unknown = 'param'}) +s = box.schema.create_space("test") +s:create_index('primary', {unique = true, parts = {0, 'NUM', 1, 'STR'}}) +s:create_index('primary', {unique = true, parts = {'NUM', 1, 'STR', 2}}) +s:create_index('primary', {unique = true, parts = 'bug'}) +s:drop() diff --git a/test/box/alter_limits.result b/test/box/alter_limits.result index 87d1cc2bbda53d04bea0bfcd3efefa66f9d3d2da..8a64a0270eb9ea246df9d8a74aa6445d33acf323 100644 --- a/test/box/alter_limits.result +++ b/test/box/alter_limits.result @@ -89,7 +89,7 @@ box.schema.create_space('tweedledee', { id = 3000 }) -- stupid space id box.schema.create_space('tweedledee', { id = 'tweedledee' }) --- -- error: 'Tuple field 0 type does not match one required by operation: expected NUM' +- error: Illegal parameters, options parameter 'id' should be of type number ... s:drop() --- @@ -392,7 +392,8 @@ s:create_index('test', { type = 'hash', parts = {}}) -- part count must be positive s:create_index('test', { type = 'hash', parts = { 1 }}) --- -- error: 'Can''t create or modify index 1 in space 512: part count must be positive' +- error: 'Illegal parameters, options.parts: expected filed_no (number), type (string) + pairs' ... -- unknown field type s:create_index('test', { type = 'hash', parts = { 1, 'nosuchtype' }}) @@ -402,7 +403,8 @@ s:create_index('test', { type = 'hash', parts = { 1, 'nosuchtype' }}) -- bad field no s:create_index('test', { type = 'hash', parts = { 'qq', 'nosuchtype' }}) --- -- error: 'Tuple field 6 type does not match one required by operation: expected NUM' +- error: 'Illegal parameters, options.parts: expected filed_no (number), type (string) + pairs' ... -- big field no s:create_index('test', { type = 'hash', parts = { box.schema.FIELD_MAX, 'num' }}) diff --git a/test/box/cfg.result b/test/box/cfg.result index d5f4fefb435ad7fc83ce544d6320c4eec0e2bf8d..e478367716f5e7d101d38a90fdce050a2924e302 100644 --- a/test/box/cfg.result +++ b/test/box/cfg.result @@ -2,7 +2,7 @@ --# push filter 'admin: .*' to 'admin: <uri>' box.cfg.nosuchoption = 1 --- -- error: '[string "-- load_cfg.lua - internal file..."]:132: Attempt to modify a read-only +- error: '[string "-- load_cfg.lua - internal file..."]:202: Attempt to modify a read-only table' ... t = {} for k,v in pairs(box.cfg) do if type(v) ~= 'table' and type(v) ~= 'function' then table.insert(t, k..': '..tostring(v)) end end