Skip to content

Commit

Permalink
box: forbid DDL operations until box.schema.upgrade
Browse files Browse the repository at this point in the history 
Currently, in case of recovery from an old snapshot, Tarantool allows to
perform DDL operations on an instance with non-upgraded schema.
It leads to various unpredictable errors (because the DDL code assumes
that the schema is already upgraded). This patch forbids the following
operations unless the user has the most recent schema version:
- box.schema.space.create
- box.schema.space.drop
- box.schema.space.alter
- box.schema.index.create
- box.schema.index.drop
- box.schema.index.alter
- box.schema.sequence.create
- box.schema.sequence.drop
- box.schema.sequence.alter
- box.schema.func.create
- box.schema.func.drop

Closes #7149

NO_DOC=bugfix
  • Loading branch information
@Gumix @locker
Gumix authored and locker committed Oct 18, 2022
1 parent 3e6393d commit 38f8879
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 13 deletions.
3 changes: 3 additions & 0 deletions changelogs/unreleased/gh-7149-forbid-ddl-until-box-schema-upgrade.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## bugfix/core

* Forbidden DDL operations for the non-upgraded schema (gh-7149).
1 change: 1 addition & 0 deletions src/box/errcode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ struct errcode_record {
/*245 */_(ER_OLD_TERM, "The term is outdated: old - %llu, new - %llu") \
/*246 */_(ER_INTERFERING_ELECTIONS, "Interfering elections started")\
/*247 */_(ER_ITERATOR_POSITION, "Iterator position is invalid") \
/*248 */_(ER_DDL_NOT_ALLOWED, "DDL operations are not allowed: %s") \

/*
* !IMPORTANT! Please follow instructions at start of the file
Expand Down
6 changes: 1 addition & 5 deletions src/box/lua/load_cfg.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -979,12 +979,8 @@ local function load_cfg(cfg)
-- Check if schema version matches Tarantool version and print
-- warning if it's not (in case user forgot to call
-- box.schema.upgrade()).
local needs, schema_version_str = private.schema_needs_upgrade()
local needs, msg = private.schema_needs_upgrade()
if needs then
local msg = string.format(
'Your schema version is %s while Tarantool %s requires a more'..
' recent schema version. Please, consider using box.'..
'schema.upgrade().', schema_version_str, box.info.version)
log.warn(msg)
end
end
Expand Down
19 changes: 19 additions & 0 deletions src/box/lua/schema.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,14 @@ local function revoke_object_privs(object_type, object_id)
end
end

-- Check if schema version matches Tarantool version and raise an error if not.
local function check_schema_version()
local needs_upgrade, msg = internal.schema_needs_upgrade()
if needs_upgrade then
box.error(box.error.DDL_NOT_ALLOWED, msg)
end
end

-- Same as type(), but returns 'number' if 'param' is
-- of type 'cdata' and represents a 64-bit integer.
local function param_type(param)
Expand Down Expand Up @@ -797,6 +805,7 @@ box.schema.space.create = function(name, options)
temporary = false,
}
check_param_table(options, options_template)
check_schema_version()
options = update_param_table(options, options_defaults)
if options.engine == 'vinyl' then
options = update_param_table(options, {
Expand Down Expand Up @@ -885,6 +894,7 @@ box.schema.space.drop = function(space_id, space_name, opts)
check_param(space_id, 'space_id', 'number')
opts = opts or {}
check_param_table(opts, { if_exists = 'boolean' })
check_schema_version()
local _space = box.space[box.schema.SPACE_ID]
local _index = box.space[box.schema.INDEX_ID]
local _trigger = box.space[box.schema.TRIGGER_ID]
Expand Down Expand Up @@ -956,6 +966,7 @@ box.schema.space.alter = function(space_id, options)
box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id))
end
check_param_table(options, alter_space_template)
check_schema_version()

local _space = box.space._space
local tuple = _space:get({space.id})
Expand Down Expand Up @@ -1527,6 +1538,7 @@ box.schema.index.create = function(space_id, name, options)
check_param(space_id, 'space_id', 'number')
check_param(name, 'name', 'string')
check_param_table(options, create_index_template)
check_schema_version()
local space = box.space[space_id]
if not space then
box.error(box.error.NO_SUCH_SPACE, '#'..tostring(space_id))
Expand Down Expand Up @@ -1656,6 +1668,7 @@ end
box.schema.index.drop = function(space_id, index_id)
check_param(space_id, 'space_id', 'number')
check_param(index_id, 'index_id', 'number')
check_schema_version()
if index_id == 0 then
local _space_sequence = box.space[box.schema.SPACE_SEQUENCE_ID]
local sequence_tuple = _space_sequence:delete{space_id}
Expand Down Expand Up @@ -1696,6 +1709,7 @@ box.schema.index.alter = function(space_id, index_id, options)
end

check_param_table(options, alter_index_template)
check_schema_version()

if type(space_id) ~= "number" then
space_id = space.id
Expand Down Expand Up @@ -2885,6 +2899,7 @@ box.schema.sequence.create = function(name, opts)
opts = opts or {}
check_param(name, 'name', 'string')
check_param_table(opts, create_sequence_options)
check_schema_version()
local ascending = not opts.step or opts.step > 0
local options_defaults = {
step = 1,
Expand All @@ -2910,6 +2925,7 @@ end

box.schema.sequence.alter = function(name, opts)
check_param_table(opts, alter_sequence_options)
check_schema_version()
local id, tuple = sequence_resolve(name)
if id == nil then
box.error(box.error.NO_SUCH_SEQUENCE, name)
Expand All @@ -2929,6 +2945,7 @@ end
box.schema.sequence.drop = function(name, opts)
opts = opts or {}
check_param_table(opts, {if_exists = 'boolean'})
check_schema_version()
local id = sequence_resolve(name)
if id == nil then
if not opts.if_exists then
Expand Down Expand Up @@ -3171,6 +3188,7 @@ box.schema.func.create = function(name, opts)
comment = 'string',
param_list = 'table', returns = 'string',
exports = 'table', opts = 'table' })
check_schema_version()
local _func = box.space[box.schema.FUNC_ID]
local _vfunc = box.space[box.schema.VFUNC_ID]
local func = _vfunc.index.name:get{name}
Expand Down Expand Up @@ -3206,6 +3224,7 @@ end
box.schema.func.drop = function(name, opts)
opts = opts or {}
check_param_table(opts, { if_exists = 'boolean' })
check_schema_version()
local _func = box.space[box.schema.FUNC_ID]
local _vfunc = box.space[box.schema.VFUNC_ID]
local fid
Expand Down
28 changes: 20 additions & 8 deletions src/box/lua/upgrade.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,17 +667,23 @@ end
local function upgrade_priv_to_1_10_2()
local _priv = box.space._priv
local _vpriv = box.space._vpriv
local _index = box.space._index
local format = _priv:format()

format[4].type = 'scalar'
_priv:format(format)
format = _vpriv:format()
format[4].type = 'scalar'
_vpriv:format(format)
_priv.index.primary:alter{parts={2, 'unsigned', 3, 'string', 4, 'scalar'}}
_vpriv.index.primary:alter{parts={2, 'unsigned', 3, 'string', 4, 'scalar'}}
_priv.index.object:alter{parts={3, 'string', 4, 'scalar'}}
_vpriv.index.object:alter{parts={3, 'string', 4, 'scalar'}}

_index:update({_priv.id, _priv.index.primary.id},
{{'=', 'parts', {{1, 'unsigned'}, {2, 'string'}, {3, 'scalar'}}}})
_index:update({_vpriv.id, _vpriv.index.primary.id},
{{'=', 'parts', {{1, 'unsigned'}, {2, 'string'}, {3, 'scalar'}}}})
_index:update({_priv.id, _priv.index.object.id},
{{'=', 'parts', {{2, 'string'}, {3, 'scalar'}}}})
_index:update({_vpriv.id, _priv.index.object.id},
{{'=', 'parts', {{2, 'string'}, {3, 'scalar'}}}})
end

local function create_vinyl_deferred_delete_space()
Expand Down Expand Up @@ -1079,8 +1085,9 @@ local function upgrade_func_to_2_2_1()
format[18] = {name='created', type='string'}
format[19] = {name='last_altered', type='string'}
_func:format(format)
_func.index.name:alter({parts = {{'name', 'string',
collation = 'unicode_ci'}}})
box.space._index:update(
{_func.id, _func.index.name.id},
{{'=', 'parts', {{field = 2, type = 'string', collation = 2}}}})
end

local function create_func_index()
Expand Down Expand Up @@ -1146,7 +1153,8 @@ end

local function drop_func_collation()
local _func = box.space[box.schema.FUNC_ID]
_func.index.name:alter({parts = {{'name', 'string'}}})
box.space._index:update({_func.id, _func.index.name.id},
{{'=', 'parts', {{2, 'string'}}}})
end

local function create_session_settings_space()
Expand Down Expand Up @@ -1306,7 +1314,11 @@ local function schema_needs_upgrade()
local schema_version, schema_version_str = get_version()
if schema_version ~= nil and
handlers[#handlers].version > schema_version then
return true, schema_version_str
local msg = string.format(
'Your schema version is %s while Tarantool %s requires a more'..
' recent schema version. Please, consider using box.'..
'schema.upgrade().', schema_version_str, box.info.version)
return true, msg
end
return false
end
Expand Down
59 changes: 59 additions & 0 deletions test/box-luatest/gh_7149_forbid_ddl_until_box_schema_upgrade_test.lua
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,62 @@ g.test_schema_access = function()
box.schema.upgrade()
end)
end

g.before_test('test_ddl_ops', function()
-- Recover from Tarantool 1.10 snapshot
local data_dir = 'test/box-luatest/upgrade/1.10'
-- Disable automatic schema upgrade
local box_cfg = {read_only = true}
g.server = server:new{alias = 'master',
datadir = data_dir,
box_cfg = box_cfg}
g.server:start()
end)

g.test_ddl_ops = function()
g.server:exec(function()
local t = require('luatest')
local error_msg = "DDL operations are not allowed: " ..
"Your schema version is 1.10.0 while Tarantool "

-- Note that automatic schema upgrade will not be performed
box.cfg{read_only = false}

t.assert_error_msg_contains(error_msg,
function() box.schema.space.create('test') end)
t.assert_error_msg_contains(error_msg,
function() box.schema.space.alter(box.space.T1.id, {}) end)
t.assert_error_msg_contains(error_msg,
function() box.schema.space.drop(box.space.T1.id) end)
t.assert_error_msg_contains(error_msg,
function() box.schema.index.create(box.space.T1.id, 'name') end)
t.assert_error_msg_contains(error_msg,
function() box.schema.index.alter(box.space.T1.id, 0, {}) end)
t.assert_error_msg_contains(error_msg,
function() box.schema.index.drop(box.space.T1.id, 0) end)
t.assert_error_msg_contains(error_msg,
function() box.schema.sequence.create('test') end)
t.assert_error_msg_contains(error_msg,
function() box.schema.sequence.alter('test', {}) end)
t.assert_error_msg_contains(error_msg,
function() box.schema.sequence.drop('test') end)
t.assert_error_msg_contains(error_msg,
function() box.schema.func.create('test') end)
t.assert_error_msg_contains(error_msg,
function() box.schema.func.drop('test') end)

box.schema.upgrade()

box.schema.space.create('test')
box.schema.space.alter(box.space.test.id, {})
box.schema.space.drop(box.space.test.id)
box.schema.index.create(box.space.T1.id, 'name')
box.schema.index.alter(box.space.T1.id, 1, {})
box.schema.index.drop(box.space.T1.id, 1)
box.schema.sequence.create('test')
box.schema.sequence.alter('test', {})
box.schema.sequence.drop('test')
box.schema.func.create('test')
box.schema.func.drop('test')
end)
end
Binary file added test/box-luatest/upgrade/1.10/00000000000000000004.snap
View file
Binary file not shown.
1 change: 1 addition & 0 deletions test/box/error.result
View file
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,7 @@ t;
| 245: box.error.OLD_TERM
| 246: box.error.INTERFERING_ELECTIONS
| 247: box.error.ITERATOR_POSITION
| 248: box.error.DDL_NOT_ALLOWED
| ...

test_run:cmd("setopt delimiter ''");
Expand Down

0 comments on commit 38f8879

Please sign in to comment.