[Tarantool-patches] [PATCH v2 0/2] allow upgrading from version 1.6

[Tarantool-patches] [PATCH v2 0/2] allow upgrading from version 1.6

[Serge Petrenko via Tarantool-patches]

Changes in v2:
  - Review fixes as per reviews from Sergos, Vladimir
  - Fix triggers being run on xlogs resulting from
    box.schema.ugrade()
  - Introduce table.equals in a separate commit.
    Use it to define whether some schema changes were
    already applied.

https://github.com/tarantool/tarantool/issues/5894
https://github.com/tarantool/tarantool/tree/sp/gh-5894-1.6-upgrade

Serge Petrenko (2):
  lua: introduce table.equals method
  box: allow upgrading from version 1.6

 src/box/lua/load_cfg.lua                      |  14 +
 src/box/lua/upgrade.lua                       | 276 +++++++++++-
 src/lua/table.lua                             |  26 ++
 test/app-tap/table.test.lua                   |  31 +-
 test/xlog/gh-5894-pre-1.7.7-upgrade.result    | 400 ++++++++++++++++++
 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua  |  77 ++++
 .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 test/xlog/upgrade/fill.lua                    |   4 +
 11 files changed, 829 insertions(+), 3 deletions(-)
 create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
 create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
 create mode 120000 test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
 create mode 120000 test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
 create mode 120000 test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
 create mode 120000 test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua

-- 
2.30.1 (Apple Git-130)

[Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Serge Petrenko via Tarantool-patches]

Introduce table.equals for comparing tables.
The method respects __eq metamethod, if provided.

Needed-for #5894
---
 src/lua/table.lua           | 26 ++++++++++++++++++++++++++
 test/app-tap/table.test.lua | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/src/lua/table.lua b/src/lua/table.lua
index 8fa9b876a..5f35a30f6 100644
--- a/src/lua/table.lua
+++ b/src/lua/table.lua
@@ -57,6 +57,31 @@ local function table_shallowcopy(orig)
     return copy
 end
 
+--- Compare two lua tables
+-- Supports __eq metamethod for comparing custom tables with metatables
+-- @function equals
+-- @return true when the two tables are equal (false otherwise).
+local function table_equals(a, b)
+    if type(a) ~= 'table' or type(b) ~= 'table' then
+        return a == b
+    end
+    local mt = getmetatable(a)
+    if mt and mt.__eq then
+        return a == b
+    end
+    for k, v in pairs(a) do
+        if not table_equals(v, b[k]) then
+            return false
+        end
+    end
+    for k, _ in pairs(b) do
+        if not a[k] then
+            return false
+        end
+    end
+    return true
+end
+
 -- table library extension
 local table = require('table')
 -- require modifies global "table" module and adds "clear" function to it.
@@ -65,3 +90,4 @@ require('table.clear')
 
 table.copy     = table_shallowcopy
 table.deepcopy = table_deepcopy
+table.equals   = table_equals
diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
index 60c095fdf..a3c9aa123 100755
--- a/test/app-tap/table.test.lua
+++ b/test/app-tap/table.test.lua
@@ -8,7 +8,7 @@ yaml.cfg{
     encode_invalid_as_nil  = true,
 }
 local test = require('tap').test('table')
-test:plan(31)
+test:plan(38)
 
 do -- check basic table.copy (deepcopy)
     local example_table = {
@@ -223,4 +223,33 @@ do -- check usage of not __copy metamethod on second level + shallow
     )
 end
 
+do -- check table.equals
+    test:ok(table.equals({}, {}), "table.equals for empty tables")
+    test:is(table.equals({}, {1}), false, "table.equals with one empty table")
+    test:is(table.equals({1}, {}), false, "table.equals with one empty table")
+    local tbl_a = {
+        first = {
+            1,
+            2,
+            {},
+        },
+        second = {
+            a = {
+                {'something'},
+            },
+            b = 'something else',
+        },
+        [3] = 'some value',
+    }
+    local tbl_b = table.deepcopy(tbl_a)
+    local tbl_c = table.copy(tbl_a)
+    test:ok(table.equals(tbl_a, tbl_b), "table.equals for complex tables")
+    test:ok(table.equals(tbl_a, tbl_c),
+            "table.equals for shallow copied tables")
+    tbl_c.second.a = 'other thing'
+    test:ok(table.equals(tbl_a, tbl_c),
+            "table.equals for shallow copied tables after modification")
+    test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a deep check")
+end
+
 os.exit(test:check() == true and 0 or 1)
-- 
2.30.1 (Apple Git-130)

[Tarantool-patches] [PATCH v2 2/2] box: allow upgrading from version 1.6

[Serge Petrenko via Tarantool-patches]

Direct upgrade support from pre-1.7.5 versions was removed in commit
7d3b80e78206c479cab75b4893629cfa1932252e
(Forbid upgrade from Tarantool < 1.7.5 and refactor upgrade.lua)
The reason for that was the mandatory space format checks introduced
back then. With these space format checks, old schema couldn't be
recovered on new Tarantool versions, because newer versions had
different system space formats. So old schema couldn't be upgraded
because it couldn't even be recovered.

Actually this was rather inconvenient. One had to perform an extra
upgrade step when upgrading from, say, 1.6 to 2.x: instead of
performing a direct upgrade one had to do 1.6 -> 1.10 -> 2.x upgrade
which takes twice the time.

Make it possible to boot from snapshots coming from Tarantool version
1.6.8 and above.

In order to do so, introduce before_replace triggers on system spaces,
which work during snapshot/xlog recovery. The triggers will set tuple
formats to the ones supported by current Tarantool (2.x). This way the
recovered data will have the correct format for a usual schema upgrade.

Also add upgrade_to_1_7_5() handler, which finishes transformation of
old schema to 1.7.5. The handler is fired together with other
box.schema.upgrade() handlers, so there's no user-visible behaviour
change.

Side note: it would be great to use the same technique to allow booting
from pre-1.6.8 snapshots. Unfortunately, this is not possible.

Current triggers don't break the order of schema upgrades, so 1.7.1
upgrades come before 1.7.2 and 1.7.5. This is because all the upgrades
in these versions are replacing existing tuples and not inserting new
ones, so the upgrades may be handled by the before_replace triggers.

Upgrade to 1.6.8 requires inserting new tuples: creating sysviews, like
_vspace, _vuser and so on. This can't be done from the before_replace
triggers, so we would have to run triggers for 1.7.x first which would
allow Tarantool to recover the snapshot, and then run an upgrade handler for
1.6.8. This looks really messy.

Closes #5894
---
 src/box/lua/load_cfg.lua                      |  14 +
 src/box/lua/upgrade.lua                       | 276 +++++++++++-
 test/xlog/gh-5894-pre-1.7.7-upgrade.result    | 400 ++++++++++++++++++
 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua  |  77 ++++
 .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
 test/xlog/upgrade/fill.lua                    |   4 +
 9 files changed, 773 insertions(+), 2 deletions(-)
 create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
 create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
 create mode 120000 test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
 create mode 120000 test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
 create mode 120000 test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
 create mode 120000 test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua

diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index 4df70c210..7a8cab3fd 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -719,9 +719,23 @@ local function load_cfg(cfg)
             __call = locked(reload_cfg),
         })
 
+    -- Check schema version of the snapshot we're about to recover, if any.
+    -- Some schema versions (below 1.7.5) are incompatible with Tarantool 2.x
+    -- When recovering from such an old snapshot, special recovery triggers on
+    -- system spaces are needed in order to be able to recover and upgrade
+    -- the schema then.
+    local snap_dir = box.cfg.memtx_dir
+    local snap_version = private.get_snapshot_version(snap_dir)
+    if snap_version then
+        private.set_recovery_triggers(snap_version)
+    end
+
     -- This call either succeeds or calls panic() / exit().
     private.cfg_load()
 
+    if snap_version then
+        private.clear_recovery_triggers()
+    end
     -- This block does not raise an error: all necessary checks
     -- already performed in private.cfg_check(). See <dynamic_cfg>
     -- comment.
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 6abce50f4..925adab18 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -1,6 +1,8 @@
 local log = require('log')
 local bit = require('bit')
 local json = require('json')
+local fio = require('fio')
+local xlog = require('xlog')
 
 -- Guest user id - the default user
 local GUEST = 0
@@ -86,6 +88,40 @@ local function set_system_triggers(val)
     foreach_system_space(function(s) s:run_triggers(val) end)
 end
 
+-- Get schema version, stored in _schema system space, by reading the latest
+-- snapshot file from the snap_dir. Useful to define schema_version before
+-- recovering the snapshot, because some schema versions are too old and cannot
+-- be recovered normally.
+local function get_snapshot_version(snap_dir)
+    local snap_pattern = snap_dir..'/'..string.rep('[0-9]', 20)..'.snap'
+    local snap_list = fio.glob(snap_pattern)
+    table.sort(snap_list)
+    local snap = snap_list[#snap_list]
+    if not snap then
+        return nil
+    end
+    local version = nil
+    for _, row in xlog.pairs(snap) do
+        local sid = row.BODY and row.BODY.space_id
+        if sid == box.schema.SCHEMA_ID then
+            local tuple = row.BODY.tuple
+            if tuple and tuple[1] == 'version' then
+                local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
+                if major and minor and patch and type(major) == 'number' and
+                   type(minor) == 'number' and type(patch) == 'number' then
+                    version = mkversion(major, minor, patch)
+                    break
+                end
+            end
+        elseif sid and sid > box.schema.SCHEMA_ID then
+            -- Exit early if version wasn't found in _schema space.
+            -- Snapshot rows are ordered by space id.
+            break
+        end
+    end
+    return version
+end
+
 --------------------------------------------------------------------------------
 -- Bootstrap
 --------------------------------------------------------------------------------
@@ -131,6 +167,144 @@ local function create_sysview(source_id, target_id)
     end
 end
 
+--------------------------------------------------------------------------------
+-- Tarantool 1.7.1
+--------------------------------------------------------------------------------
+local function user_trig_1_7_1(_, tuple)
+    if tuple and tuple[3] == 'guest' and not tuple[5] then
+        local auth_method_list = {}
+        auth_method_list["chap-sha1"] = box.schema.user.password("")
+        tuple = tuple:update{{'=', 5, auth_method_list}}
+        log.info("Set empty password to user 'guest'")
+    end
+    return tuple
+end
+
+--------------------------------------------------------------------------------
+-- Tarantool 1.7.2
+--------------------------------------------------------------------------------
+local function index_trig_1_7_2(_, tuple)
+    local field_types_v16 = {
+        num = 'unsigned',
+        int = 'integer',
+        str = 'string',
+    }
+    if not tuple then
+        return tuple
+    end
+    local parts = tuple[6]
+    local changed = false
+    for _, part in pairs(parts) do
+        local field_type = part[2]:lower()
+        if field_types_v16[field_type] ~= nil then
+            part[2] = field_types_v16[field_type]
+            changed = true
+        end
+    end
+    if changed then
+        log.info("Update index '%s' on space '%s': set parts to %s", tuple[3],
+                 box.space[tuple[1]].name, json.encode(parts))
+        tuple = tuple:update{{'=', 6, parts}}
+    end
+    return tuple
+end
+
+--------------------------------------------------------------------------------
+-- Tarantool 1.7.5
+--------------------------------------------------------------------------------
+local function create_truncate_space()
+    local _truncate = box.space[box.schema.TRUNCATE_ID]
+
+    log.info("create space _truncate")
+    box.space._space:insert{
+        _truncate.id, ADMIN, '_truncate', 'memtx', 0, setmap({}),
+        {{name = 'id', type = 'unsigned'}, {name = 'count', type = 'unsigned'}}
+    }
+
+    log.info("create index primary on _truncate")
+    box.space._index:insert{
+        _truncate.id, 0, 'primary', 'tree', {unique = true}, {{0, 'unsigned'}}
+    }
+
+    local _priv = box.space[box.schema.PRIV_ID]
+    _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
+end
+
+local function upgrade_to_1_7_5()
+    create_truncate_space()
+end
+
+local function user_trig_1_7_5(_, tuple)
+    if tuple and not tuple[5] then
+        tuple = tuple:update{{'=', 5, setmap({})}}
+        log.info("Set empty password to %s '%s'", tuple[4], tuple[3])
+    end
+    return tuple
+end
+
+local space_formats_1_7_5 = {
+    _schema = {
+        {name = 'key', type = 'string'},
+    },
+    _space = {
+        {name = 'id', type = 'unsigned'},
+        {name = 'owner', type = 'unsigned'},
+        {name = 'name', type = 'string'},
+        {name = 'engine', type = 'string'},
+        {name = 'field_count', type = 'unsigned'},
+        {name = 'flags', type = 'map'},
+        {name = 'format', type = 'array'},
+    },
+    _index = {
+        {name = 'id', type = 'unsigned'},
+        {name = 'iid', type = 'unsigned'},
+        {name = 'name', type = 'string'},
+        {name = 'type', type = 'string'},
+        {name = 'opts', type = 'map'},
+        {name = 'parts', type = 'array'},
+    },
+    _func = {
+        {name = 'id', type = 'unsigned'},
+        {name = 'owner', type = 'unsigned'},
+        {name = 'name', type = 'string'},
+        {name = 'setuid', type = 'unsigned'},
+    },
+    _user = {
+        {name = 'id', type = 'unsigned'},
+        {name = 'owner', type = 'unsigned'},
+        {name = 'name', type = 'string'},
+        {name = 'type', type = 'string'},
+        {name = 'auth', type = 'map'},
+    },
+    _priv = {
+        {name = 'grantor', type = 'unsigned'},
+        {name = 'grantee', type = 'unsigned'},
+        {name = 'object_type', type = 'string'},
+        {name = 'object_id', type = 'unsigned'},
+        {name = 'privilege', type = 'unsigned'},
+    },
+    _cluster = {
+        {name = 'id', type = 'unsigned'},
+        {name = 'uuid', type = 'string'},
+    },
+}
+
+space_formats_1_7_5._vspace = space_formats_1_7_5._space
+space_formats_1_7_5._vindex = space_formats_1_7_5._index
+space_formats_1_7_5._vfunc = space_formats_1_7_5._func
+space_formats_1_7_5._vuser = space_formats_1_7_5._user
+space_formats_1_7_5._vpriv = space_formats_1_7_5._priv
+
+local function space_trig_1_7_5(_, tuple)
+    if tuple and space_formats_1_7_5[tuple[3]] and
+       not table.equals(space_formats_1_7_5[tuple[3]], tuple[7]) then
+        tuple = tuple:update{{'=', 7, space_formats_1_7_5[tuple[3]]}}
+        log.info("Update space '%s' format: new format %s", tuple[3],
+                 json.encode(tuple[7]))
+    end
+    return tuple
+end
+
 local function initial_1_7_5()
     -- stick to the following convention:
     -- prefer user id (owner id) in field #1
@@ -452,6 +626,15 @@ local function upgrade_to_1_7_7()
     _priv:replace({ADMIN, SUPER, 'universe', 0, 4294967295})
 end
 
+local function priv_trig_1_7_7(_, tuple)
+    if tuple and tuple[2] == ADMIN and tuple[3] == 'universe' and
+       tuple[5] ~= box.priv.ALL then
+        tuple = tuple:update{{'=', 5, box.priv.ALL}}
+        log.info("Grant all privileges to user 'admin'")
+    end
+    return tuple
+end
+
 --------------------------------------------------------------------------------
 --- Tarantool 1.10.0
 --------------------------------------------------------------------------------
@@ -1021,6 +1204,7 @@ end
 --------------------------------------------------------------------------------
 
 local handlers = {
+    {version = mkversion(1, 7, 5), func = upgrade_to_1_7_5, auto=true},
     {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
     {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
     {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
@@ -1061,13 +1245,98 @@ local function schema_needs_upgrade()
     return false
 end
 
+local trig_oldest_version = nil
+
+-- Some schema changes before version 1.7.7 make it impossible to recover from
+-- older snapshot. The table below consists of before_replace triggers on system
+-- spaces, which make old snapshot schema compatible with current Tarantool
+-- (version 2.x). The triggers replace old format tuples with new ones
+-- in-memory, thus making it possible to recover from a rather old snapshot
+-- (up to schema version 1.6.8). Once the snapshot is recovered, a normal
+-- upgrade procedure may set schema version to the latest one.
+--
+-- The triggers mostly repeat the corresponding upgrade_to_1_7_x functions,
+-- which were used when pre-1.7.x snapshot schema was still recoverable.
+--
+-- When the triggers are used (i.e. when snapshot schema version is below 1.7.5,
+-- the upgrade procedure works as follows:
+-- * first the snapshot is recovered and 1.7.5-compatible schema is applied to
+--   it in-memory with the help of triggers.
+-- * then usual upgrade_to_X_X_X() handlers may be fired to turn schema into the
+--   latest one.
+local recovery_triggers = {
+    {version = mkversion(1, 7, 1), tbl = {
+        _user   = user_trig_1_7_1,
+    }},
+    {version = mkversion(1, 7, 2), tbl = {
+        _index = index_trig_1_7_2,
+    }},
+    {version = mkversion(1, 7, 5), tbl = {
+        _space = space_trig_1_7_5,
+        _user  = user_trig_1_7_5,
+    }},
+    {version = mkversion(1, 7, 7), tbl = {
+        _priv   = priv_trig_1_7_7,
+    }},
+}
+
+-- Once newer schema version is recovered (say, from an xlog following the old
+-- snapshot), the triggers helping recover the old schema should be removed.
+local function schema_trig_last(_, tuple)
+    if tuple and tuple[1] == 'version' then
+        local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
+        if major and minor and patch and type(major) == 'number' and
+           type(minor) == 'number' and type(patch) == 'number' then
+            local version = mkversion(major, minor, patch)
+            log.info("Recovery trigger: recovered schema version %s. "..
+                     "Removing outdated recovery triggers.", version)
+            box.internal.clear_recovery_triggers(version)
+            trig_oldest_version = version
+        end
+    end
+    return tuple
+end
+
+recovery_triggers[#recovery_triggers].tbl['_schema'] = schema_trig_last
+
+local function on_init_set_recovery_triggers()
+    log.info("Recovering snapshot with schema version %s", trig_oldest_version)
+    for _, trig_tbl in ipairs(recovery_triggers) do
+        if trig_tbl.version > trig_oldest_version then
+            for space, trig in pairs(trig_tbl.tbl) do
+                box.space[space]:before_replace(trig)
+                log.info("Set recovery trigger on space '%s' to comply with "..
+                         "version %s format", space, trig_tbl.version)
+            end
+        end
+    end
+end
+
+local function set_recovery_triggers(version)
+    trig_oldest_version = version
+    box.ctl.on_schema_init(on_init_set_recovery_triggers)
+end
+
+local function clear_recovery_triggers(version)
+    for _, trig_tbl in ipairs(recovery_triggers) do
+        if trig_tbl.version > trig_oldest_version and
+           (not version or trig_tbl.version <= version) then
+            for space, trig in pairs(trig_tbl.tbl) do
+                box.space[space]:before_replace(nil, trig)
+                log.info("Remove recovery trigger on space '%s' for version %s",
+                         space, trig_tbl.version)
+            end
+        end
+    end
+end
+
 local function upgrade(options)
     options = options or {}
     setmetatable(options, {__index = {auto = false}})
 
     local version = get_version()
-    if version < mkversion(1, 7, 5) then
-        log.warn('can upgrade from 1.7.5 only')
+    if version < mkversion(1, 6, 8) then
+        log.warn('can upgrade from 1.6.8 only')
         return
     end
 
@@ -1110,3 +1379,6 @@ end
 box.schema.upgrade = upgrade;
 box.internal.bootstrap = bootstrap;
 box.internal.schema_needs_upgrade = schema_needs_upgrade;
+box.internal.get_snapshot_version = get_snapshot_version;
+box.internal.set_recovery_triggers = set_recovery_triggers;
+box.internal.clear_recovery_triggers = clear_recovery_triggers;
diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.result b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
new file mode 100644
index 000000000..aba5a56ab
--- /dev/null
+++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
@@ -0,0 +1,400 @@
+-- test-run result file version 2
+test_run = require('test_run').new()
+ | ---
+ | ...
+
+-- Upgrade from 1.6.8.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:switch('upgrade')
+ | ---
+ | - true
+ | ...
+
+assert(not box.internal.schema_needs_upgrade())
+ | ---
+ | - true
+ | ...
+box.space.distro:select{}
+ | ---
+ | - - ['debian', 'sarge', 31, 1118059200]
+ |   - ['debian', 'etch', 40, 1176033600]
+ |   - ['ubuntu', 'trusty', 1404, 1397736000]
+ |   - ['ubuntu', 'vivid', 1504, 1429790400]
+ |   - ['ubuntu', 'wily', 1510, 1445515200]
+ |   - ['debian', 'wheezy', 70, 1367668800]
+ |   - ['debian', 'squeeze', 60, 1296907200]
+ |   - ['debian', 'lenny', 50, 1234612800]
+ |   - ['debian', 'jessie', 80, 1430049600]
+ |   - ['ubuntu', 'precise', 1510, 1335441600]
+ |   - ['debian', 'woody', 30, 1027080000]
+ | ...
+box.space._index:select{box.space.distro.id}
+ | ---
+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
+ |         2, 'unsigned']]]
+ |   - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
+ |   - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
+ | ...
+box.space._space:format()
+ | ---
+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
+ |     'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
+ |     'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
+ | ...
+box.schema.user.info('admin')
+ | ---
+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('guest')
+ | ---
+ | - - - execute
+ |     - role
+ |     - public
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('someuser')
+ | ---
+ | - - - execute
+ |     - function
+ |     - someotherfunc
+ |   - - execute
+ |     - role
+ |     - public
+ |   - - execute
+ |     - role
+ |     - somerole
+ |   - - read,write,drop,alter
+ |     - space
+ |     - temporary
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.role.info('somerole')
+ | ---
+ | - - - read,write,drop,alter
+ |     - space
+ |     - distro
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server upgrade')
+ | ---
+ | - true
+ | ...
+
+-- Upgrade from 1.7.1.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:switch('upgrade')
+ | ---
+ | - true
+ | ...
+
+assert(not box.internal.schema_needs_upgrade())
+ | ---
+ | - true
+ | ...
+box.space.distro:select{}
+ | ---
+ | - - ['debian', 'etch', 40, 1176033600]
+ |   - ['debian', 'sarge', 31, 1118059200]
+ |   - ['ubuntu', 'wily', 1510, 1445515200]
+ |   - ['ubuntu', 'trusty', 1404, 1397736000]
+ |   - ['ubuntu', 'vivid', 1504, 1429790400]
+ |   - ['debian', 'wheezy', 70, 1367668800]
+ |   - ['debian', 'squeeze', 60, 1296907200]
+ |   - ['debian', 'lenny', 50, 1234612800]
+ |   - ['debian', 'jessie', 80, 1430049600]
+ |   - ['ubuntu', 'precise', 1510, 1335441600]
+ |   - ['debian', 'woody', 30, 1027080000]
+ | ...
+box.space._index:select{box.space.distro.id}
+ | ---
+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
+ |         2, 'unsigned']]]
+ |   - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
+ |   - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
+ | ...
+box.space._space:format()
+ | ---
+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
+ |     'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
+ |     'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
+ | ...
+box.schema.user.info('admin')
+ | ---
+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('guest')
+ | ---
+ | - - - execute
+ |     - role
+ |     - public
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('someuser')
+ | ---
+ | - - - execute
+ |     - function
+ |     - someotherfunc
+ |   - - execute
+ |     - role
+ |     - public
+ |   - - execute
+ |     - role
+ |     - somerole
+ |   - - read,write,drop,alter
+ |     - space
+ |     - temporary
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.role.info('somerole')
+ | ---
+ | - - - read,write,drop,alter
+ |     - space
+ |     - distro
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server upgrade')
+ | ---
+ | - true
+ | ...
+
+-- Upgrade from 1.7.2.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:switch('upgrade')
+ | ---
+ | - true
+ | ...
+
+assert(not box.internal.schema_needs_upgrade())
+ | ---
+ | - true
+ | ...
+box.space.distro:select{}
+ | ---
+ | - - ['debian', 'sarge', 31, 1118059200]
+ |   - ['debian', 'etch', 40, 1176033600]
+ |   - ['ubuntu', 'trusty', 1404, 1397736000]
+ |   - ['ubuntu', 'vivid', 1504, 1429790400]
+ |   - ['debian', 'lenny', 50, 1234612800]
+ |   - ['debian', 'wheezy', 70, 1367668800]
+ |   - ['debian', 'squeeze', 60, 1296907200]
+ |   - ['ubuntu', 'wily', 1510, 1445515200]
+ |   - ['debian', 'jessie', 80, 1430049600]
+ |   - ['ubuntu', 'precise', 1510, 1335441600]
+ |   - ['debian', 'woody', 30, 1027080000]
+ | ...
+box.space._index:select{box.space.distro.id}
+ | ---
+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
+ |         2, 'unsigned']]]
+ |   - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
+ |   - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
+ | ...
+box.space._space:format()
+ | ---
+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
+ |     'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
+ |     'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
+ | ...
+box.schema.user.info('admin')
+ | ---
+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('guest')
+ | ---
+ | - - - execute
+ |     - role
+ |     - public
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('someuser')
+ | ---
+ | - - - execute
+ |     - function
+ |     - someotherfunc
+ |   - - execute
+ |     - role
+ |     - public
+ |   - - execute
+ |     - role
+ |     - somerole
+ |   - - read,write,drop,alter
+ |     - space
+ |     - temporary
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.role.info('somerole')
+ | ---
+ | - - - read,write,drop,alter
+ |     - space
+ |     - distro
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server upgrade')
+ | ---
+ | - true
+ | ...
+
+-- Upgrade from 1.7.5.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
+ | ---
+ | - true
+ | ...
+test_run:cmd('start server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:switch('upgrade')
+ | ---
+ | - true
+ | ...
+
+assert(not box.internal.schema_needs_upgrade())
+ | ---
+ | - true
+ | ...
+box.space.distro:select{}
+ | ---
+ | - - ['debian', 'etch', 40, 1176033600]
+ |   - ['debian', 'sarge', 31, 1118059200]
+ |   - ['debian', 'lenny', 50, 1234612800]
+ |   - ['ubuntu', 'trusty', 1404, 1397736000]
+ |   - ['ubuntu', 'vivid', 1504, 1429790400]
+ |   - ['debian', 'wheezy', 70, 1367668800]
+ |   - ['debian', 'squeeze', 60, 1296907200]
+ |   - ['ubuntu', 'wily', 1510, 1445515200]
+ |   - ['debian', 'jessie', 80, 1430049600]
+ |   - ['ubuntu', 'precise', 1510, 1335441600]
+ |   - ['debian', 'woody', 30, 1027080000]
+ | ...
+box.space._index:select{box.space.distro.id}
+ | ---
+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
+ |         2, 'unsigned']]]
+ |   - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
+ |   - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
+ | ...
+box.space._space:format()
+ | ---
+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
+ |     'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
+ |     'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
+ | ...
+box.schema.user.info('admin')
+ | ---
+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('guest')
+ | ---
+ | - - - execute
+ |     - role
+ |     - public
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.user.info('someuser')
+ | ---
+ | - - - execute
+ |     - function
+ |     - someotherfunc
+ |   - - execute
+ |     - role
+ |     - public
+ |   - - execute
+ |     - role
+ |     - somerole
+ |   - - read,write,drop,alter
+ |     - space
+ |     - temporary
+ |   - - session,usage
+ |     - universe
+ |     - 
+ | ...
+box.schema.role.info('somerole')
+ | ---
+ | - - - read,write,drop,alter
+ |     - space
+ |     - distro
+ | ...
+
+test_run:switch('default')
+ | ---
+ | - true
+ | ...
+test_run:cmd('stop server upgrade')
+ | ---
+ | - true
+ | ...
+test_run:cmd('delete server upgrade')
+ | ---
+ | - true
+ | ...
diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
new file mode 100644
index 000000000..9096bcb7a
--- /dev/null
+++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
@@ -0,0 +1,77 @@
+test_run = require('test_run').new()
+
+-- Upgrade from 1.6.8.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
+test_run:cmd('start server upgrade')
+test_run:switch('upgrade')
+
+assert(not box.internal.schema_needs_upgrade())
+box.space.distro:select{}
+box.space._index:select{box.space.distro.id}
+box.space._space:format()
+box.schema.user.info('admin')
+box.schema.user.info('guest')
+box.schema.user.info('someuser')
+box.schema.role.info('somerole')
+
+test_run:switch('default')
+test_run:cmd('stop server upgrade')
+test_run:cmd('delete server upgrade')
+
+-- Upgrade from 1.7.1.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
+test_run:cmd('start server upgrade')
+test_run:switch('upgrade')
+
+assert(not box.internal.schema_needs_upgrade())
+box.space.distro:select{}
+box.space._index:select{box.space.distro.id}
+box.space._space:format()
+box.schema.user.info('admin')
+box.schema.user.info('guest')
+box.schema.user.info('someuser')
+box.schema.role.info('somerole')
+
+test_run:switch('default')
+test_run:cmd('stop server upgrade')
+test_run:cmd('delete server upgrade')
+
+-- Upgrade from 1.7.2.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
+test_run:cmd('start server upgrade')
+test_run:switch('upgrade')
+
+assert(not box.internal.schema_needs_upgrade())
+box.space.distro:select{}
+box.space._index:select{box.space.distro.id}
+box.space._space:format()
+box.schema.user.info('admin')
+box.schema.user.info('guest')
+box.schema.user.info('someuser')
+box.schema.role.info('somerole')
+
+test_run:switch('default')
+test_run:cmd('stop server upgrade')
+test_run:cmd('delete server upgrade')
+
+-- Upgrade from 1.7.5.
+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
+             workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
+test_run:cmd('start server upgrade')
+test_run:switch('upgrade')
+
+assert(not box.internal.schema_needs_upgrade())
+box.space.distro:select{}
+box.space._index:select{box.space.distro.id}
+box.space._space:format()
+box.schema.user.info('admin')
+box.schema.user.info('guest')
+box.schema.user.info('someuser')
+box.schema.role.info('somerole')
+
+test_run:switch('default')
+test_run:cmd('stop server upgrade')
+test_run:cmd('delete server upgrade')
diff --git a/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
new file mode 120000
index 000000000..2f2a84962
--- /dev/null
+++ b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
@@ -0,0 +1 @@
+../../fill.lua
\ No newline at end of file
diff --git a/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
new file mode 120000
index 000000000..2f2a84962
--- /dev/null
+++ b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
@@ -0,0 +1 @@
+../../fill.lua
\ No newline at end of file
diff --git a/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
new file mode 120000
index 000000000..2f2a84962
--- /dev/null
+++ b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
@@ -0,0 +1 @@
+../../fill.lua
\ No newline at end of file
diff --git a/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
new file mode 120000
index 000000000..2f2a84962
--- /dev/null
+++ b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
@@ -0,0 +1 @@
+../../fill.lua
\ No newline at end of file
diff --git a/test/xlog/upgrade/fill.lua b/test/xlog/upgrade/fill.lua
index 0ef1a8bb9..310c1ca72 100644
--- a/test/xlog/upgrade/fill.lua
+++ b/test/xlog/upgrade/fill.lua
@@ -56,4 +56,8 @@ end
 box.schema.func.create('someotherfunc')
 box.schema.user.grant('someuser', 'execute', 'function', 'someotherfunc')
 box.schema.user.grant('someuser', 'read,write', 'space', 'temporary')
+
+box.schema.upgrade()
+box.snapshot()
+
 os.exit(0)
-- 
2.30.1 (Apple Git-130)

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Oleg Babin via Tarantool-patches]

Thanks for your patch.


It seems it works in quite strange way:

```

tarantool> table.equals({a = box.NULL}, {})
---
- true
...

tarantool> table.equals({}, {a = box.NULL})
---
- false
...

```


I can change arguments order to get different result. Expected false in 
both cases.

For tap it was considered as buggy behaviour 
https://github.com/tarantool/tarantool/issues/4125


On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
> Introduce table.equals for comparing tables.
> The method respects __eq metamethod, if provided.
>
> Needed-for #5894
> ---
>

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Serge Petrenko via Tarantool-patches]

13.08.2021 08:30, Oleg Babin пишет:
> Thanks for your patch.
>
>
> It seems it works in quite strange way:
>
> ```
>
> tarantool> table.equals({a = box.NULL}, {})
> ---
> - true
> ...
>
> tarantool> table.equals({}, {a = box.NULL})
> ---
> - false
> ...
>
> ```
>
>
> I can change arguments order to get different result. Expected false 
> in both cases.
>
> For tap it was considered as buggy behaviour 
> https://github.com/tarantool/tarantool/issues/4125
>
>

====================================

Good catch! Thanks!

Check out the diff:

diff --git a/src/lua/table.lua b/src/lua/table.lua
index 5f35a30f6..3d5e69e97 100644
--- a/src/lua/table.lua
+++ b/src/lua/table.lua
@@ -63,7 +63,7 @@ end
  -- @return true when the two tables are equal (false otherwise).
  local function table_equals(a, b)
      if type(a) ~= 'table' or type(b) ~= 'table' then
-        return a == b
+        return type(a) == type(b) and a == b
      end
      local mt = getmetatable(a)
      if mt and mt.__eq then
diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
index a3c9aa123..ec81593f3 100755
--- a/test/app-tap/table.test.lua
+++ b/test/app-tap/table.test.lua
@@ -8,7 +8,7 @@ yaml.cfg{
      encode_invalid_as_nil  = true,
  }
  local test = require('tap').test('table')
-test:plan(38)
+test:plan(40)

  do -- check basic table.copy (deepcopy)
      local example_table = {
@@ -227,6 +227,10 @@ do -- check table.equals
      test:ok(table.equals({}, {}), "table.equals for empty tables")
      test:is(table.equals({}, {1}), false, "table.equals with one empty 
table")
      test:is(table.equals({1}, {}), false, "table.equals with one empty 
table")
+    test:is(table.equals({key = box.NULL}, {key = nil}), false,
+            "table.equals for box.NULL and nil")
+    test:is(table.equals({key = nil}, {key = box.NULL}), false,
+            "table.equals for box.NULL and nil")
      local tbl_a = {
          first = {
              1,

====================================
> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>> Introduce table.equals for comparing tables.
>> The method respects __eq metamethod, if provided.
>>
>> Needed-for #5894
>> ---
>>

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH v2 2/2] box: allow upgrading from version 1.6

[Vladimir Davydov via Tarantool-patches]

On Fri, Aug 13, 2021 at 02:30:32AM +0300, Serge Petrenko wrote:
> Direct upgrade support from pre-1.7.5 versions was removed in commit
> 7d3b80e78206c479cab75b4893629cfa1932252e
> (Forbid upgrade from Tarantool < 1.7.5 and refactor upgrade.lua)
> The reason for that was the mandatory space format checks introduced
> back then. With these space format checks, old schema couldn't be
> recovered on new Tarantool versions, because newer versions had
> different system space formats. So old schema couldn't be upgraded
> because it couldn't even be recovered.
> 
> Actually this was rather inconvenient. One had to perform an extra
> upgrade step when upgrading from, say, 1.6 to 2.x: instead of
> performing a direct upgrade one had to do 1.6 -> 1.10 -> 2.x upgrade
> which takes twice the time.
> 
> Make it possible to boot from snapshots coming from Tarantool version
> 1.6.8 and above.
> 
> In order to do so, introduce before_replace triggers on system spaces,
> which work during snapshot/xlog recovery. The triggers will set tuple
> formats to the ones supported by current Tarantool (2.x). This way the
> recovered data will have the correct format for a usual schema upgrade.
> 
> Also add upgrade_to_1_7_5() handler, which finishes transformation of
> old schema to 1.7.5. The handler is fired together with other
> box.schema.upgrade() handlers, so there's no user-visible behaviour
> change.
> 
> Side note: it would be great to use the same technique to allow booting
> from pre-1.6.8 snapshots. Unfortunately, this is not possible.
> 
> Current triggers don't break the order of schema upgrades, so 1.7.1
> upgrades come before 1.7.2 and 1.7.5. This is because all the upgrades
> in these versions are replacing existing tuples and not inserting new
> ones, so the upgrades may be handled by the before_replace triggers.
> 
> Upgrade to 1.6.8 requires inserting new tuples: creating sysviews, like
> _vspace, _vuser and so on. This can't be done from the before_replace
> triggers, so we would have to run triggers for 1.7.x first which would
> allow Tarantool to recover the snapshot, and then run an upgrade handler for
> 1.6.8. This looks really messy.
> 
> Closes #5894
> ---
>  src/box/lua/load_cfg.lua                      |  14 +
>  src/box/lua/upgrade.lua                       | 276 +++++++++++-
>  test/xlog/gh-5894-pre-1.7.7-upgrade.result    | 400 ++++++++++++++++++
>  test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua  |  77 ++++
>  .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
>  .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
>  .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
>  .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua  |   1 +
>  test/xlog/upgrade/fill.lua                    |   4 +
>  9 files changed, 773 insertions(+), 2 deletions(-)
>  create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
>  create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>  create mode 120000 test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>  create mode 120000 test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>  create mode 120000 test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>  create mode 120000 test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua

LGTM

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Vladimir Davydov via Tarantool-patches]

On Fri, Aug 13, 2021 at 02:30:31AM +0300, Serge Petrenko wrote:
> Introduce table.equals for comparing tables.
> The method respects __eq metamethod, if provided.
> 
> Needed-for #5894
> ---
>  src/lua/table.lua           | 26 ++++++++++++++++++++++++++
>  test/app-tap/table.test.lua | 31 ++++++++++++++++++++++++++++++-
>  2 files changed, 56 insertions(+), 1 deletion(-)

LGTM (with the fix).

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Oleg Babin via Tarantool-patches]

Thanks for your fixes. Seems I found one more issue.

```

mt = {__eq = function(a, b) return a.a == b.a end}

t1 = setmetatable({a = 3}, mt)

t2 = setmetatable({a = 3}, mt)

tarantool> t1 == t2 -- metamethod is triggered
---
- true
...

tarantool> t1 == {a = 3} -- tables have different metatables
---
- false
...

tarantool> {a = 3} == t1
---
- false
...

tarantool> table.equals(t1, t2) -- OK
---
- true
...

tarantool> table.equals(t1, {a = 3}) -- Seems also ok
---
- false
...

tarantool> table.equals({a = 3}, t1) -- It's strange
---
- true
...

```


Seems you can use two tables if they have the same metatable.

Probably here it's described better 
https://stackoverflow.com/questions/32637684/equality-operator-on-mixed-types-in-lua



On 13.08.2021 13:22, Serge Petrenko wrote:
>
>
> 13.08.2021 08:30, Oleg Babin пишет:
>> Thanks for your patch.
>>
>>
>> It seems it works in quite strange way:
>>
>> ```
>>
>> tarantool> table.equals({a = box.NULL}, {})
>> ---
>> - true
>> ...
>>
>> tarantool> table.equals({}, {a = box.NULL})
>> ---
>> - false
>> ...
>>
>> ```
>>
>>
>> I can change arguments order to get different result. Expected false 
>> in both cases.
>>
>> For tap it was considered as buggy behaviour 
>> https://github.com/tarantool/tarantool/issues/4125
>>
>>
>
> ====================================
>
> Good catch! Thanks!
>
> Check out the diff:
>
> diff --git a/src/lua/table.lua b/src/lua/table.lua
> index 5f35a30f6..3d5e69e97 100644
> --- a/src/lua/table.lua
> +++ b/src/lua/table.lua
> @@ -63,7 +63,7 @@ end
>  -- @return true when the two tables are equal (false otherwise).
>  local function table_equals(a, b)
>      if type(a) ~= 'table' or type(b) ~= 'table' then
> -        return a == b
> +        return type(a) == type(b) and a == b
>      end
>      local mt = getmetatable(a)
>      if mt and mt.__eq then
> diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
> index a3c9aa123..ec81593f3 100755
> --- a/test/app-tap/table.test.lua
> +++ b/test/app-tap/table.test.lua
> @@ -8,7 +8,7 @@ yaml.cfg{
>      encode_invalid_as_nil  = true,
>  }
>  local test = require('tap').test('table')
> -test:plan(38)
> +test:plan(40)
>
>  do -- check basic table.copy (deepcopy)
>      local example_table = {
> @@ -227,6 +227,10 @@ do -- check table.equals
>      test:ok(table.equals({}, {}), "table.equals for empty tables")
>      test:is(table.equals({}, {1}), false, "table.equals with one 
> empty table")
>      test:is(table.equals({1}, {}), false, "table.equals with one 
> empty table")
> +    test:is(table.equals({key = box.NULL}, {key = nil}), false,
> +            "table.equals for box.NULL and nil")
> +    test:is(table.equals({key = nil}, {key = box.NULL}), false,
> +            "table.equals for box.NULL and nil")
>      local tbl_a = {
>          first = {
>              1,
>
> ====================================
>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>>> Introduce table.equals for comparing tables.
>>> The method respects __eq metamethod, if provided.
>>>
>>> Needed-for #5894
>>> ---
>>>
>

Re: [Tarantool-patches] [PATCH v2 0/2] allow upgrading from version 1.6

[Vitaliia Ioffe via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 1788 bytes --]


Hi team, 
 
QA LGTM
 
 
--
Vitaliia Ioffe
 
  
>Пятница, 13 августа 2021, 2:32 +03:00 от Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
> 
>Changes in v2:
>  - Review fixes as per reviews from Sergos, Vladimir
>  - Fix triggers being run on xlogs resulting from
>    box.schema.ugrade()
>  - Introduce table.equals in a separate commit.
>    Use it to define whether some schema changes were
>    already applied.
>
>https://github.com/tarantool/tarantool/issues/5894
>https://github.com/tarantool/tarantool/tree/sp/gh-5894-1.6-upgrade
>
>Serge Petrenko (2):
>  lua: introduce table.equals method
>  box: allow upgrading from version 1.6
>
> src/box/lua/load_cfg.lua | 14 +
> src/box/lua/upgrade.lua | 276 +++++++++++-
> src/lua/table.lua | 26 ++
> test/app-tap/table.test.lua | 31 +-
> test/xlog/gh-5894-pre-1.7.7-upgrade.result | 400 ++++++++++++++++++
> test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua | 77 ++++
> .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> test/xlog/upgrade/fill.lua | 4 +
> 11 files changed, 829 insertions(+), 3 deletions(-)
> create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
> create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
> create mode 120000 test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
> create mode 120000 test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
> create mode 120000 test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
> create mode 120000 test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>
>--
>2.30.1 (Apple Git-130)
 

[-- Attachment #2: Type: text/html, Size: 2657 bytes --]

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Serge Petrenko via Tarantool-patches]

13.08.2021 23:13, Oleg Babin пишет:
> Thanks for your fixes. Seems I found one more issue.
>
> ```
>
> mt = {__eq = function(a, b) return a.a == b.a end}
>
> t1 = setmetatable({a = 3}, mt)
>
> t2 = setmetatable({a = 3}, mt)
>
> tarantool> t1 == t2 -- metamethod is triggered
> ---
> - true
> ...
>
> tarantool> t1 == {a = 3} -- tables have different metatables
> ---
> - false
> ...
>
> tarantool> {a = 3} == t1
> ---
> - false
> ...
>
> tarantool> table.equals(t1, t2) -- OK
> ---
> - true
> ...
>
> tarantool> table.equals(t1, {a = 3}) -- Seems also ok
> ---
> - false
> ...
>
> tarantool> table.equals({a = 3}, t1) -- It's strange
> ---
> - true
> ...
>
> ```
>
>
> Seems you can use two tables if they have the same metatable.
>
> Probably here it's described better 
> https://stackoverflow.com/questions/32637684/equality-operator-on-mixed-types-in-lua
>
>

Thanks for a thorough review!

I've force pushed the following fixes:

=============================================

diff --git a/src/lua/table.lua b/src/lua/table.lua
index 3d5e69e97..edd60d1be 100644
--- a/src/lua/table.lua
+++ b/src/lua/table.lua
@@ -65,8 +65,11 @@ local function table_equals(a, b)
      if type(a) ~= 'table' or type(b) ~= 'table' then
          return type(a) == type(b) and a == b
      end
-    local mt = getmetatable(a)
-    if mt and mt.__eq then
+    local mta = getmetatable(a)
+    local mtb = getmetatable(b)
+    -- Let Lua decide what should happen when at least one of the 
tables has a
+    -- metatable.
+    if mta and mta.__eq or mtb and mtb.__eq then
          return a == b
      end
      for k, v in pairs(a) do
diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
index ec81593f3..029b923fb 100755
--- a/test/app-tap/table.test.lua
+++ b/test/app-tap/table.test.lua
@@ -8,7 +8,7 @@ yaml.cfg{
      encode_invalid_as_nil  = true,
  }
  local test = require('tap').test('table')
-test:plan(40)
+test:plan(43)

  do -- check basic table.copy (deepcopy)
      local example_table = {
@@ -254,6 +254,14 @@ do -- check table.equals
      test:ok(table.equals(tbl_a, tbl_c),
              "table.equals for shallow copied tables after modification")
      test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a 
deep check")
+    local mt = {__eq = function(a, b) return true end}
+    local tbl_d = setmetatable({a=15}, mt)
+    local tbl_e = setmetatable({b=2, c=3}, mt)
+    test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")
+    test:is(table.equals(tbl_d, {a=15}), false,
+            "table.equals when metatables don't match")
+    test:is(table.equals({a=15}, tbl_d), false,
+            "table.equals when metatables don't match")
  end

  os.exit(test:check() == true and 0 or 1)

=============================================

>
> On 13.08.2021 13:22, Serge Petrenko wrote:
>>
>>
>> 13.08.2021 08:30, Oleg Babin пишет:
>>> Thanks for your patch.
>>>
>>>
>>> It seems it works in quite strange way:
>>>
>>> ```
>>>
>>> tarantool> table.equals({a = box.NULL}, {})
>>> ---
>>> - true
>>> ...
>>>
>>> tarantool> table.equals({}, {a = box.NULL})
>>> ---
>>> - false
>>> ...
>>>
>>> ```
>>>
>>>
>>> I can change arguments order to get different result. Expected false 
>>> in both cases.
>>>
>>> For tap it was considered as buggy behaviour 
>>> https://github.com/tarantool/tarantool/issues/4125
>>>
>>>
>>
>> ====================================
>>
>> Good catch! Thanks!
>>
>> Check out the diff:
>>
>> diff --git a/src/lua/table.lua b/src/lua/table.lua
>> index 5f35a30f6..3d5e69e97 100644
>> --- a/src/lua/table.lua
>> +++ b/src/lua/table.lua
>> @@ -63,7 +63,7 @@ end
>>  -- @return true when the two tables are equal (false otherwise).
>>  local function table_equals(a, b)
>>      if type(a) ~= 'table' or type(b) ~= 'table' then
>> -        return a == b
>> +        return type(a) == type(b) and a == b
>>      end
>>      local mt = getmetatable(a)
>>      if mt and mt.__eq then
>> diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>> index a3c9aa123..ec81593f3 100755
>> --- a/test/app-tap/table.test.lua
>> +++ b/test/app-tap/table.test.lua
>> @@ -8,7 +8,7 @@ yaml.cfg{
>>      encode_invalid_as_nil  = true,
>>  }
>>  local test = require('tap').test('table')
>> -test:plan(38)
>> +test:plan(40)
>>
>>  do -- check basic table.copy (deepcopy)
>>      local example_table = {
>> @@ -227,6 +227,10 @@ do -- check table.equals
>>      test:ok(table.equals({}, {}), "table.equals for empty tables")
>>      test:is(table.equals({}, {1}), false, "table.equals with one 
>> empty table")
>>      test:is(table.equals({1}, {}), false, "table.equals with one 
>> empty table")
>> +    test:is(table.equals({key = box.NULL}, {key = nil}), false,
>> +            "table.equals for box.NULL and nil")
>> +    test:is(table.equals({key = nil}, {key = box.NULL}), false,
>> +            "table.equals for box.NULL and nil")
>>      local tbl_a = {
>>          first = {
>>              1,
>>
>> ====================================
>>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>>>> Introduce table.equals for comparing tables.
>>>> The method respects __eq metamethod, if provided.
>>>>
>>>> Needed-for #5894
>>>> ---
>>>>
>>

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Бабин Олег via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 5362 bytes --]


Thanks for your fixes.

  
>Понедельник, 16 августа 2021, 10:53 +03:00 от Serge Petrenko <sergepetrenko@tarantool.org>:
>
>Thanks for a thorough review!
>
>I've force pushed the following fixes:
>
>=============================================
>
>diff --git a/src/lua/table.lua b/src/lua/table.lua
>index 3d5e69e97..edd60d1be 100644
>--- a/src/lua/table.lua
>+++ b/src/lua/table.lua
>@@ -65,8 +65,11 @@ local function table_equals(a, b)
>      if type(a) ~= 'table' or type(b) ~= 'table' then
>          return type(a) == type(b) and a == b
>      end
>-    local mt = getmetatable(a)
>-    if mt and mt.__eq then
>+    local mta = getmetatable(a)
>+    local mtb = getmetatable(b)
>+    -- Let Lua decide what should happen when at least one of the
>tables has a
>+    -- metatable.
>+    if mta and mta.__eq or mtb and mtb.__eq then
 
I’m not sure that it’s correct. I’ve added a reference to SO ( https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables ) and it seems condition should be following
mta and mta.__eq and mta == mtb. Otherwise tables have different __eq metamathods and you should compare their content.
 
I’m not sure that my proposal is completely correct and suggest to ask Igor about it.
 
>          return a == b
>      end
>      for k, v in pairs(a) do
>diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>index ec81593f3..029b923fb 100755
>--- a/test/app-tap/table.test.lua
>+++ b/test/app-tap/table.test.lua
>@@ -8,7 +8,7 @@ yaml.cfg{
>      encode_invalid_as_nil  = true,
>  }
>  local test = require('tap').test('table')
>-test:plan(40)
>+test:plan(43)
>
>  do -- check basic table.copy (deepcopy)
>      local example_table = {
>@@ -254,6 +254,14 @@ do -- check table.equals
>      test:ok(table.equals(tbl_a, tbl_c),
>              "table.equals for shallow copied tables after modification")
>      test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a
>deep check")
>+    local mt = {__eq = function(a, b) return true end}
>+    local tbl_d = setmetatable({a=15}, mt)
>+    local tbl_e = setmetatable({b=2, c=3}, mt)
>+    test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")
>+    test:is(table.equals(tbl_d, {a=15}), false,
>+            "table.equals when metatables don't match")
>+    test:is(table.equals({a=15}, tbl_d), false,
>+            "table.equals when metatables don't match")
>  end
>
>  os.exit(test:check() == true and 0 or 1)
>
>=============================================
>
>>
>> On 13.08.2021 13:22, Serge Petrenko wrote:
>>>
>>>
>>> 13.08.2021 08:30, Oleg Babin пишет:
>>>> Thanks for your patch.
>>>>
>>>>
>>>> It seems it works in quite strange way:
>>>>
>>>> ```
>>>>
>>>> tarantool> table.equals({a = box.NULL}, {})
>>>> ---
>>>> - true
>>>> ...
>>>>
>>>> tarantool> table.equals({}, {a = box.NULL})
>>>> ---
>>>> - false
>>>> ...
>>>>
>>>> ```
>>>>
>>>>
>>>> I can change arguments order to get different result. Expected false
>>>> in both cases.
>>>>
>>>> For tap it was considered as buggy behaviour
>>>>  https://github.com/tarantool/tarantool/issues/4125
>>>>
>>>>
>>>
>>> ====================================
>>>
>>> Good catch! Thanks!
>>>
>>> Check out the diff:
>>>
>>> diff --git a/src/lua/table.lua b/src/lua/table.lua
>>> index 5f35a30f6..3d5e69e97 100644
>>> --- a/src/lua/table.lua
>>> +++ b/src/lua/table.lua
>>> @@ -63,7 +63,7 @@ end
>>>  -- @return true when the two tables are equal (false otherwise).
>>>  local function table_equals(a, b)
>>>      if type(a) ~= 'table' or type(b) ~= 'table' then
>>> -        return a == b
>>> +        return type(a) == type(b) and a == b
>>>      end
>>>      local mt = getmetatable(a)
>>>      if mt and mt.__eq then
>>> diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>>> index a3c9aa123..ec81593f3 100755
>>> --- a/test/app-tap/table.test.lua
>>> +++ b/test/app-tap/table.test.lua
>>> @@ -8,7 +8,7 @@ yaml.cfg{
>>>      encode_invalid_as_nil  = true,
>>>  }
>>>  local test = require('tap').test('table')
>>> -test:plan(38)
>>> +test:plan(40)
>>>
>>>  do -- check basic table.copy (deepcopy)
>>>      local example_table = {
>>> @@ -227,6 +227,10 @@ do -- check table.equals
>>>      test:ok(table.equals({}, {}), "table.equals for empty tables")
>>>      test:is(table.equals({}, {1}), false, "table.equals with one
>>> empty table")
>>>      test:is(table.equals({1}, {}), false, "table.equals with one
>>> empty table")
>>> +    test:is(table.equals({key = box.NULL}, {key = nil}), false,
>>> +            "table.equals for box.NULL and nil")
>>> +    test:is(table.equals({key = nil}, {key = box.NULL}), false,
>>> +            "table.equals for box.NULL and nil")
>>>      local tbl_a = {
>>>          first = {
>>>              1,
>>>
>>> ====================================
>>>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>>>>> Introduce table.equals for comparing tables.
>>>>> The method respects __eq metamethod, if provided.
>>>>>
>>>>> Needed-for #5894
>>>>> ---
>>>>>
>>>
>
>--
>Serge Petrenko 
 
 
--
Oleg Babin
 

[-- Attachment #2: Type: text/html, Size: 8140 bytes --]

Re: [Tarantool-patches] [PATCH v2 2/2] box: allow upgrading from version 1.6

[Бабин Олег via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 31493 bytes --]


Thanks for your patch. LGTM but consider several nits below.

  
>Пятница, 13 августа 2021, 2:35 +03:00 от Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
> 
>Direct upgrade support from pre-1.7.5 versions was removed in commit
>7d3b80e78206c479cab75b4893629cfa1932252e
>(Forbid upgrade from Tarantool < 1.7.5 and refactor upgrade.lua)
>The reason for that was the mandatory space format checks introduced
>back then. With these space format checks, old schema couldn't be
>recovered on new Tarantool versions, because newer versions had
>different system space formats. So old schema couldn't be upgraded
>because it couldn't even be recovered.
>
>Actually this was rather inconvenient. One had to perform an extra
>upgrade step when upgrading from, say, 1.6 to 2.x: instead of
>performing a direct upgrade one had to do 1.6 -> 1.10 -> 2.x upgrade
>which takes twice the time.
>
>Make it possible to boot from snapshots coming from Tarantool version
>1.6.8 and above.
>
>In order to do so, introduce before_replace triggers on system spaces,
>which work during snapshot/xlog recovery. The triggers will set tuple
>formats to the ones supported by current Tarantool (2.x). This way the
>recovered data will have the correct format for a usual schema upgrade.
>
>Also add upgrade_to_1_7_5() handler, which finishes transformation of
>old schema to 1.7.5. The handler is fired together with other
>box.schema.upgrade() handlers, so there's no user-visible behaviour
>change.
>
>Side note: it would be great to use the same technique to allow booting
>from pre-1.6.8 snapshots. Unfortunately, this is not possible.
>
>Current triggers don't break the order of schema upgrades, so 1.7.1
>upgrades come before 1.7.2 and 1.7.5. This is because all the upgrades
>in these versions are replacing existing tuples and not inserting new
>ones, so the upgrades may be handled by the before_replace triggers.
>
>Upgrade to 1.6.8 requires inserting new tuples: creating sysviews, like
>_vspace, _vuser and so on. This can't be done from the before_replace
>triggers, so we would have to run triggers for 1.7.x first which would
>allow Tarantool to recover the snapshot, and then run an upgrade handler for
>1.6.8. This looks really messy.
>
>Closes #5894
>---
> src/box/lua/load_cfg.lua | 14 +
> src/box/lua/upgrade.lua | 276 +++++++++++-
> test/xlog/gh-5894-pre-1.7.7-upgrade.result | 400 ++++++++++++++++++
> test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua | 77 ++++
> .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
> test/xlog/upgrade/fill.lua | 4 +
> 9 files changed, 773 insertions(+), 2 deletions(-)
> create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
> create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
> create mode 120000 test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
> create mode 120000 test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
> create mode 120000 test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
> create mode 120000 test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>
>diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>index 4df70c210..7a8cab3fd 100644
>--- a/src/box/lua/load_cfg.lua
>+++ b/src/box/lua/load_cfg.lua
>@@ -719,9 +719,23 @@ local function load_cfg(cfg)
>             __call = locked(reload_cfg),
>         })
> 
>+ -- Check schema version of the snapshot we're about to recover, if any.
>+ -- Some schema versions (below 1.7.5) are incompatible with Tarantool 2.x
>+ -- When recovering from such an old snapshot, special recovery triggers on
>+ -- system spaces are needed in order to be able to recover and upgrade
>+ -- the schema then.
>+ local snap_dir = box.cfg.memtx_dir
>+ local snap_version = private.get_snapshot_version(snap_dir)
>+ if snap_version then
>+ private.set_recovery_triggers(snap_version)
>+ end
>+
>     -- This call either succeeds or calls panic() / exit().
>     private.cfg_load()
> 
>+ if snap_version then
>+ private.clear_recovery_triggers()
>+ end
>     -- This block does not raise an error: all necessary checks
>     -- already performed in private.cfg_check(). See <dynamic_cfg>
>     -- comment.
>diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>index 6abce50f4..925adab18 100644
>--- a/src/box/lua/upgrade.lua
>+++ b/src/box/lua/upgrade.lua
>@@ -1,6 +1,8 @@
> local log = require('log')
> local bit = require('bit')
> local json = require('json')
>+local fio = require('fio')
>+local xlog = require('xlog')
> 
> -- Guest user id - the default user
> local GUEST = 0
>@@ -86,6 +88,40 @@ local function set_system_triggers(val)
>     foreach_system_space(function(s) s:run_triggers(val) end)
> end
> 
>+-- Get schema version, stored in _schema system space, by reading the latest
>+-- snapshot file from the snap_dir. Useful to define schema_version before
>+-- recovering the snapshot, because some schema versions are too old and cannot
>+-- be recovered normally.
>+local function get_snapshot_version(snap_dir)
>+ local snap_pattern = snap_dir..'/'..string.rep('[0-9]', 20)..'.snap'
 
Probably we could use fio.pathjoin here
 
>+ local snap_list = fio.glob(snap_pattern)
>+ table.sort(snap_list)
>+ local snap = snap_list[#snap_list]
>+ if not snap then
>+ return nil
>+ end
>+ local version = nil
>+ for _, row in xlog.pairs(snap) do
>+ local sid = row.BODY and row.BODY.space_id
>+ if sid == box.schema.SCHEMA_ID then
>+ local tuple = row.BODY.tuple
>+ if tuple and tuple[1] == 'version' then
>+ local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
 
Could it be replaced with tuple:unpack(2, 4)?
 
>+ if major and minor and patch and type(major) == 'number' and
>+ type(minor) == 'number' and type(patch) == 'number' then
 
Here you use type() == number checks. Could it be different? In case of broken snap?
So, we should log it I assume. The same for similar places below.
 
>+ version = mkversion(major, minor, patch)
>+ break
>+ end
>+ end
>+ elseif sid and sid > box.schema.SCHEMA_ID then
>+ -- Exit early if version wasn't found in _schema space.
>+ -- Snapshot rows are ordered by space id.
>+ break
>+ end
>+ end
>+ return version
>+end
>+
> --------------------------------------------------------------------------------
> -- Bootstrap
> --------------------------------------------------------------------------------
>@@ -131,6 +167,144 @@ local function create_sysview(source_id, target_id)
>     end
> end
> 
>+--------------------------------------------------------------------------------
>+-- Tarantool 1.7.1
>+--------------------------------------------------------------------------------
>+local function user_trig_1_7_1(_, tuple)
>+ if tuple and tuple[3] == 'guest' and not tuple[5] then
 
I think it’s better to use explicit check for tuple[5] value.
If it’s a boolean value could it be box.NULL?
 
>+ local auth_method_list = {}
>+ auth_method_list["chap-sha1"] = box.schema.user.password("")
>+ tuple = tuple:update{{'=', 5, auth_method_list}}
>+ log.info("Set empty password to user 'guest'")
>+ end
>+ return tuple
>+end
>+
>+--------------------------------------------------------------------------------
>+-- Tarantool 1.7.2
>+--------------------------------------------------------------------------------
>+local function index_trig_1_7_2(_, tuple)
>+ local field_types_v16 = {
>+ num = 'unsigned',
>+ int = 'integer',
>+ str = 'string',
>+ }
>+ if not tuple then
>+ return tuple
>+ end
>+ local parts = tuple[6]
>+ local changed = false
>+ for _, part in pairs(parts) do
>+ local field_type = part[2]:lower()
>+ if field_types_v16[field_type] ~= nil then
>+ part[2] = field_types_v16[field_type]
>+ changed = true
>+ end
>+ end
>+ if changed then
>+ log.info("Update index '%s' on space '%s': set parts to %s", tuple[3],
>+ box.space[tuple[1]].name, json.encode(parts))
>+ tuple = tuple:update{{'=', 6, parts}}
>+ end
>+ return tuple
>+end
>+
>+--------------------------------------------------------------------------------
>+-- Tarantool 1.7.5
>+--------------------------------------------------------------------------------
>+local function create_truncate_space()
>+ local _truncate = box.space[box.schema.TRUNCATE_ID]
>+
>+ log.info("create space _truncate")
>+ box.space._space:insert{
>+ _truncate.id, ADMIN, '_truncate', 'memtx', 0, setmap({}),
>+ {{name = 'id', type = 'unsigned'}, {name = 'count', type = 'unsigned'}}
>+ }
>+
>+ log.info("create index primary on _truncate")
>+ box.space._index:insert{
>+ _truncate.id, 0, 'primary', 'tree', {unique = true}, {{0, 'unsigned'}}
>+ }
>+
>+ local _priv = box.space[box.schema.PRIV_ID]
>+ _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
>+end
>+
>+local function upgrade_to_1_7_5()
>+ create_truncate_space()
>+end
>+
>+local function user_trig_1_7_5(_, tuple)
>+ if tuple and not tuple[5] then
>+ tuple = tuple:update{{'=', 5, setmap({})}}
>+ log.info("Set empty password to %s '%s'", tuple[4], tuple[3])
>+ end
>+ return tuple
>+end
>+
>+local space_formats_1_7_5 = {
>+ _schema = {
>+ {name = 'key', type = 'string'},
>+ },
>+ _space = {
>+ {name = 'id', type = 'unsigned'},
>+ {name = 'owner', type = 'unsigned'},
>+ {name = 'name', type = 'string'},
>+ {name = 'engine', type = 'string'},
>+ {name = 'field_count', type = 'unsigned'},
>+ {name = 'flags', type = 'map'},
>+ {name = 'format', type = 'array'},
>+ },
>+ _index = {
>+ {name = 'id', type = 'unsigned'},
>+ {name = 'iid', type = 'unsigned'},
>+ {name = 'name', type = 'string'},
>+ {name = 'type', type = 'string'},
>+ {name = 'opts', type = 'map'},
>+ {name = 'parts', type = 'array'},
>+ },
>+ _func = {
>+ {name = 'id', type = 'unsigned'},
>+ {name = 'owner', type = 'unsigned'},
>+ {name = 'name', type = 'string'},
>+ {name = 'setuid', type = 'unsigned'},
>+ },
>+ _user = {
>+ {name = 'id', type = 'unsigned'},
>+ {name = 'owner', type = 'unsigned'},
>+ {name = 'name', type = 'string'},
>+ {name = 'type', type = 'string'},
>+ {name = 'auth', type = 'map'},
>+ },
>+ _priv = {
>+ {name = 'grantor', type = 'unsigned'},
>+ {name = 'grantee', type = 'unsigned'},
>+ {name = 'object_type', type = 'string'},
>+ {name = 'object_id', type = 'unsigned'},
>+ {name = 'privilege', type = 'unsigned'},
>+ },
>+ _cluster = {
>+ {name = 'id', type = 'unsigned'},
>+ {name = 'uuid', type = 'string'},
>+ },
>+}
>+
>+space_formats_1_7_5._vspace = space_formats_1_7_5._space
>+space_formats_1_7_5._vindex = space_formats_1_7_5._index
>+space_formats_1_7_5._vfunc = space_formats_1_7_5._func
>+space_formats_1_7_5._vuser = space_formats_1_7_5._user
>+space_formats_1_7_5._vpriv = space_formats_1_7_5._priv
>+
>+local function space_trig_1_7_5(_, tuple)
>+ if tuple and space_formats_1_7_5[tuple[3]] and
>+ not table.equals(space_formats_1_7_5[tuple[3]], tuple[7]) then
>+ tuple = tuple:update{{'=', 7, space_formats_1_7_5[tuple[3]]}}
>+ log.info("Update space '%s' format: new format %s", tuple[3],
>+ json.encode(tuple[7]))
>+ end
>+ return tuple
>+end
>+
> local function initial_1_7_5()
>     -- stick to the following convention:
>     -- prefer user id (owner id) in field #1
>@@ -452,6 +626,15 @@ local function upgrade_to_1_7_7()
>     _priv:replace({ADMIN, SUPER, 'universe', 0, 4294967295})
> end
> 
>+local function priv_trig_1_7_7(_, tuple)
>+ if tuple and tuple[2] == ADMIN and tuple[3] == 'universe' and
>+ tuple[5] ~= box.priv.ALL then
>+ tuple = tuple:update{{'=', 5, box.priv.ALL}}
>+ log.info("Grant all privileges to user 'admin'")
>+ end
>+ return tuple
>+end
>+
> --------------------------------------------------------------------------------
> --- Tarantool 1.10.0
> --------------------------------------------------------------------------------
>@@ -1021,6 +1204,7 @@ end
> --------------------------------------------------------------------------------
> 
> local handlers = {
>+ {version = mkversion(1, 7, 5), func = upgrade_to_1_7_5, auto=true},
>     {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto = true},
>     {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto = true},
>     {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0, auto = true},
>@@ -1061,13 +1245,98 @@ local function schema_needs_upgrade()
>     return false
> end
> 
>+local trig_oldest_version = nil
>+
>+-- Some schema changes before version 1.7.7 make it impossible to recover from
>+-- older snapshot. The table below consists of before_replace triggers on system
>+-- spaces, which make old snapshot schema compatible with current Tarantool
>+-- (version 2.x). The triggers replace old format tuples with new ones
>+-- in-memory, thus making it possible to recover from a rather old snapshot
>+-- (up to schema version 1.6.8). Once the snapshot is recovered, a normal
>+-- upgrade procedure may set schema version to the latest one.
>+--
>+-- The triggers mostly repeat the corresponding upgrade_to_1_7_x functions,
>+-- which were used when pre-1.7.x snapshot schema was still recoverable.
>+--
>+-- When the triggers are used (i.e. when snapshot schema version is below 1.7.5,
>+-- the upgrade procedure works as follows:
>+-- * first the snapshot is recovered and 1.7.5-compatible schema is applied to
>+-- it in-memory with the help of triggers.
>+-- * then usual upgrade_to_X_X_X() handlers may be fired to turn schema into the
>+-- latest one.
>+local recovery_triggers = {
>+ {version = mkversion(1, 7, 1), tbl = {
>+ _user = user_trig_1_7_1,
>+ }},
>+ {version = mkversion(1, 7, 2), tbl = {
>+ _index = index_trig_1_7_2,
>+ }},
>+ {version = mkversion(1, 7, 5), tbl = {
>+ _space = space_trig_1_7_5,
>+ _user = user_trig_1_7_5,
>+ }},
>+ {version = mkversion(1, 7, 7), tbl = {
>+ _priv = priv_trig_1_7_7,
>+ }},
>+}
>+
>+-- Once newer schema version is recovered (say, from an xlog following the old
>+-- snapshot), the triggers helping recover the old schema should be removed.
>+local function schema_trig_last(_, tuple)
>+ if tuple and tuple[1] == 'version' then
>+ local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
>+ if major and minor and patch and type(major) == 'number' and
>+ type(minor) == 'number' and type(patch) == 'number' then
>+ local version = mkversion(major, minor, patch)
>+ log.info("Recovery trigger: recovered schema version %s. "..
>+ "Removing outdated recovery triggers.", version)
>+ box.internal.clear_recovery_triggers(version)
>+ trig_oldest_version = version
>+ end
>+ end
>+ return tuple
>+end
>+
>+recovery_triggers[#recovery_triggers].tbl['_schema'] = schema_trig_last
>+
>+local function on_init_set_recovery_triggers()
>+ log.info("Recovering snapshot with schema version %s", trig_oldest_version)
>+ for _, trig_tbl in ipairs(recovery_triggers) do
>+ if trig_tbl.version > trig_oldest_version then
>+ for space, trig in pairs(trig_tbl.tbl) do
>+ box.space[space]:before_replace(trig)
>+ log.info("Set recovery trigger on space '%s' to comply with "..
>+ "version %s format", space, trig_tbl.version)
>+ end
>+ end
>+ end
>+end
>+
>+local function set_recovery_triggers(version)
>+ trig_oldest_version = version
>+ box.ctl.on_schema_init(on_init_set_recovery_triggers)
>+end
>+
>+local function clear_recovery_triggers(version)
>+ for _, trig_tbl in ipairs(recovery_triggers) do
>+ if trig_tbl.version > trig_oldest_version and
>+ (not version or trig_tbl.version <= version) then
>+ for space, trig in pairs(trig_tbl.tbl) do
>+ box.space[space]:before_replace(nil, trig)
>+ log.info("Remove recovery trigger on space '%s' for version %s",
>+ space, trig_tbl.version)
>+ end
>+ end
>+ end
>+end
>+
> local function upgrade(options)
>     options = options or {}
>     setmetatable(options, {__index = {auto = false}})
> 
>     local version = get_version()
>- if version < mkversion(1, 7, 5) then
>- log.warn('can upgrade from 1.7.5 only')
>+ if version < mkversion(1, 6, 8) then
>+ log.warn('can upgrade from 1.6.8 only')
>         return
>     end
> 
>@@ -1110,3 +1379,6 @@ end
> box.schema.upgrade = upgrade;
> box.internal.bootstrap = bootstrap;
> box.internal.schema_needs_upgrade = schema_needs_upgrade;
>+box.internal.get_snapshot_version = get_snapshot_version;
>+box.internal.set_recovery_triggers = set_recovery_triggers;
>+box.internal.clear_recovery_triggers = clear_recovery_triggers;
>diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.result b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
>new file mode 100644
>index 000000000..aba5a56ab
>--- /dev/null
>+++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
>@@ -0,0 +1,400 @@
>+-- test-run result file version 2
>+test_run = require('test_run').new()
>+ | ---
>+ | ...
>+
>+-- Upgrade from 1.6.8.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('start server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:switch('upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+assert(not box.internal.schema_needs_upgrade())
>+ | ---
>+ | - true
>+ | ...
>+box.space.distro:select{}
>+ | ---
>+ | - - ['debian', 'sarge', 31, 1118059200]
>+ | - ['debian', 'etch', 40, 1176033600]
>+ | - ['ubuntu', 'trusty', 1404, 1397736000]
>+ | - ['ubuntu', 'vivid', 1504, 1429790400]
>+ | - ['ubuntu', 'wily', 1510, 1445515200]
>+ | - ['debian', 'wheezy', 70, 1367668800]
>+ | - ['debian', 'squeeze', 60, 1296907200]
>+ | - ['debian', 'lenny', 50, 1234612800]
>+ | - ['debian', 'jessie', 80, 1430049600]
>+ | - ['ubuntu', 'precise', 1510, 1335441600]
>+ | - ['debian', 'woody', 30, 1027080000]
>+ | ...
>+box.space._index:select{box.space.distro.id}
>+ | ---
>+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
>+ | 2, 'unsigned']]]
>+ | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>+ | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>+ | ...
>+box.space._space:format()
>+ | ---
>+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
>+ | 'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
>+ | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
>+ | ...
>+box.schema.user.info('admin')
>+ | ---
>+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('guest')
>+ | ---
>+ | - - - execute
>+ | - role
>+ | - public
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('someuser')
>+ | ---
>+ | - - - execute
>+ | - function
>+ | - someotherfunc
>+ | - - execute
>+ | - role
>+ | - public
>+ | - - execute
>+ | - role
>+ | - somerole
>+ | - - read,write,drop,alter
>+ | - space
>+ | - temporary
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.role.info('somerole')
>+ | ---
>+ | - - - read,write,drop,alter
>+ | - space
>+ | - distro
>+ | ...
>+
>+test_run:switch('default')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('stop server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('delete server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+-- Upgrade from 1.7.1.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('start server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:switch('upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+assert(not box.internal.schema_needs_upgrade())
>+ | ---
>+ | - true
>+ | ...
>+box.space.distro:select{}
>+ | ---
>+ | - - ['debian', 'etch', 40, 1176033600]
>+ | - ['debian', 'sarge', 31, 1118059200]
>+ | - ['ubuntu', 'wily', 1510, 1445515200]
>+ | - ['ubuntu', 'trusty', 1404, 1397736000]
>+ | - ['ubuntu', 'vivid', 1504, 1429790400]
>+ | - ['debian', 'wheezy', 70, 1367668800]
>+ | - ['debian', 'squeeze', 60, 1296907200]
>+ | - ['debian', 'lenny', 50, 1234612800]
>+ | - ['debian', 'jessie', 80, 1430049600]
>+ | - ['ubuntu', 'precise', 1510, 1335441600]
>+ | - ['debian', 'woody', 30, 1027080000]
>+ | ...
>+box.space._index:select{box.space.distro.id}
>+ | ---
>+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
>+ | 2, 'unsigned']]]
>+ | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>+ | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>+ | ...
>+box.space._space:format()
>+ | ---
>+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
>+ | 'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
>+ | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
>+ | ...
>+box.schema.user.info('admin')
>+ | ---
>+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('guest')
>+ | ---
>+ | - - - execute
>+ | - role
>+ | - public
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('someuser')
>+ | ---
>+ | - - - execute
>+ | - function
>+ | - someotherfunc
>+ | - - execute
>+ | - role
>+ | - public
>+ | - - execute
>+ | - role
>+ | - somerole
>+ | - - read,write,drop,alter
>+ | - space
>+ | - temporary
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.role.info('somerole')
>+ | ---
>+ | - - - read,write,drop,alter
>+ | - space
>+ | - distro
>+ | ...
>+
>+test_run:switch('default')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('stop server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('delete server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+-- Upgrade from 1.7.2.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('start server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:switch('upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+assert(not box.internal.schema_needs_upgrade())
>+ | ---
>+ | - true
>+ | ...
>+box.space.distro:select{}
>+ | ---
>+ | - - ['debian', 'sarge', 31, 1118059200]
>+ | - ['debian', 'etch', 40, 1176033600]
>+ | - ['ubuntu', 'trusty', 1404, 1397736000]
>+ | - ['ubuntu', 'vivid', 1504, 1429790400]
>+ | - ['debian', 'lenny', 50, 1234612800]
>+ | - ['debian', 'wheezy', 70, 1367668800]
>+ | - ['debian', 'squeeze', 60, 1296907200]
>+ | - ['ubuntu', 'wily', 1510, 1445515200]
>+ | - ['debian', 'jessie', 80, 1430049600]
>+ | - ['ubuntu', 'precise', 1510, 1335441600]
>+ | - ['debian', 'woody', 30, 1027080000]
>+ | ...
>+box.space._index:select{box.space.distro.id}
>+ | ---
>+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
>+ | 2, 'unsigned']]]
>+ | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>+ | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>+ | ...
>+box.space._space:format()
>+ | ---
>+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
>+ | 'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
>+ | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
>+ | ...
>+box.schema.user.info('admin')
>+ | ---
>+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('guest')
>+ | ---
>+ | - - - execute
>+ | - role
>+ | - public
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('someuser')
>+ | ---
>+ | - - - execute
>+ | - function
>+ | - someotherfunc
>+ | - - execute
>+ | - role
>+ | - public
>+ | - - execute
>+ | - role
>+ | - somerole
>+ | - - read,write,drop,alter
>+ | - space
>+ | - temporary
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.role.info('somerole')
>+ | ---
>+ | - - - read,write,drop,alter
>+ | - space
>+ | - distro
>+ | ...
>+
>+test_run:switch('default')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('stop server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('delete server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+-- Upgrade from 1.7.5.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('start server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:switch('upgrade')
>+ | ---
>+ | - true
>+ | ...
>+
>+assert(not box.internal.schema_needs_upgrade())
>+ | ---
>+ | - true
>+ | ...
>+box.space.distro:select{}
>+ | ---
>+ | - - ['debian', 'etch', 40, 1176033600]
>+ | - ['debian', 'sarge', 31, 1118059200]
>+ | - ['debian', 'lenny', 50, 1234612800]
>+ | - ['ubuntu', 'trusty', 1404, 1397736000]
>+ | - ['ubuntu', 'vivid', 1504, 1429790400]
>+ | - ['debian', 'wheezy', 70, 1367668800]
>+ | - ['debian', 'squeeze', 60, 1296907200]
>+ | - ['ubuntu', 'wily', 1510, 1445515200]
>+ | - ['debian', 'jessie', 80, 1430049600]
>+ | - ['ubuntu', 'precise', 1510, 1335441600]
>+ | - ['debian', 'woody', 30, 1027080000]
>+ | ...
>+box.space._index:select{box.space.distro.id}
>+ | ---
>+ | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0, 'string'], [1, 'string'], [
>+ | 2, 'unsigned']]]
>+ | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>+ | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>+ | ...
>+box.space._space:format()
>+ | ---
>+ | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner', 'type': 'unsigned'}, {'name': 'name',
>+ | 'type': 'string'}, {'name': 'engine', 'type': 'string'}, {'name': 'field_count',
>+ | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'}, {'name': 'format', 'type': 'array'}]
>+ | ...
>+box.schema.user.info('admin')
>+ | ---
>+ | - - - read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('guest')
>+ | ---
>+ | - - - execute
>+ | - role
>+ | - public
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.user.info('someuser')
>+ | ---
>+ | - - - execute
>+ | - function
>+ | - someotherfunc
>+ | - - execute
>+ | - role
>+ | - public
>+ | - - execute
>+ | - role
>+ | - somerole
>+ | - - read,write,drop,alter
>+ | - space
>+ | - temporary
>+ | - - session,usage
>+ | - universe
>+ | -
>+ | ...
>+box.schema.role.info('somerole')
>+ | ---
>+ | - - - read,write,drop,alter
>+ | - space
>+ | - distro
>+ | ...
>+
>+test_run:switch('default')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('stop server upgrade')
>+ | ---
>+ | - true
>+ | ...
>+test_run:cmd('delete server upgrade')
>+ | ---
>+ | - true
>+ | ...
>diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>new file mode 100644
>index 000000000..9096bcb7a
>--- /dev/null
>+++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>@@ -0,0 +1,77 @@
>+test_run = require('test_run').new()
>+
>+-- Upgrade from 1.6.8.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
>+test_run:cmd('start server upgrade')
>+test_run:switch('upgrade')
>+
>+assert(not box.internal.schema_needs_upgrade())
>+box.space.distro:select{}
>+box.space._index:select{box.space.distro.id}
>+box.space._space:format()
>+box.schema.user.info('admin')
>+box.schema.user.info('guest')
>+box.schema.user.info('someuser')
>+box.schema.role.info('somerole')
>+
>+test_run:switch('default')
>+test_run:cmd('stop server upgrade')
>+test_run:cmd('delete server upgrade')
>+
>+-- Upgrade from 1.7.1.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
>+test_run:cmd('start server upgrade')
>+test_run:switch('upgrade')
>+
>+assert(not box.internal.schema_needs_upgrade())
>+box.space.distro:select{}
>+box.space._index:select{box.space.distro.id}
>+box.space._space:format()
>+box.schema.user.info('admin')
>+box.schema.user.info('guest')
>+box.schema.user.info('someuser')
>+box.schema.role.info('somerole')
>+
>+test_run:switch('default')
>+test_run:cmd('stop server upgrade')
>+test_run:cmd('delete server upgrade')
>+
>+-- Upgrade from 1.7.2.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
>+test_run:cmd('start server upgrade')
>+test_run:switch('upgrade')
>+
>+assert(not box.internal.schema_needs_upgrade())
>+box.space.distro:select{}
>+box.space._index:select{box.space.distro.id}
>+box.space._space:format()
>+box.schema.user.info('admin')
>+box.schema.user.info('guest')
>+box.schema.user.info('someuser')
>+box.schema.role.info('somerole')
>+
>+test_run:switch('default')
>+test_run:cmd('stop server upgrade')
>+test_run:cmd('delete server upgrade')
>+
>+-- Upgrade from 1.7.5.
>+test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>+ workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
>+test_run:cmd('start server upgrade')
>+test_run:switch('upgrade')
>+
>+assert(not box.internal.schema_needs_upgrade())
>+box.space.distro:select{}
>+box.space._index:select{box.space.distro.id}
>+box.space._space:format()
>+box.schema.user.info('admin')
>+box.schema.user.info('guest')
>+box.schema.user.info('someuser')
>+box.schema.role.info('somerole')
>+
>+test_run:switch('default')
>+test_run:cmd('stop server upgrade')
>+test_run:cmd('delete server upgrade')
>diff --git a/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>new file mode 120000
>index 000000000..2f2a84962
>--- /dev/null
>+++ b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>@@ -0,0 +1 @@
>+../../fill.lua
>\ No newline at end of file
>diff --git a/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>new file mode 120000
>index 000000000..2f2a84962
>--- /dev/null
>+++ b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>@@ -0,0 +1 @@
>+../../fill.lua
>\ No newline at end of file
>diff --git a/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>new file mode 120000
>index 000000000..2f2a84962
>--- /dev/null
>+++ b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>@@ -0,0 +1 @@
>+../../fill.lua
>\ No newline at end of file
>diff --git a/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>new file mode 120000
>index 000000000..2f2a84962
>--- /dev/null
>+++ b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>@@ -0,0 +1 @@
>+../../fill.lua
>\ No newline at end of file
>diff --git a/test/xlog/upgrade/fill.lua b/test/xlog/upgrade/fill.lua
>index 0ef1a8bb9..310c1ca72 100644
>--- a/test/xlog/upgrade/fill.lua
>+++ b/test/xlog/upgrade/fill.lua
>@@ -56,4 +56,8 @@ end
> box.schema.func.create('someotherfunc')
> box.schema.user.grant('someuser', 'execute', 'function', 'someotherfunc')
> box.schema.user.grant('someuser', 'read,write', 'space', 'temporary')
>+
>+box.schema.upgrade()
>+box.snapshot()
>+
> os.exit(0)
>--
>2.30.1 (Apple Git-130) 
 
 
--
Oleg Babin
 

[-- Attachment #2: Type: text/html, Size: 35433 bytes --]

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Serge Petrenko via Tarantool-patches]

16.08.2021 16:03, Бабин Олег пишет:
> Thanks for your fixes.
>
>     Понедельник, 16 августа 2021, 10:53 +03:00 от Serge Petrenko
>     <sergepetrenko@tarantool.org>:
>
>     Thanks for a thorough review!
>
>     I've force pushed the following fixes:
>
>     =============================================
>
>     diff --git a/src/lua/table.lua b/src/lua/table.lua
>     index 3d5e69e97..edd60d1be 100644
>     --- a/src/lua/table.lua
>     +++ b/src/lua/table.lua
>     @@ -65,8 +65,11 @@ local function table_equals(a, b)
>           if type(a) ~= 'table' or type(b) ~= 'table' then
>               return type(a) == type(b) and a == b
>           end
>     -    local mt = getmetatable(a)
>     -    if mt and mt.__eq then
>     +    local mta = getmetatable(a)
>     +    local mtb = getmetatable(b)
>     +    -- Let Lua decide what should happen when at least one of the
>     tables has a
>     +    -- metatable.
>     +    if mta and mta.__eq or mtb and mtb.__eq then
>
> I’m not sure that it’s correct. I’ve added a reference to SO 
> (https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables 
> <https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables>) 
> and it seems condition should be following
> mta and mta.__eq and mta == mtb. Otherwise tables have different __eq 
> metamathods and you should compare their content.
> I’m not sure that my proposal is completely correct and suggest to ask 
> Igor about it.

Actually the question you refer to describes Lua 5.3, while LuaJIT 
supports Lua 5.1
The rules for Lua 5.1 are as follows (taken from 
https://www.lua.org/manual/5.1/manual.html#2.4):

Use the __eq metamethod only when both objects have the same one.

So, in my case, if at least one metamethod is defined, try to use 
comparison operator.
If both tables have the same __eq metamethod, Lua will call it.
If the tables don't share the same metamethod, they will be compared like
Lua does usually: by reference. The result will be false then.


I've added one more test though, just in case:

diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
index c7a075aba..ae6fdfa2d 100755
--- a/test/app-tap/table.test.lua
+++ b/test/app-tap/table.test.lua
@@ -8,7 +8,7 @@ yaml.cfg{
      encode_invalid_as_nil  = true,
  }
  local test = require('tap').test('table')
-test:plan(43)
+test:plan(44)

  do -- check basic table.copy (deepcopy)
      local example_table = {
@@ -258,12 +258,15 @@ do -- check table.equals
          __eq = function(a, b) -- luacheck: no unused args
              return true
          end}
-    local tbl_d = setmetatable({a=15}, mt)
-    local tbl_e = setmetatable({b=2, c=3}, mt)
+    local tbl_d = setmetatable({a = 15}, mt)
+    local tbl_e = setmetatable({b = 2, c = 3}, mt)
      test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")
-    test:is(table.equals(tbl_d, {a=15}), false,
+    test:is(table.equals(tbl_d, {a = 15}), false,
              "table.equals when metatables don't match")
-    test:is(table.equals({a=15}, tbl_d), false,
+    test:is(table.equals({a = 15}, tbl_d), false,
+            "table.equals when metatables don't match")
+    local tbl_f = setmetatable({a = 15}, {__eq = function() return true 
end})
+    test:is(table.equals(tbl_d, tbl_f), false,
              "table.equals when metatables don't match")
  end




>               return a == b
>           end
>           for k, v in pairs(a) do
>     diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>     index ec81593f3..029b923fb 100755
>     --- a/test/app-tap/table.test.lua
>     +++ b/test/app-tap/table.test.lua
>     @@ -8,7 +8,7 @@ yaml.cfg{
>           encode_invalid_as_nil  = true,
>       }
>       local test = require('tap').test('table')
>     -test:plan(40)
>     +test:plan(43)
>
>       do -- check basic table.copy (deepcopy)
>           local example_table = {
>     @@ -254,6 +254,14 @@ do -- check table.equals
>           test:ok(table.equals(tbl_a, tbl_c),
>                   "table.equals for shallow copied tables after
>     modification")
>           test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a
>     deep check")
>     +    local mt = {__eq = function(a, b) return true end}
>     +    local tbl_d = setmetatable({a=15}, mt)
>     +    local tbl_e = setmetatable({b=2, c=3}, mt)
>     +    test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")
>     +    test:is(table.equals(tbl_d, {a=15}), false,
>     +            "table.equals when metatables don't match")
>     +    test:is(table.equals({a=15}, tbl_d), false,
>     +            "table.equals when metatables don't match")
>       end
>
>       os.exit(test:check() == true and 0 or 1)
>
>     =============================================
>
>     >
>     > On 13.08.2021 13:22, Serge Petrenko wrote:
>     >>
>     >>
>     >> 13.08.2021 08:30, Oleg Babin пишет:
>     >>> Thanks for your patch.
>     >>>
>     >>>
>     >>> It seems it works in quite strange way:
>     >>>
>     >>> ```
>     >>>
>     >>> tarantool> table.equals({a = box.NULL}, {})
>     >>> ---
>     >>> - true
>     >>> ...
>     >>>
>     >>> tarantool> table.equals({}, {a = box.NULL})
>     >>> ---
>     >>> - false
>     >>> ...
>     >>>
>     >>> ```
>     >>>
>     >>>
>     >>> I can change arguments order to get different result. Expected
>     false
>     >>> in both cases.
>     >>>
>     >>> For tap it was considered as buggy behaviour
>     >>> https://github.com/tarantool/tarantool/issues/4125
>     <https://github.com/tarantool/tarantool/issues/4125>
>     >>>
>     >>>
>     >>
>     >> ====================================
>     >>
>     >> Good catch! Thanks!
>     >>
>     >> Check out the diff:
>     >>
>     >> diff --git a/src/lua/table.lua b/src/lua/table.lua
>     >> index 5f35a30f6..3d5e69e97 100644
>     >> --- a/src/lua/table.lua
>     >> +++ b/src/lua/table.lua
>     >> @@ -63,7 +63,7 @@ end
>     >>  -- @return true when the two tables are equal (false otherwise).
>     >>  local function table_equals(a, b)
>     >>      if type(a) ~= 'table' or type(b) ~= 'table' then
>     >> -        return a == b
>     >> +        return type(a) == type(b) and a == b
>     >>      end
>     >>      local mt = getmetatable(a)
>     >>      if mt and mt.__eq then
>     >> diff --git a/test/app-tap/table.test.lua
>     b/test/app-tap/table.test.lua
>     >> index a3c9aa123..ec81593f3 100755
>     >> --- a/test/app-tap/table.test.lua
>     >> +++ b/test/app-tap/table.test.lua
>     >> @@ -8,7 +8,7 @@ yaml.cfg{
>     >>      encode_invalid_as_nil  = true,
>     >>  }
>     >>  local test = require('tap').test('table')
>     >> -test:plan(38)
>     >> +test:plan(40)
>     >>
>     >>  do -- check basic table.copy (deepcopy)
>     >>      local example_table = {
>     >> @@ -227,6 +227,10 @@ do -- check table.equals
>     >>      test:ok(table.equals({}, {}), "table.equals for empty tables")
>     >>      test:is(table.equals({}, {1}), false, "table.equals with one
>     >> empty table")
>     >>      test:is(table.equals({1}, {}), false, "table.equals with one
>     >> empty table")
>     >> +    test:is(table.equals({key = box.NULL}, {key = nil}), false,
>     >> +            "table.equals for box.NULL and nil")
>     >> +    test:is(table.equals({key = nil}, {key = box.NULL}), false,
>     >> +            "table.equals for box.NULL and nil")
>     >>      local tbl_a = {
>     >>          first = {
>     >>              1,
>     >>
>     >> ====================================
>     >>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>     >>>> Introduce table.equals for comparing tables.
>     >>>> The method respects __eq metamethod, if provided.
>     >>>>
>     >>>> Needed-for #5894
>     >>>> ---
>     >>>>
>     >>
>
>     --
>     Serge Petrenko
>
> --
> Oleg Babin

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Бабин Олег via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 8227 bytes --]


Ok. Thanks for your changes. Don’t forget to changelog entry and docbot request and LGTM then.

  
>Понедельник, 16 августа 2021, 18:36 +03:00 от Serge Petrenko <sergepetrenko@tarantool.org>:
> 
>
>
>16.08.2021 16:03, Бабин Олег пишет:
>> Thanks for your fixes.
>>
>> Понедельник, 16 августа 2021, 10:53 +03:00 от Serge Petrenko
>> < sergepetrenko@tarantool.org >:
>>
>> Thanks for a thorough review!
>>
>> I've force pushed the following fixes:
>>
>> =============================================
>>
>> diff --git a/src/lua/table.lua b/src/lua/table.lua
>> index 3d5e69e97..edd60d1be 100644
>> --- a/src/lua/table.lua
>> +++ b/src/lua/table.lua
>> @@ -65,8 +65,11 @@ local function table_equals(a, b)
>>       if type(a) ~= 'table' or type(b) ~= 'table' then
>>           return type(a) == type(b) and a == b
>>       end
>> -    local mt = getmetatable(a)
>> -    if mt and mt.__eq then
>> +    local mta = getmetatable(a)
>> +    local mtb = getmetatable(b)
>> +    -- Let Lua decide what should happen when at least one of the
>> tables has a
>> +    -- metatable.
>> +    if mta and mta.__eq or mtb and mtb.__eq then
>>
>> I’m not sure that it’s correct. I’ve added a reference to SO
>> ( https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables
>> < https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables >)
>> and it seems condition should be following
>> mta and mta.__eq and mta == mtb. Otherwise tables have different __eq
>> metamathods and you should compare their content.
>> I’m not sure that my proposal is completely correct and suggest to ask
>> Igor about it.
>
>Actually the question you refer to describes Lua 5.3, while LuaJIT
>supports Lua 5.1
>The rules for Lua 5.1 are as follows (taken from
>https://www.lua.org/manual/5.1/manual.html#2.4 ):
>
>Use the __eq metamethod only when both objects have the same one.
>
>So, in my case, if at least one metamethod is defined, try to use
>comparison operator.
>If both tables have the same __eq metamethod, Lua will call it.
>If the tables don't share the same metamethod, they will be compared like
>Lua does usually: by reference. The result will be false then.
>
>
>I've added one more test though, just in case:
>
>diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>index c7a075aba..ae6fdfa2d 100755
>--- a/test/app-tap/table.test.lua
>+++ b/test/app-tap/table.test.lua
>@@ -8,7 +8,7 @@ yaml.cfg{
>      encode_invalid_as_nil  = true,
>  }
>  local test = require('tap').test('table')
>-test:plan(43)
>+test:plan(44)
>
>  do -- check basic table.copy (deepcopy)
>      local example_table = {
>@@ -258,12 +258,15 @@ do -- check table.equals
>          __eq = function(a, b) -- luacheck: no unused args
>              return true
>          end}
>-    local tbl_d = setmetatable({a=15}, mt)
>-    local tbl_e = setmetatable({b=2, c=3}, mt)
>+    local tbl_d = setmetatable({a = 15}, mt)
>+    local tbl_e = setmetatable({b = 2, c = 3}, mt)
>      test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")
>-    test:is(table.equals(tbl_d, {a=15}), false,
>+    test:is(table.equals(tbl_d, {a = 15}), false,
>              "table.equals when metatables don't match")
>-    test:is(table.equals({a=15}, tbl_d), false,
>+    test:is(table.equals({a = 15}, tbl_d), false,
>+            "table.equals when metatables don't match")
>+    local tbl_f = setmetatable({a = 15}, {__eq = function() return true
>end})
>+    test:is(table.equals(tbl_d, tbl_f), false,
>              "table.equals when metatables don't match")
>  end
>
>
>
>
>>           return a == b
>>       end
>>       for k, v in pairs(a) do
>> diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>> index ec81593f3..029b923fb 100755
>> --- a/test/app-tap/table.test.lua
>> +++ b/test/app-tap/table.test.lua
>> @@ -8,7 +8,7 @@ yaml.cfg{
>>       encode_invalid_as_nil  = true,
>>   }
>>   local test = require('tap').test('table')
>> -test:plan(40)
>> +test:plan(43)
>>
>>   do -- check basic table.copy (deepcopy)
>>       local example_table = {
>> @@ -254,6 +254,14 @@ do -- check table.equals
>>       test:ok(table.equals(tbl_a, tbl_c),
>>               "table.equals for shallow copied tables after
>> modification")
>>       test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a
>> deep check")
>> +    local mt = {__eq = function(a, b) return true end}
>> +    local tbl_d = setmetatable({a=15}, mt)
>> +    local tbl_e = setmetatable({b=2, c=3}, mt)
>> +    test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")
>> +    test:is(table.equals(tbl_d, {a=15}), false,
>> +            "table.equals when metatables don't match")
>> +    test:is(table.equals({a=15}, tbl_d), false,
>> +            "table.equals when metatables don't match")
>>   end
>>
>>   os.exit(test:check() == true and 0 or 1)
>>
>> =============================================
>>
>> >
>> > On 13.08.2021 13:22, Serge Petrenko wrote:
>> >>
>> >>
>> >> 13.08.2021 08:30, Oleg Babin пишет:
>> >>> Thanks for your patch.
>> >>>
>> >>>
>> >>> It seems it works in quite strange way:
>> >>>
>> >>> ```
>> >>>
>> >>> tarantool> table.equals({a = box.NULL}, {})
>> >>> ---
>> >>> - true
>> >>> ...
>> >>>
>> >>> tarantool> table.equals({}, {a = box.NULL})
>> >>> ---
>> >>> - false
>> >>> ...
>> >>>
>> >>> ```
>> >>>
>> >>>
>> >>> I can change arguments order to get different result. Expected
>> false
>> >>> in both cases.
>> >>>
>> >>> For tap it was considered as buggy behaviour
>> >>>  https://github.com/tarantool/tarantool/issues/4125
>> < https://github.com/tarantool/tarantool/issues/4125 >
>> >>>
>> >>>
>> >>
>> >> ====================================
>> >>
>> >> Good catch! Thanks!
>> >>
>> >> Check out the diff:
>> >>
>> >> diff --git a/src/lua/table.lua b/src/lua/table.lua
>> >> index 5f35a30f6..3d5e69e97 100644
>> >> --- a/src/lua/table.lua
>> >> +++ b/src/lua/table.lua
>> >> @@ -63,7 +63,7 @@ end
>> >>  -- @return true when the two tables are equal (false otherwise).
>> >>  local function table_equals(a, b)
>> >>      if type(a) ~= 'table' or type(b) ~= 'table' then
>> >> -        return a == b
>> >> +        return type(a) == type(b) and a == b
>> >>      end
>> >>      local mt = getmetatable(a)
>> >>      if mt and mt.__eq then
>> >> diff --git a/test/app-tap/table.test.lua
>> b/test/app-tap/table.test.lua
>> >> index a3c9aa123..ec81593f3 100755
>> >> --- a/test/app-tap/table.test.lua
>> >> +++ b/test/app-tap/table.test.lua
>> >> @@ -8,7 +8,7 @@ yaml.cfg{
>> >>      encode_invalid_as_nil  = true,
>> >>  }
>> >>  local test = require('tap').test('table')
>> >> -test:plan(38)
>> >> +test:plan(40)
>> >>
>> >>  do -- check basic table.copy (deepcopy)
>> >>      local example_table = {
>> >> @@ -227,6 +227,10 @@ do -- check table.equals
>> >>      test:ok(table.equals({}, {}), "table.equals for empty tables")
>> >>      test:is(table.equals({}, {1}), false, "table.equals with one
>> >> empty table")
>> >>      test:is(table.equals({1}, {}), false, "table.equals with one
>> >> empty table")
>> >> +    test:is(table.equals({key = box.NULL}, {key = nil}), false,
>> >> +            "table.equals for box.NULL and nil")
>> >> +    test:is(table.equals({key = nil}, {key = box.NULL}), false,
>> >> +            "table.equals for box.NULL and nil")
>> >>      local tbl_a = {
>> >>          first = {
>> >>              1,
>> >>
>> >> ====================================
>> >>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>> >>>> Introduce table.equals for comparing tables.
>> >>>> The method respects __eq metamethod, if provided.
>> >>>>
>> >>>> Needed-for #5894
>> >>>> ---
>> >>>>
>> >>
>>
>> --
>> Serge Petrenko
>>
>> --
>> Oleg Babin
>
>--
>Serge Petrenko 
 
 
--
Oleg Babin
 

[-- Attachment #2: Type: text/html, Size: 12271 bytes --]

Re: [Tarantool-patches] [PATCH v2 2/2] box: allow upgrading from version 1.6

[Serge Petrenko via Tarantool-patches]

16.08.2021 16:18, Бабин Олег пишет:
> Thanks for your patch. LGTM but consider several nits below.
>

Thanks for the review! Please, find my answers and incremental diff below.
>
>     Пятница, 13 августа 2021, 2:35 +03:00 от Serge Petrenko via
>     Tarantool-patches <tarantool-patches@dev.tarantool.org>:
>     Direct upgrade support from pre-1.7.5 versions was removed in commit
>     7d3b80e78206c479cab75b4893629cfa1932252e
>     (Forbid upgrade from Tarantool < 1.7.5 and refactor upgrade.lua)
>     The reason for that was the mandatory space format checks introduced
>     back then. With these space format checks, old schema couldn't be
>     recovered on new Tarantool versions, because newer versions had
>     different system space formats. So old schema couldn't be upgraded
>     because it couldn't even be recovered.
>
>     Actually this was rather inconvenient. One had to perform an extra
>     upgrade step when upgrading from, say, 1.6 to 2.x: instead of
>     performing a direct upgrade one had to do 1.6 -> 1.10 -> 2.x upgrade
>     which takes twice the time.
>
>     Make it possible to boot from snapshots coming from Tarantool version
>     1.6.8 and above.
>
>     In order to do so, introduce before_replace triggers on system spaces,
>     which work during snapshot/xlog recovery. The triggers will set tuple
>     formats to the ones supported by current Tarantool (2.x). This way the
>     recovered data will have the correct format for a usual schema
>     upgrade.
>
>     Also add upgrade_to_1_7_5() handler, which finishes transformation of
>     old schema to 1.7.5. The handler is fired together with other
>     box.schema.upgrade() handlers, so there's no user-visible behaviour
>     change.
>
>     Side note: it would be great to use the same technique to allow
>     booting
>     from pre-1.6.8 snapshots. Unfortunately, this is not possible.
>
>     Current triggers don't break the order of schema upgrades, so 1.7.1
>     upgrades come before 1.7.2 and 1.7.5. This is because all the upgrades
>     in these versions are replacing existing tuples and not inserting new
>     ones, so the upgrades may be handled by the before_replace triggers.
>
>     Upgrade to 1.6.8 requires inserting new tuples: creating sysviews,
>     like
>     _vspace, _vuser and so on. This can't be done from the before_replace
>     triggers, so we would have to run triggers for 1.7.x first which would
>     allow Tarantool to recover the snapshot, and then run an upgrade
>     handler for
>     1.6.8. This looks really messy.
>
>     Closes #5894
>     ---
>      src/box/lua/load_cfg.lua | 14 +
>      src/box/lua/upgrade.lua | 276 +++++++++++-
>      test/xlog/gh-5894-pre-1.7.7-upgrade.result | 400 ++++++++++++++++++
>      test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua | 77 ++++
>      .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>      .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>      .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>      .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>      test/xlog/upgrade/fill.lua | 4 +
>      9 files changed, 773 insertions(+), 2 deletions(-)
>      create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
>      create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>      create mode 120000
>     test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>      create mode 120000
>     test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>      create mode 120000
>     test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>      create mode 120000
>     test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>
>     diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
>     index 4df70c210..7a8cab3fd 100644
>     --- a/src/box/lua/load_cfg.lua
>     +++ b/src/box/lua/load_cfg.lua
>     @@ -719,9 +719,23 @@ local function load_cfg(cfg)
>                  __call = locked(reload_cfg),
>              })
>
>     + -- Check schema version of the snapshot we're about to recover,
>     if any.
>     + -- Some schema versions (below 1.7.5) are incompatible with
>     Tarantool 2.x
>     + -- When recovering from such an old snapshot, special recovery
>     triggers on
>     + -- system spaces are needed in order to be able to recover and
>     upgrade
>     + -- the schema then.
>     + local snap_dir = box.cfg.memtx_dir
>     + local snap_version = private.get_snapshot_version(snap_dir)
>     + if snap_version then
>     + private.set_recovery_triggers(snap_version)
>     + end
>     +
>          -- This call either succeeds or calls panic() / exit().
>          private.cfg_load()
>
>     + if snap_version then
>     + private.clear_recovery_triggers()
>     + end
>          -- This block does not raise an error: all necessary checks
>          -- already performed in private.cfg_check(). See <dynamic_cfg>
>          -- comment.
>     diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>     index 6abce50f4..925adab18 100644
>     --- a/src/box/lua/upgrade.lua
>     +++ b/src/box/lua/upgrade.lua
>     @@ -1,6 +1,8 @@
>      local log = require('log')
>      local bit = require('bit')
>      local json = require('json')
>     +local fio = require('fio')
>     +local xlog = require('xlog')
>
>      -- Guest user id - the default user
>      local GUEST = 0
>     @@ -86,6 +88,40 @@ local function set_system_triggers(val)
>          foreach_system_space(function(s) s:run_triggers(val) end)
>      end
>
>     +-- Get schema version, stored in _schema system space, by reading
>     the latest
>     +-- snapshot file from the snap_dir. Useful to define
>     schema_version before
>     +-- recovering the snapshot, because some schema versions are too
>     old and cannot
>     +-- be recovered normally.
>     +local function get_snapshot_version(snap_dir)
>     + local snap_pattern = snap_dir..'/'..string.rep('[0-9]', 20)..'.snap'
>
> Probably we could use fio.pathjoin here

Yep, sure. Changed.

>     + local snap_list = fio.glob(snap_pattern)
>     + table.sort(snap_list)
>     + local snap = snap_list[#snap_list]
>     + if not snap then
>     + return nil
>     + end
>     + local version = nil
>     + for _, row in xlog.pairs(snap) do
>     + local sid = row.BODY and row.BODY.space_id
>     + if sid == box.schema.SCHEMA_ID then
>     + local tuple = row.BODY.tuple
>     + if tuple and tuple[1] == 'version' then
>     + local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
>
> Could it be replaced with tuple:unpack(2, 4)?

Yep. Fixed as well.

>     + if major and minor and patch and type(major) == 'number' and
>     + type(minor) == 'number' and type(patch) == 'number' then
>
> Here you use type() == number checks. Could it be different? In case 
> of broken snap?

Yep, that's a check for a broken snap.
> So, we should log it I assume. The same for similar places below.

Added logging.

>     + version = mkversion(major, minor, patch)
>     + break
>     + end
>     + end
>     + elseif sid and sid > box.schema.SCHEMA_ID then
>     + -- Exit early if version wasn't found in _schema space.
>     + -- Snapshot rows are ordered by space id.
>     + break
>     + end
>     + end
>     + return version
>     +end
>     +
>      --------------------------------------------------------------------------------
>      -- Bootstrap
>      --------------------------------------------------------------------------------
>     @@ -131,6 +167,144 @@ local function create_sysview(source_id,
>     target_id)
>          end
>      end
>
>     +--------------------------------------------------------------------------------
>     +-- Tarantool 1.7.1
>     +--------------------------------------------------------------------------------
>     +local function user_trig_1_7_1(_, tuple)
>     + if tuple and tuple[3] == 'guest' and not tuple[5] then
>
> I think it’s better to use explicit check for tuple[5] value.
> If it’s a boolean value could it be box.NULL?

not box.NULL returns false, while not nil returns true.

On the other hand, box.NULL == nil returns true.

So I'd rather leave this check.


Here's the full diff:

============================================
diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
index 925adab18..01e22aa7c 100644
--- a/src/box/lua/upgrade.lua
+++ b/src/box/lua/upgrade.lua
@@ -88,12 +88,23 @@ local function set_system_triggers(val)
      foreach_system_space(function(s) s:run_triggers(val) end)
  end

+local function version_from_tuple(tuple)
+    local major, minor, patch = tuple:unpack(2, 4)
+    patch = patch or 0
+    if major and minor and type(major) == 'number' and
+       type(minor) == 'number' and type(patch) == 'number' then
+        return mkversion(major, minor, patch)
+    end
+    return nil
+end
+
  -- Get schema version, stored in _schema system space, by reading the 
latest
  -- snapshot file from the snap_dir. Useful to define schema_version before
  -- recovering the snapshot, because some schema versions are too old 
and cannot
  -- be recovered normally.
  local function get_snapshot_version(snap_dir)
-    local snap_pattern = snap_dir..'/'..string.rep('[0-9]', 20)..'.snap'
+    local snap_pattern = fio.pathjoin(snap_dir,
+                                      string.rep('[0-9]', 20)..'.snap')
      local snap_list = fio.glob(snap_pattern)
      table.sort(snap_list)
      local snap = snap_list[#snap_list]
@@ -106,12 +117,14 @@ local function get_snapshot_version(snap_dir)
          if sid == box.schema.SCHEMA_ID then
              local tuple = row.BODY.tuple
              if tuple and tuple[1] == 'version' then
-                local major, minor, patch = tuple[2], tuple[3], 
tuple[4] or 0
-                if major and minor and patch and type(major) == 
'number' and
-                   type(minor) == 'number' and type(patch) == 'number' then
-                    version = mkversion(major, minor, patch)
-                    break
+                local major, minor, patch = tuple:unpack(2, 4)
+                patch = patch or 0
+                version = version_from_tuple(tuple)
+                if not version then
+                    log.error("Corrupted version tuple in space 
'_schema' "..
+                              "in snapshot '%s': %s ", snap, tuple)
                  end
+                break
              end
          elseif sid and sid > box.schema.SCHEMA_ID then
              -- Exit early if version wasn't found in _schema space.
@@ -1284,10 +1297,8 @@ local recovery_triggers = {
  -- snapshot), the triggers helping recover the old schema should be 
removed.
  local function schema_trig_last(_, tuple)
      if tuple and tuple[1] == 'version' then
-        local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
-        if major and minor and patch and type(major) == 'number' and
-           type(minor) == 'number' and type(patch) == 'number' then
-            local version = mkversion(major, minor, patch)
+        local version = version_from_tuple(tuple)
+        if version then
              log.info("Recovery trigger: recovered schema version %s. "..
                       "Removing outdated recovery triggers.", version)
              box.internal.clear_recovery_triggers(version)


============================================
>
>     + local auth_method_list = {}
>     + auth_method_list["chap-sha1"] = box.schema.user.password("")
>     + tuple = tuple:update{{'=', 5, auth_method_list}}
>     + log.info("Set empty password to user 'guest'")
>     + end
>     + return tuple
>     +end
>     +
>     +--------------------------------------------------------------------------------
>     +-- Tarantool 1.7.2
>     +--------------------------------------------------------------------------------
>     +local function index_trig_1_7_2(_, tuple)
>     + local field_types_v16 = {
>     + num = 'unsigned',
>     + int = 'integer',
>     + str = 'string',
>     + }
>     + if not tuple then
>     + return tuple
>     + end
>     + local parts = tuple[6]
>     + local changed = false
>     + for _, part in pairs(parts) do
>     + local field_type = part[2]:lower()
>     + if field_types_v16[field_type] ~= nil then
>     + part[2] = field_types_v16[field_type]
>     + changed = true
>     + end
>     + end
>     + if changed then
>     + log.info("Update index '%s' on space '%s': set parts to %s",
>     tuple[3],
>     + box.space[tuple[1]].name, json.encode(parts))
>     + tuple = tuple:update{{'=', 6, parts}}
>     + end
>     + return tuple
>     +end
>     +
>     +--------------------------------------------------------------------------------
>     +-- Tarantool 1.7.5
>     +--------------------------------------------------------------------------------
>     +local function create_truncate_space()
>     + local _truncate = box.space[box.schema.TRUNCATE_ID]
>     +
>     + log.info("create space _truncate")
>     + box.space._space:insert{
>     + _truncate.id, ADMIN, '_truncate', 'memtx', 0, setmap({}),
>     + {{name = 'id', type = 'unsigned'}, {name = 'count', type =
>     'unsigned'}}
>     + }
>     +
>     + log.info("create index primary on _truncate")
>     + box.space._index:insert{
>     + _truncate.id, 0, 'primary', 'tree', {unique = true}, {{0,
>     'unsigned'}}
>     + }
>     +
>     + local _priv = box.space[box.schema.PRIV_ID]
>     + _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
>     +end
>     +
>     +local function upgrade_to_1_7_5()
>     + create_truncate_space()
>     +end
>     +
>     +local function user_trig_1_7_5(_, tuple)
>     + if tuple and not tuple[5] then
>     + tuple = tuple:update{{'=', 5, setmap({})}}
>     + log.info("Set empty password to %s '%s'", tuple[4], tuple[3])
>     + end
>     + return tuple
>     +end
>     +
>     +local space_formats_1_7_5 = {
>     + _schema = {
>     + {name = 'key', type = 'string'},
>     + },
>     + _space = {
>     + {name = 'id', type = 'unsigned'},
>     + {name = 'owner', type = 'unsigned'},
>     + {name = 'name', type = 'string'},
>     + {name = 'engine', type = 'string'},
>     + {name = 'field_count', type = 'unsigned'},
>     + {name = 'flags', type = 'map'},
>     + {name = 'format', type = 'array'},
>     + },
>     + _index = {
>     + {name = 'id', type = 'unsigned'},
>     + {name = 'iid', type = 'unsigned'},
>     + {name = 'name', type = 'string'},
>     + {name = 'type', type = 'string'},
>     + {name = 'opts', type = 'map'},
>     + {name = 'parts', type = 'array'},
>     + },
>     + _func = {
>     + {name = 'id', type = 'unsigned'},
>     + {name = 'owner', type = 'unsigned'},
>     + {name = 'name', type = 'string'},
>     + {name = 'setuid', type = 'unsigned'},
>     + },
>     + _user = {
>     + {name = 'id', type = 'unsigned'},
>     + {name = 'owner', type = 'unsigned'},
>     + {name = 'name', type = 'string'},
>     + {name = 'type', type = 'string'},
>     + {name = 'auth', type = 'map'},
>     + },
>     + _priv = {
>     + {name = 'grantor', type = 'unsigned'},
>     + {name = 'grantee', type = 'unsigned'},
>     + {name = 'object_type', type = 'string'},
>     + {name = 'object_id', type = 'unsigned'},
>     + {name = 'privilege', type = 'unsigned'},
>     + },
>     + _cluster = {
>     + {name = 'id', type = 'unsigned'},
>     + {name = 'uuid', type = 'string'},
>     + },
>     +}
>     +
>     +space_formats_1_7_5._vspace = space_formats_1_7_5._space
>     +space_formats_1_7_5._vindex = space_formats_1_7_5._index
>     +space_formats_1_7_5._vfunc = space_formats_1_7_5._func
>     +space_formats_1_7_5._vuser = space_formats_1_7_5._user
>     +space_formats_1_7_5._vpriv = space_formats_1_7_5._priv
>     +
>     +local function space_trig_1_7_5(_, tuple)
>     + if tuple and space_formats_1_7_5[tuple[3]] and
>     + not table.equals(space_formats_1_7_5[tuple[3]], tuple[7]) then
>     + tuple = tuple:update{{'=', 7, space_formats_1_7_5[tuple[3]]}}
>     + log.info("Update space '%s' format: new format %s", tuple[3],
>     + json.encode(tuple[7]))
>     + end
>     + return tuple
>     +end
>     +
>      local function initial_1_7_5()
>          -- stick to the following convention:
>          -- prefer user id (owner id) in field #1
>     @@ -452,6 +626,15 @@ local function upgrade_to_1_7_7()
>          _priv:replace({ADMIN, SUPER, 'universe', 0, 4294967295})
>      end
>
>     +local function priv_trig_1_7_7(_, tuple)
>     + if tuple and tuple[2] == ADMIN and tuple[3] == 'universe' and
>     + tuple[5] ~= box.priv.ALL then
>     + tuple = tuple:update{{'=', 5, box.priv.ALL}}
>     + log.info("Grant all privileges to user 'admin'")
>     + end
>     + return tuple
>     +end
>     +
>      --------------------------------------------------------------------------------
>      --- Tarantool 1.10.0
>      --------------------------------------------------------------------------------
>     @@ -1021,6 +1204,7 @@ end
>      --------------------------------------------------------------------------------
>
>      local handlers = {
>     + {version = mkversion(1, 7, 5), func = upgrade_to_1_7_5, auto=true},
>          {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto
>     = true},
>          {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto
>     = true},
>          {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0,
>     auto = true},
>     @@ -1061,13 +1245,98 @@ local function schema_needs_upgrade()
>          return false
>      end
>
>     +local trig_oldest_version = nil
>     +
>     +-- Some schema changes before version 1.7.7 make it impossible to
>     recover from
>     +-- older snapshot. The table below consists of before_replace
>     triggers on system
>     +-- spaces, which make old snapshot schema compatible with current
>     Tarantool
>     +-- (version 2.x). The triggers replace old format tuples with new
>     ones
>     +-- in-memory, thus making it possible to recover from a rather
>     old snapshot
>     +-- (up to schema version 1.6.8). Once the snapshot is recovered,
>     a normal
>     +-- upgrade procedure may set schema version to the latest one.
>     +--
>     +-- The triggers mostly repeat the corresponding upgrade_to_1_7_x
>     functions,
>     +-- which were used when pre-1.7.x snapshot schema was still
>     recoverable.
>     +--
>     +-- When the triggers are used (i.e. when snapshot schema version
>     is below 1.7.5,
>     +-- the upgrade procedure works as follows:
>     +-- * first the snapshot is recovered and 1.7.5-compatible schema
>     is applied to
>     +-- it in-memory with the help of triggers.
>     +-- * then usual upgrade_to_X_X_X() handlers may be fired to turn
>     schema into the
>     +-- latest one.
>     +local recovery_triggers = {
>     + {version = mkversion(1, 7, 1), tbl = {
>     + _user = user_trig_1_7_1,
>     + }},
>     + {version = mkversion(1, 7, 2), tbl = {
>     + _index = index_trig_1_7_2,
>     + }},
>     + {version = mkversion(1, 7, 5), tbl = {
>     + _space = space_trig_1_7_5,
>     + _user = user_trig_1_7_5,
>     + }},
>     + {version = mkversion(1, 7, 7), tbl = {
>     + _priv = priv_trig_1_7_7,
>     + }},
>     +}
>     +
>     +-- Once newer schema version is recovered (say, from an xlog
>     following the old
>     +-- snapshot), the triggers helping recover the old schema should
>     be removed.
>     +local function schema_trig_last(_, tuple)
>     + if tuple and tuple[1] == 'version' then
>     + local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
>     + if major and minor and patch and type(major) == 'number' and
>     + type(minor) == 'number' and type(patch) == 'number' then
>     + local version = mkversion(major, minor, patch)
>     + log.info("Recovery trigger: recovered schema version %s. "..
>     + "Removing outdated recovery triggers.", version)
>     + box.internal.clear_recovery_triggers(version)
>     + trig_oldest_version = version
>     + end
>     + end
>     + return tuple
>     +end
>     +
>     +recovery_triggers[#recovery_triggers].tbl['_schema'] =
>     schema_trig_last
>     +
>     +local function on_init_set_recovery_triggers()
>     + log.info("Recovering snapshot with schema version %s",
>     trig_oldest_version)
>     + for _, trig_tbl in ipairs(recovery_triggers) do
>     + if trig_tbl.version > trig_oldest_version then
>     + for space, trig in pairs(trig_tbl.tbl) do
>     + box.space[space]:before_replace(trig)
>     + log.info("Set recovery trigger on space '%s' to comply with "..
>     + "version %s format", space, trig_tbl.version)
>     + end
>     + end
>     + end
>     +end
>     +
>     +local function set_recovery_triggers(version)
>     + trig_oldest_version = version
>     + box.ctl.on_schema_init(on_init_set_recovery_triggers)
>     +end
>     +
>     +local function clear_recovery_triggers(version)
>     + for _, trig_tbl in ipairs(recovery_triggers) do
>     + if trig_tbl.version > trig_oldest_version and
>     + (not version or trig_tbl.version <= version) then
>     + for space, trig in pairs(trig_tbl.tbl) do
>     + box.space[space]:before_replace(nil, trig)
>     + log.info("Remove recovery trigger on space '%s' for version %s",
>     + space, trig_tbl.version)
>     + end
>     + end
>     + end
>     +end
>     +
>      local function upgrade(options)
>          options = options or {}
>          setmetatable(options, {__index = {auto = false}})
>
>          local version = get_version()
>     - if version < mkversion(1, 7, 5) then
>     - log.warn('can upgrade from 1.7.5 only')
>     + if version < mkversion(1, 6, 8) then
>     + log.warn('can upgrade from 1.6.8 only')
>              return
>          end
>
>     @@ -1110,3 +1379,6 @@ end
>      box.schema.upgrade = upgrade;
>      box.internal.bootstrap = bootstrap;
>      box.internal.schema_needs_upgrade = schema_needs_upgrade;
>     +box.internal.get_snapshot_version = get_snapshot_version;
>     +box.internal.set_recovery_triggers = set_recovery_triggers;
>     +box.internal.clear_recovery_triggers = clear_recovery_triggers;
>     diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.result
>     b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
>     new file mode 100644
>     index 000000000..aba5a56ab
>     --- /dev/null
>     +++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
>     @@ -0,0 +1,400 @@
>     +-- test-run result file version 2
>     +test_run = require('test_run').new()
>     + | ---
>     + | ...
>     +
>     +-- Upgrade from 1.6.8.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('start server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:switch('upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     + | ---
>     + | - true
>     + | ...
>     +box.space.distro:select{}
>     + | ---
>     + | - - ['debian', 'sarge', 31, 1118059200]
>     + | - ['debian', 'etch', 40, 1176033600]
>     + | - ['ubuntu', 'trusty', 1404, 1397736000]
>     + | - ['ubuntu', 'vivid', 1504, 1429790400]
>     + | - ['ubuntu', 'wily', 1510, 1445515200]
>     + | - ['debian', 'wheezy', 70, 1367668800]
>     + | - ['debian', 'squeeze', 60, 1296907200]
>     + | - ['debian', 'lenny', 50, 1234612800]
>     + | - ['debian', 'jessie', 80, 1430049600]
>     + | - ['ubuntu', 'precise', 1510, 1335441600]
>     + | - ['debian', 'woody', 30, 1027080000]
>     + | ...
>     +box.space._index:select{box.space.distro.id}
>     + | ---
>     + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
>     'string'], [1, 'string'], [
>     + | 2, 'unsigned']]]
>     + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>     + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>     + | ...
>     +box.space._space:format()
>     + | ---
>     + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
>     'type': 'unsigned'}, {'name': 'name',
>     + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
>     {'name': 'field_count',
>     + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
>     {'name': 'format', 'type': 'array'}]
>     + | ...
>     +box.schema.user.info('admin')
>     + | ---
>     + | - - -
>     read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('guest')
>     + | ---
>     + | - - - execute
>     + | - role
>     + | - public
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('someuser')
>     + | ---
>     + | - - - execute
>     + | - function
>     + | - someotherfunc
>     + | - - execute
>     + | - role
>     + | - public
>     + | - - execute
>     + | - role
>     + | - somerole
>     + | - - read,write,drop,alter
>     + | - space
>     + | - temporary
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.role.info('somerole')
>     + | ---
>     + | - - - read,write,drop,alter
>     + | - space
>     + | - distro
>     + | ...
>     +
>     +test_run:switch('default')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('stop server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('delete server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +-- Upgrade from 1.7.1.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('start server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:switch('upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     + | ---
>     + | - true
>     + | ...
>     +box.space.distro:select{}
>     + | ---
>     + | - - ['debian', 'etch', 40, 1176033600]
>     + | - ['debian', 'sarge', 31, 1118059200]
>     + | - ['ubuntu', 'wily', 1510, 1445515200]
>     + | - ['ubuntu', 'trusty', 1404, 1397736000]
>     + | - ['ubuntu', 'vivid', 1504, 1429790400]
>     + | - ['debian', 'wheezy', 70, 1367668800]
>     + | - ['debian', 'squeeze', 60, 1296907200]
>     + | - ['debian', 'lenny', 50, 1234612800]
>     + | - ['debian', 'jessie', 80, 1430049600]
>     + | - ['ubuntu', 'precise', 1510, 1335441600]
>     + | - ['debian', 'woody', 30, 1027080000]
>     + | ...
>     +box.space._index:select{box.space.distro.id}
>     + | ---
>     + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
>     'string'], [1, 'string'], [
>     + | 2, 'unsigned']]]
>     + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>     + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>     + | ...
>     +box.space._space:format()
>     + | ---
>     + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
>     'type': 'unsigned'}, {'name': 'name',
>     + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
>     {'name': 'field_count',
>     + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
>     {'name': 'format', 'type': 'array'}]
>     + | ...
>     +box.schema.user.info('admin')
>     + | ---
>     + | - - -
>     read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('guest')
>     + | ---
>     + | - - - execute
>     + | - role
>     + | - public
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('someuser')
>     + | ---
>     + | - - - execute
>     + | - function
>     + | - someotherfunc
>     + | - - execute
>     + | - role
>     + | - public
>     + | - - execute
>     + | - role
>     + | - somerole
>     + | - - read,write,drop,alter
>     + | - space
>     + | - temporary
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.role.info('somerole')
>     + | ---
>     + | - - - read,write,drop,alter
>     + | - space
>     + | - distro
>     + | ...
>     +
>     +test_run:switch('default')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('stop server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('delete server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +-- Upgrade from 1.7.2.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('start server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:switch('upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     + | ---
>     + | - true
>     + | ...
>     +box.space.distro:select{}
>     + | ---
>     + | - - ['debian', 'sarge', 31, 1118059200]
>     + | - ['debian', 'etch', 40, 1176033600]
>     + | - ['ubuntu', 'trusty', 1404, 1397736000]
>     + | - ['ubuntu', 'vivid', 1504, 1429790400]
>     + | - ['debian', 'lenny', 50, 1234612800]
>     + | - ['debian', 'wheezy', 70, 1367668800]
>     + | - ['debian', 'squeeze', 60, 1296907200]
>     + | - ['ubuntu', 'wily', 1510, 1445515200]
>     + | - ['debian', 'jessie', 80, 1430049600]
>     + | - ['ubuntu', 'precise', 1510, 1335441600]
>     + | - ['debian', 'woody', 30, 1027080000]
>     + | ...
>     +box.space._index:select{box.space.distro.id}
>     + | ---
>     + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
>     'string'], [1, 'string'], [
>     + | 2, 'unsigned']]]
>     + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>     + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>     + | ...
>     +box.space._space:format()
>     + | ---
>     + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
>     'type': 'unsigned'}, {'name': 'name',
>     + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
>     {'name': 'field_count',
>     + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
>     {'name': 'format', 'type': 'array'}]
>     + | ...
>     +box.schema.user.info('admin')
>     + | ---
>     + | - - -
>     read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('guest')
>     + | ---
>     + | - - - execute
>     + | - role
>     + | - public
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('someuser')
>     + | ---
>     + | - - - execute
>     + | - function
>     + | - someotherfunc
>     + | - - execute
>     + | - role
>     + | - public
>     + | - - execute
>     + | - role
>     + | - somerole
>     + | - - read,write,drop,alter
>     + | - space
>     + | - temporary
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.role.info('somerole')
>     + | ---
>     + | - - - read,write,drop,alter
>     + | - space
>     + | - distro
>     + | ...
>     +
>     +test_run:switch('default')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('stop server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('delete server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +-- Upgrade from 1.7.5.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('start server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:switch('upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     + | ---
>     + | - true
>     + | ...
>     +box.space.distro:select{}
>     + | ---
>     + | - - ['debian', 'etch', 40, 1176033600]
>     + | - ['debian', 'sarge', 31, 1118059200]
>     + | - ['debian', 'lenny', 50, 1234612800]
>     + | - ['ubuntu', 'trusty', 1404, 1397736000]
>     + | - ['ubuntu', 'vivid', 1504, 1429790400]
>     + | - ['debian', 'wheezy', 70, 1367668800]
>     + | - ['debian', 'squeeze', 60, 1296907200]
>     + | - ['ubuntu', 'wily', 1510, 1445515200]
>     + | - ['debian', 'jessie', 80, 1430049600]
>     + | - ['ubuntu', 'precise', 1510, 1335441600]
>     + | - ['debian', 'woody', 30, 1027080000]
>     + | ...
>     +box.space._index:select{box.space.distro.id}
>     + | ---
>     + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
>     'string'], [1, 'string'], [
>     + | 2, 'unsigned']]]
>     + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
>     + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
>     + | ...
>     +box.space._space:format()
>     + | ---
>     + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
>     'type': 'unsigned'}, {'name': 'name',
>     + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
>     {'name': 'field_count',
>     + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
>     {'name': 'format', 'type': 'array'}]
>     + | ...
>     +box.schema.user.info('admin')
>     + | ---
>     + | - - -
>     read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('guest')
>     + | ---
>     + | - - - execute
>     + | - role
>     + | - public
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.user.info('someuser')
>     + | ---
>     + | - - - execute
>     + | - function
>     + | - someotherfunc
>     + | - - execute
>     + | - role
>     + | - public
>     + | - - execute
>     + | - role
>     + | - somerole
>     + | - - read,write,drop,alter
>     + | - space
>     + | - temporary
>     + | - - session,usage
>     + | - universe
>     + | -
>     + | ...
>     +box.schema.role.info('somerole')
>     + | ---
>     + | - - - read,write,drop,alter
>     + | - space
>     + | - distro
>     + | ...
>     +
>     +test_run:switch('default')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('stop server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     +test_run:cmd('delete server upgrade')
>     + | ---
>     + | - true
>     + | ...
>     diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>     b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>     new file mode 100644
>     index 000000000..9096bcb7a
>     --- /dev/null
>     +++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>     @@ -0,0 +1,77 @@
>     +test_run = require('test_run').new()
>     +
>     +-- Upgrade from 1.6.8.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
>     +test_run:cmd('start server upgrade')
>     +test_run:switch('upgrade')
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     +box.space.distro:select{}
>     +box.space._index:select{box.space.distro.id}
>     +box.space._space:format()
>     +box.schema.user.info('admin')
>     +box.schema.user.info('guest')
>     +box.schema.user.info('someuser')
>     +box.schema.role.info('somerole')
>     +
>     +test_run:switch('default')
>     +test_run:cmd('stop server upgrade')
>     +test_run:cmd('delete server upgrade')
>     +
>     +-- Upgrade from 1.7.1.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
>     +test_run:cmd('start server upgrade')
>     +test_run:switch('upgrade')
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     +box.space.distro:select{}
>     +box.space._index:select{box.space.distro.id}
>     +box.space._space:format()
>     +box.schema.user.info('admin')
>     +box.schema.user.info('guest')
>     +box.schema.user.info('someuser')
>     +box.schema.role.info('somerole')
>     +
>     +test_run:switch('default')
>     +test_run:cmd('stop server upgrade')
>     +test_run:cmd('delete server upgrade')
>     +
>     +-- Upgrade from 1.7.2.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
>     +test_run:cmd('start server upgrade')
>     +test_run:switch('upgrade')
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     +box.space.distro:select{}
>     +box.space._index:select{box.space.distro.id}
>     +box.space._space:format()
>     +box.schema.user.info('admin')
>     +box.schema.user.info('guest')
>     +box.schema.user.info('someuser')
>     +box.schema.role.info('somerole')
>     +
>     +test_run:switch('default')
>     +test_run:cmd('stop server upgrade')
>     +test_run:cmd('delete server upgrade')
>     +
>     +-- Upgrade from 1.7.5.
>     +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
>     + workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
>     +test_run:cmd('start server upgrade')
>     +test_run:switch('upgrade')
>     +
>     +assert(not box.internal.schema_needs_upgrade())
>     +box.space.distro:select{}
>     +box.space._index:select{box.space.distro.id}
>     +box.space._space:format()
>     +box.schema.user.info('admin')
>     +box.schema.user.info('guest')
>     +box.schema.user.info('someuser')
>     +box.schema.role.info('somerole')
>     +
>     +test_run:switch('default')
>     +test_run:cmd('stop server upgrade')
>     +test_run:cmd('delete server upgrade')
>     diff --git
>     a/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>     b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>     new file mode 120000
>     index 000000000..2f2a84962
>     --- /dev/null
>     +++ b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>     @@ -0,0 +1 @@
>     +../../fill.lua
>     \ No newline at end of file
>     diff --git
>     a/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>     b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>     new file mode 120000
>     index 000000000..2f2a84962
>     --- /dev/null
>     +++ b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>     @@ -0,0 +1 @@
>     +../../fill.lua
>     \ No newline at end of file
>     diff --git
>     a/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>     b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>     new file mode 120000
>     index 000000000..2f2a84962
>     --- /dev/null
>     +++ b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>     @@ -0,0 +1 @@
>     +../../fill.lua
>     \ No newline at end of file
>     diff --git
>     a/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>     b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>     new file mode 120000
>     index 000000000..2f2a84962
>     --- /dev/null
>     +++ b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>     @@ -0,0 +1 @@
>     +../../fill.lua
>     \ No newline at end of file
>     diff --git a/test/xlog/upgrade/fill.lua b/test/xlog/upgrade/fill.lua
>     index 0ef1a8bb9..310c1ca72 100644
>     --- a/test/xlog/upgrade/fill.lua
>     +++ b/test/xlog/upgrade/fill.lua
>     @@ -56,4 +56,8 @@ end
>      box.schema.func.create('someotherfunc')
>      box.schema.user.grant('someuser', 'execute', 'function',
>     'someotherfunc')
>      box.schema.user.grant('someuser', 'read,write', 'space', 'temporary')
>     +
>     +box.schema.upgrade()
>     +box.snapshot()
>     +
>      os.exit(0)
>     --
>     2.30.1 (Apple Git-130)
>
> --
> Oleg Babin

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method

[Serge Petrenko via Tarantool-patches]

16.08.2021 18:41, Бабин Олег пишет:
> Ok. Thanks for your changes. Don’t forget to changelog entry and 
> docbot request and LGTM then.
>

Thanks for your answers!
Yep, sure. Done.

>     Понедельник, 16 августа 2021, 18:36 +03:00 от Serge Petrenko
>     <sergepetrenko@tarantool.org>:
>
>
>     16.08.2021 16:03, Бабин Олег пишет:
>     > Thanks for your fixes.
>     >
>     > Понедельник, 16 августа 2021, 10:53 +03:00 от Serge Petrenko
>     > <sergepetrenko@tarantool.org
>     </compose?To=sergepetrenko@tarantool.org>>:
>     >
>     > Thanks for a thorough review!
>     >
>     > I've force pushed the following fixes:
>     >
>     > =============================================
>     >
>     > diff --git a/src/lua/table.lua b/src/lua/table.lua
>     > index 3d5e69e97..edd60d1be 100644
>     > --- a/src/lua/table.lua
>     > +++ b/src/lua/table.lua
>     > @@ -65,8 +65,11 @@ local function table_equals(a, b)
>     >       if type(a) ~= 'table' or type(b) ~= 'table' then
>     >           return type(a) == type(b) and a == b
>     >       end
>     > -    local mt = getmetatable(a)
>     > -    if mt and mt.__eq then
>     > +    local mta = getmetatable(a)
>     > +    local mtb = getmetatable(b)
>     > +    -- Let Lua decide what should happen when at least one of the
>     > tables has a
>     > +    -- metatable.
>     > +    if mta and mta.__eq or mtb and mtb.__eq then
>     >
>     > I’m not sure that it’s correct. I’ve added a reference to SO
>     >
>     (https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables
>     <https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables>
>     >
>     <https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables
>     <https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables>>)
>     > and it seems condition should be following
>     > mta and mta.__eq and mta == mtb. Otherwise tables have different
>     __eq
>     > metamathods and you should compare their content.
>     > I’m not sure that my proposal is completely correct and suggest
>     to ask
>     > Igor about it.
>
>     Actually the question you refer to describes Lua 5.3, while LuaJIT
>     supports Lua 5.1
>     The rules for Lua 5.1 are as follows (taken from
>     https://www.lua.org/manual/5.1/manual.html#2.4
>     <https://www.lua.org/manual/5.1/manual.html#2.4>):
>
>     Use the __eq metamethod only when both objects have the same one.
>
>     So, in my case, if at least one metamethod is defined, try to use
>     comparison operator.
>     If both tables have the same __eq metamethod, Lua will call it.
>     If the tables don't share the same metamethod, they will be
>     compared like
>     Lua does usually: by reference. The result will be false then.
>
>
>     I've added one more test though, just in case:
>
>     diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua
>     index c7a075aba..ae6fdfa2d 100755
>     --- a/test/app-tap/table.test.lua
>     +++ b/test/app-tap/table.test.lua
>     @@ -8,7 +8,7 @@ yaml.cfg{
>           encode_invalid_as_nil  = true,
>       }
>       local test = require('tap').test('table')
>     -test:plan(43)
>     +test:plan(44)
>
>       do -- check basic table.copy (deepcopy)
>           local example_table = {
>     @@ -258,12 +258,15 @@ do -- check table.equals
>               __eq = function(a, b) -- luacheck: no unused args
>                   return true
>               end}
>     -    local tbl_d = setmetatable({a=15}, mt)
>     -    local tbl_e = setmetatable({b=2, c=3}, mt)
>     +    local tbl_d = setmetatable({a = 15}, mt)
>     +    local tbl_e = setmetatable({b = 2, c = 3}, mt)
>           test:ok(table.equals(tbl_d, tbl_e), "table.equals respects
>     __eq")
>     -    test:is(table.equals(tbl_d, {a=15}), false,
>     +    test:is(table.equals(tbl_d, {a = 15}), false,
>                   "table.equals when metatables don't match")
>     -    test:is(table.equals({a=15}, tbl_d), false,
>     +    test:is(table.equals({a = 15}, tbl_d), false,
>     +            "table.equals when metatables don't match")
>     +    local tbl_f = setmetatable({a = 15}, {__eq = function()
>     return true
>     end})
>     +    test:is(table.equals(tbl_d, tbl_f), false,
>                   "table.equals when metatables don't match")
>       end
>
>
>
>
>     >           return a == b
>     >       end
>     >       for k, v in pairs(a) do
>     > diff --git a/test/app-tap/table.test.lua
>     b/test/app-tap/table.test.lua
>     > index ec81593f3..029b923fb 100755
>     > --- a/test/app-tap/table.test.lua
>     > +++ b/test/app-tap/table.test.lua
>     > @@ -8,7 +8,7 @@ yaml.cfg{
>     >       encode_invalid_as_nil  = true,
>     >   }
>     >   local test = require('tap').test('table')
>     > -test:plan(40)
>     > +test:plan(43)
>     >
>     >   do -- check basic table.copy (deepcopy)
>     >       local example_table = {
>     > @@ -254,6 +254,14 @@ do -- check table.equals
>     >       test:ok(table.equals(tbl_a, tbl_c),
>     >               "table.equals for shallow copied tables after
>     > modification")
>     >       test:is(table.equals(tbl_a, tbl_b), false, "table.equals
>     does a
>     > deep check")
>     > +    local mt = {__eq = function(a, b) return true end}
>     > +    local tbl_d = setmetatable({a=15}, mt)
>     > +    local tbl_e = setmetatable({b=2, c=3}, mt)
>     > +    test:ok(table.equals(tbl_d, tbl_e), "table.equals respects
>     __eq")
>     > +    test:is(table.equals(tbl_d, {a=15}), false,
>     > +            "table.equals when metatables don't match")
>     > +    test:is(table.equals({a=15}, tbl_d), false,
>     > +            "table.equals when metatables don't match")
>     >   end
>     >
>     >   os.exit(test:check() == true and 0 or 1)
>     >
>     > =============================================
>     >
>     > >
>     > > On 13.08.2021 13:22, Serge Petrenko wrote:
>     > >>
>     > >>
>     > >> 13.08.2021 08:30, Oleg Babin пишет:
>     > >>> Thanks for your patch.
>     > >>>
>     > >>>
>     > >>> It seems it works in quite strange way:
>     > >>>
>     > >>> ```
>     > >>>
>     > >>> tarantool> table.equals({a = box.NULL}, {})
>     > >>> ---
>     > >>> - true
>     > >>> ...
>     > >>>
>     > >>> tarantool> table.equals({}, {a = box.NULL})
>     > >>> ---
>     > >>> - false
>     > >>> ...
>     > >>>
>     > >>> ```
>     > >>>
>     > >>>
>     > >>> I can change arguments order to get different result. Expected
>     > false
>     > >>> in both cases.
>     > >>>
>     > >>> For tap it was considered as buggy behaviour
>     > >>> https://github.com/tarantool/tarantool/issues/4125
>     <https://github.com/tarantool/tarantool/issues/4125>
>     > <https://github.com/tarantool/tarantool/issues/4125
>     <https://github.com/tarantool/tarantool/issues/4125>>
>     > >>>
>     > >>>
>     > >>
>     > >> ====================================
>     > >>
>     > >> Good catch! Thanks!
>     > >>
>     > >> Check out the diff:
>     > >>
>     > >> diff --git a/src/lua/table.lua b/src/lua/table.lua
>     > >> index 5f35a30f6..3d5e69e97 100644
>     > >> --- a/src/lua/table.lua
>     > >> +++ b/src/lua/table.lua
>     > >> @@ -63,7 +63,7 @@ end
>     > >>  -- @return true when the two tables are equal (false otherwise).
>     > >>  local function table_equals(a, b)
>     > >>      if type(a) ~= 'table' or type(b) ~= 'table' then
>     > >> -        return a == b
>     > >> +        return type(a) == type(b) and a == b
>     > >>      end
>     > >>      local mt = getmetatable(a)
>     > >>      if mt and mt.__eq then
>     > >> diff --git a/test/app-tap/table.test.lua
>     > b/test/app-tap/table.test.lua
>     > >> index a3c9aa123..ec81593f3 100755
>     > >> --- a/test/app-tap/table.test.lua
>     > >> +++ b/test/app-tap/table.test.lua
>     > >> @@ -8,7 +8,7 @@ yaml.cfg{
>     > >>      encode_invalid_as_nil  = true,
>     > >>  }
>     > >>  local test = require('tap').test('table')
>     > >> -test:plan(38)
>     > >> +test:plan(40)
>     > >>
>     > >>  do -- check basic table.copy (deepcopy)
>     > >>      local example_table = {
>     > >> @@ -227,6 +227,10 @@ do -- check table.equals
>     > >>      test:ok(table.equals({}, {}), "table.equals for empty
>     tables")
>     > >>      test:is(table.equals({}, {1}), false, "table.equals with one
>     > >> empty table")
>     > >>      test:is(table.equals({1}, {}), false, "table.equals with one
>     > >> empty table")
>     > >> +    test:is(table.equals({key = box.NULL}, {key = nil}), false,
>     > >> +            "table.equals for box.NULL and nil")
>     > >> +    test:is(table.equals({key = nil}, {key = box.NULL}), false,
>     > >> +            "table.equals for box.NULL and nil")
>     > >>      local tbl_a = {
>     > >>          first = {
>     > >>              1,
>     > >>
>     > >> ====================================
>     > >>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:
>     > >>>> Introduce table.equals for comparing tables.
>     > >>>> The method respects __eq metamethod, if provided.
>     > >>>>
>     > >>>> Needed-for #5894
>     > >>>> ---
>     > >>>>
>     > >>
>     >
>     > --
>     > Serge Petrenko
>     >
>     > --
>     > Oleg Babin
>
>     --
>     Serge Petrenko
>
> --
> Oleg Babin

-- 
Serge Petrenko

Re: [Tarantool-patches] [PATCH v2 2/2] box: allow upgrading from version 1.6

[Бабин Олег via Tarantool-patches]

[-- Attachment #1: Type: text/plain, Size: 36859 bytes --]


Thanks for fixes. LGTM.  понедельник, 16 августа 2021г., 19:32 +03:00 от Serge Petrenko  sergepetrenko@tarantool.org :

>
>
>16.08.2021 16:18, Бабин Олег пишет:
> Thanks for your patch. LGTM but consider several nits below.
>
>
>Thanks for the review! Please, find my answers and incremental diff below.
>
> Пятница, 13 августа 2021, 2:35 +03:00 от Serge Petrenko via
> Tarantool-patches < tarantool-patches@dev.tarantool.org> :
> Direct upgrade support from pre-1.7.5 versions was removed in commit
> 7d3b80e78206c479cab75b4893629cfa1932252e
> (Forbid upgrade from Tarantool < 1.7.5 and refactor upgrade.lua)
> The reason for that was the mandatory space format checks introduced
> back then. With these space format checks, old schema couldn't be
> recovered on new Tarantool versions, because newer versions had
> different system space formats. So old schema couldn't be upgraded
> because it couldn't even be recovered.
>
> Actually this was rather inconvenient. One had to perform an extra
> upgrade step when upgrading from, say, 1.6 to 2.x: instead of
> performing a direct upgrade one had to do 1.6 -> 1.10 -> 2.x upgrade
> which takes twice the time.
>
> Make it possible to boot from snapshots coming from Tarantool version
> 1.6.8 and above.
>
> In order to do so, introduce before_replace triggers on system spaces,
> which work during snapshot/xlog recovery. The triggers will set tuple
> formats to the ones supported by current Tarantool (2.x). This way the
> recovered data will have the correct format for a usual schema
> upgrade.
>
> Also add upgrade_to_1_7_5() handler, which finishes transformation of
> old schema to 1.7.5. The handler is fired together with other
> box.schema.upgrade() handlers, so there's no user-visible behaviour
> change.
>
> Side note: it would be great to use the same technique to allow
> booting
> from pre-1.6.8 snapshots. Unfortunately, this is not possible.
>
> Current triggers don't break the order of schema upgrades, so 1.7.1
> upgrades come before 1.7.2 and 1.7.5. This is because all the upgrades
> in these versions are replacing existing tuples and not inserting new
> ones, so the upgrades may be handled by the before_replace triggers.
>
> Upgrade to 1.6.8 requires inserting new tuples: creating sysviews,
> like
> _vspace, _vuser and so on. This can't be done from the before_replace
> triggers, so we would have to run triggers for 1.7.x first which would
> allow Tarantool to recover the snapshot, and then run an upgrade
> handler for
> 1.6.8. This looks really messy.
>
> Closes #5894
> ---
>  src/box/lua/load_cfg.lua | 14 +
>  src/box/lua/upgrade.lua | 276 +++++++++++-
>  test/xlog/gh-5894-pre-1.7.7-upgrade.result | 400 ++++++++++++++++++
>  test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua | 77 ++++
>  .../1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>  .../1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>  .../1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>  .../1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua | 1 +
>  test/xlog/upgrade/fill.lua | 4 +
>  9 files changed, 773 insertions(+), 2 deletions(-)
>  create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.result
>  create mode 100644 test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
>  create mode 120000
> test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
>  create mode 120000
> test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
>  create mode 120000
> test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
>  create mode 120000
> test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
>
> diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
> index 4df70c210..7a8cab3fd 100644
> --- a/src/box/lua/load_cfg.lua
> +++ b/src/box/lua/load_cfg.lua
> @@ -719,9 +719,23 @@ local function load_cfg(cfg)
>              __call = locked(reload_cfg),
>          })
>
> + -- Check schema version of the snapshot we're about to recover,
> if any.
> + -- Some schema versions (below 1.7.5) are incompatible with
> Tarantool 2.x
> + -- When recovering from such an old snapshot, special recovery
> triggers on
> + -- system spaces are needed in order to be able to recover and
> upgrade
> + -- the schema then.
> + local snap_dir = box.cfg.memtx_dir
> + local snap_version = private.get_snapshot_version(snap_dir)
> + if snap_version then
> + private.set_recovery_triggers(snap_version)
> + end
> +
>      -- This call either succeeds or calls panic() / exit().
>      private.cfg_load()
>
> + if snap_version then
> + private.clear_recovery_triggers()
> + end
>      -- This block does not raise an error: all necessary checks
>      -- already performed in private.cfg_check(). See 
>      -- comment.
> diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
> index 6abce50f4..925adab18 100644
> --- a/src/box/lua/upgrade.lua
> +++ b/src/box/lua/upgrade.lua
> @@ -1,6 +1,8 @@
>  local log = require('log')
>  local bit = require('bit')
>  local json = require('json')
> +local fio = require('fio')
> +local xlog = require('xlog')
>
>  -- Guest user id - the default user
>  local GUEST = 0
> @@ -86,6 +88,40 @@ local function set_system_triggers(val)
>      foreach_system_space(function(s) s:run_triggers(val) end)
>  end
>
> +-- Get schema version, stored in _schema system space, by reading
> the latest
> +-- snapshot file from the snap_dir. Useful to define
> schema_version before
> +-- recovering the snapshot, because some schema versions are too
> old and cannot
> +-- be recovered normally.
> +local function get_snapshot_version(snap_dir)
> + local snap_pattern = snap_dir..'/'..string.rep('[0-9]', 20)..'.snap'
>
> Probably we could use fio.pathjoin here
>
>Yep, sure. Changed.
>
> + local snap_list = fio.glob(snap_pattern)
> + table.sort(snap_list)
> + local snap = snap_list[#snap_list]
> + if not snap then
> + return nil
> + end
> + local version = nil
> + for _, row in xlog.pairs(snap) do
> + local sid = row.BODY and row.BODY.space_id
> + if sid == box.schema.SCHEMA_ID then
> + local tuple = row.BODY.tuple
> + if tuple and tuple[1] == 'version' then
> + local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
>
> Could it be replaced with tuple:unpack(2, 4)?
>
>Yep. Fixed as well.
>
> + if major and minor and patch and type(major) == 'number' and
> + type(minor) == 'number' and type(patch) == 'number' then
>
> Here you use type() == number checks. Could it be different? In case 
> of broken snap?
>
>Yep, that's a check for a broken snap.
> So, we should log it I assume. The same for similar places below.
>
>Added logging.
>
> + version = mkversion(major, minor, patch)
> + break
> + end
> + end
> + elseif sid and sid > box.schema.SCHEMA_ID then
> + -- Exit early if version wasn't found in _schema space.
> + -- Snapshot rows are ordered by space id.
> + break
> + end
> + end
> + return version
> +end
> +
>  --------------------------------------------------------------------------------
>  -- Bootstrap
>  --------------------------------------------------------------------------------
> @@ -131,6 +167,144 @@ local function create_sysview(source_id,
> target_id)
>      end
>  end
>
> +--------------------------------------------------------------------------------
> +-- Tarantool 1.7.1
> +--------------------------------------------------------------------------------
> +local function user_trig_1_7_1(_, tuple)
> + if tuple and tuple[3] == 'guest' and not tuple[5] then
>
> I think it’s better to use explicit check for tuple[5] value.
> If it’s a boolean value could it be box.NULL?
>
>not box.NULL returns false, while not nil returns true.
>
>On the other hand, box.NULL == nil returns true.
>
>So I'd rather leave this check.
>
>
>Here's the full diff:
>
>============================================
>diff --git a/src/box/lua/upgrade.lua b/src/box/lua/upgrade.lua
>index 925adab18..01e22aa7c 100644
>--- a/src/box/lua/upgrade.lua
>+++ b/src/box/lua/upgrade.lua
>@@ -88,12 +88,23 @@ local function set_system_triggers(val)
>      foreach_system_space(function(s) s:run_triggers(val) end)
>  end
>
>+local function version_from_tuple(tuple)
>+    local major, minor, patch = tuple:unpack(2, 4)
>+    patch = patch or 0
>+    if major and minor and type(major) == 'number' and
>+       type(minor) == 'number' and type(patch) == 'number' then
>+        return mkversion(major, minor, patch)
>+    end
>+    return nil
>+end
>+
>  -- Get schema version, stored in _schema system space, by reading the 
>latest
>  -- snapshot file from the snap_dir. Useful to define schema_version before
>  -- recovering the snapshot, because some schema versions are too old 
>and cannot
>  -- be recovered normally.
>  local function get_snapshot_version(snap_dir)
>-    local snap_pattern = snap_dir..'/'..string.rep('[0-9]', 20)..'.snap'
>+    local snap_pattern = fio.pathjoin(snap_dir,
>+                                      string.rep('[0-9]', 20)..'.snap')
>      local snap_list = fio.glob(snap_pattern)
>      table.sort(snap_list)
>      local snap = snap_list[#snap_list]
>@@ -106,12 +117,14 @@ local function get_snapshot_version(snap_dir)
>          if sid == box.schema.SCHEMA_ID then
>              local tuple = row.BODY.tuple
>              if tuple and tuple[1] == 'version' then
>-                local major, minor, patch = tuple[2], tuple[3], 
>tuple[4] or 0
>-                if major and minor and patch and type(major) == 
>'number' and
>-                   type(minor) == 'number' and type(patch) == 'number' then
>-                    version = mkversion(major, minor, patch)
>-                    break
>+                local major, minor, patch = tuple:unpack(2, 4)
>+                patch = patch or 0
>+                version = version_from_tuple(tuple)
>+                if not version then
>+                    log.error("Corrupted version tuple in space 
>'_schema' "..
>+                              "in snapshot '%s': %s ", snap, tuple)
>                  end
>+                break
>              end
>          elseif sid and sid > box.schema.SCHEMA_ID then
>              -- Exit early if version wasn't found in _schema space.
>@@ -1284,10 +1297,8 @@ local recovery_triggers = {
>  -- snapshot), the triggers helping recover the old schema should be 
>removed.
>  local function schema_trig_last(_, tuple)
>      if tuple and tuple[1] == 'version' then
>-        local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
>-        if major and minor and patch and type(major) == 'number' and
>-           type(minor) == 'number' and type(patch) == 'number' then
>-            local version = mkversion(major, minor, patch)
>+        local version = version_from_tuple(tuple)
>+        if version then
>              log.info("Recovery trigger: recovered schema version %s. "..
>                       "Removing outdated recovery triggers.", version)
>              box.internal.clear_recovery_triggers(version)
>
>
>============================================
>
> + local auth_method_list = {}
> + auth_method_list["chap-sha1"] = box.schema.user.password("")
> + tuple = tuple:update{{'=', 5, auth_method_list}}
> + log.info("Set empty password to user 'guest'")
> + end
> + return tuple
> +end
> +
> +--------------------------------------------------------------------------------
> +-- Tarantool 1.7.2
> +--------------------------------------------------------------------------------
> +local function index_trig_1_7_2(_, tuple)
> + local field_types_v16 = {
> + num = 'unsigned',
> + int = 'integer',
> + str = 'string',
> + }
> + if not tuple then
> + return tuple
> + end
> + local parts = tuple[6]
> + local changed = false
> + for _, part in pairs(parts) do
> + local field_type = part[2]:lower()
> + if field_types_v16[field_type] ~= nil then
> + part[2] = field_types_v16[field_type]
> + changed = true
> + end
> + end
> + if changed then
> + log.info("Update index '%s' on space '%s': set parts to %s",
> tuple[3],
> + box.space[tuple[1]].name, json.encode(parts))
> + tuple = tuple:update{{'=', 6, parts}}
> + end
> + return tuple
> +end
> +
> +--------------------------------------------------------------------------------
> +-- Tarantool 1.7.5
> +--------------------------------------------------------------------------------
> +local function create_truncate_space()
> + local _truncate = box.space[box.schema.TRUNCATE_ID]
> +
> + log.info("create space _truncate")
> + box.space._space:insert{
> + _truncate.id, ADMIN, '_truncate', 'memtx', 0, setmap({}),
> + {{name = 'id', type = 'unsigned'}, {name = 'count', type =
> 'unsigned'}}
> + }
> +
> + log.info("create index primary on _truncate")
> + box.space._index:insert{
> + _truncate.id, 0, 'primary', 'tree', {unique = true}, {{0,
> 'unsigned'}}
> + }
> +
> + local _priv = box.space[box.schema.PRIV_ID]
> + _priv:insert{ADMIN, PUBLIC, 'space', _truncate.id, box.priv.W}
> +end
> +
> +local function upgrade_to_1_7_5()
> + create_truncate_space()
> +end
> +
> +local function user_trig_1_7_5(_, tuple)
> + if tuple and not tuple[5] then
> + tuple = tuple:update{{'=', 5, setmap({})}}
> + log.info("Set empty password to %s '%s'", tuple[4], tuple[3])
> + end
> + return tuple
> +end
> +
> +local space_formats_1_7_5 = {
> + _schema = {
> + {name = 'key', type = 'string'},
> + },
> + _space = {
> + {name = 'id', type = 'unsigned'},
> + {name = 'owner', type = 'unsigned'},
> + {name = 'name', type = 'string'},
> + {name = 'engine', type = 'string'},
> + {name = 'field_count', type = 'unsigned'},
> + {name = 'flags', type = 'map'},
> + {name = 'format', type = 'array'},
> + },
> + _index = {
> + {name = 'id', type = 'unsigned'},
> + {name = 'iid', type = 'unsigned'},
> + {name = 'name', type = 'string'},
> + {name = 'type', type = 'string'},
> + {name = 'opts', type = 'map'},
> + {name = 'parts', type = 'array'},
> + },
> + _func = {
> + {name = 'id', type = 'unsigned'},
> + {name = 'owner', type = 'unsigned'},
> + {name = 'name', type = 'string'},
> + {name = 'setuid', type = 'unsigned'},
> + },
> + _user = {
> + {name = 'id', type = 'unsigned'},
> + {name = 'owner', type = 'unsigned'},
> + {name = 'name', type = 'string'},
> + {name = 'type', type = 'string'},
> + {name = 'auth', type = 'map'},
> + },
> + _priv = {
> + {name = 'grantor', type = 'unsigned'},
> + {name = 'grantee', type = 'unsigned'},
> + {name = 'object_type', type = 'string'},
> + {name = 'object_id', type = 'unsigned'},
> + {name = 'privilege', type = 'unsigned'},
> + },
> + _cluster = {
> + {name = 'id', type = 'unsigned'},
> + {name = 'uuid', type = 'string'},
> + },
> +}
> +
> +space_formats_1_7_5._vspace = space_formats_1_7_5._space
> +space_formats_1_7_5._vindex = space_formats_1_7_5._index
> +space_formats_1_7_5._vfunc = space_formats_1_7_5._func
> +space_formats_1_7_5._vuser = space_formats_1_7_5._user
> +space_formats_1_7_5._vpriv = space_formats_1_7_5._priv
> +
> +local function space_trig_1_7_5(_, tuple)
> + if tuple and space_formats_1_7_5[tuple[3]] and
> + not table.equals(space_formats_1_7_5[tuple[3]], tuple[7]) then
> + tuple = tuple:update{{'=', 7, space_formats_1_7_5[tuple[3]]}}
> + log.info("Update space '%s' format: new format %s", tuple[3],
> + json.encode(tuple[7]))
> + end
> + return tuple
> +end
> +
>  local function initial_1_7_5()
>      -- stick to the following convention:
>      -- prefer user id (owner id) in field #1
> @@ -452,6 +626,15 @@ local function upgrade_to_1_7_7()
>      _priv:replace({ADMIN, SUPER, 'universe', 0, 4294967295})
>  end
>
> +local function priv_trig_1_7_7(_, tuple)
> + if tuple and tuple[2] == ADMIN and tuple[3] == 'universe' and
> + tuple[5] ~= box.priv.ALL then
> + tuple = tuple:update{{'=', 5, box.priv.ALL}}
> + log.info("Grant all privileges to user 'admin'")
> + end
> + return tuple
> +end
> +
>  --------------------------------------------------------------------------------
>  --- Tarantool 1.10.0
>  --------------------------------------------------------------------------------
> @@ -1021,6 +1204,7 @@ end
>  --------------------------------------------------------------------------------
>
>  local handlers = {
> + {version = mkversion(1, 7, 5), func = upgrade_to_1_7_5, auto=true},
>      {version = mkversion(1, 7, 6), func = upgrade_to_1_7_6, auto
> = true},
>      {version = mkversion(1, 7, 7), func = upgrade_to_1_7_7, auto
> = true},
>      {version = mkversion(1, 10, 0), func = upgrade_to_1_10_0,
> auto = true},
> @@ -1061,13 +1245,98 @@ local function schema_needs_upgrade()
>      return false
>  end
>
> +local trig_oldest_version = nil
> +
> +-- Some schema changes before version 1.7.7 make it impossible to
> recover from
> +-- older snapshot. The table below consists of before_replace
> triggers on system
> +-- spaces, which make old snapshot schema compatible with current
> Tarantool
> +-- (version 2.x). The triggers replace old format tuples with new
> ones
> +-- in-memory, thus making it possible to recover from a rather
> old snapshot
> +-- (up to schema version 1.6.8). Once the snapshot is recovered,
> a normal
> +-- upgrade procedure may set schema version to the latest one.
> +--
> +-- The triggers mostly repeat the corresponding upgrade_to_1_7_x
> functions,
> +-- which were used when pre-1.7.x snapshot schema was still
> recoverable.
> +--
> +-- When the triggers are used (i.e. when snapshot schema version
> is below 1.7.5,
> +-- the upgrade procedure works as follows:
> +-- * first the snapshot is recovered and 1.7.5-compatible schema
> is applied to
> +-- it in-memory with the help of triggers.
> +-- * then usual upgrade_to_X_X_X() handlers may be fired to turn
> schema into the
> +-- latest one.
> +local recovery_triggers = {
> + {version = mkversion(1, 7, 1), tbl = {
> + _user = user_trig_1_7_1,
> + }},
> + {version = mkversion(1, 7, 2), tbl = {
> + _index = index_trig_1_7_2,
> + }},
> + {version = mkversion(1, 7, 5), tbl = {
> + _space = space_trig_1_7_5,
> + _user = user_trig_1_7_5,
> + }},
> + {version = mkversion(1, 7, 7), tbl = {
> + _priv = priv_trig_1_7_7,
> + }},
> +}
> +
> +-- Once newer schema version is recovered (say, from an xlog
> following the old
> +-- snapshot), the triggers helping recover the old schema should
> be removed.
> +local function schema_trig_last(_, tuple)
> + if tuple and tuple[1] == 'version' then
> + local major, minor, patch = tuple[2], tuple[3], tuple[4] or 0
> + if major and minor and patch and type(major) == 'number' and
> + type(minor) == 'number' and type(patch) == 'number' then
> + local version = mkversion(major, minor, patch)
> + log.info("Recovery trigger: recovered schema version %s. "..
> + "Removing outdated recovery triggers.", version)
> + box.internal.clear_recovery_triggers(version)
> + trig_oldest_version = version
> + end
> + end
> + return tuple
> +end
> +
> +recovery_triggers[#recovery_triggers].tbl['_schema'] =
> schema_trig_last
> +
> +local function on_init_set_recovery_triggers()
> + log.info("Recovering snapshot with schema version %s",
> trig_oldest_version)
> + for _, trig_tbl in ipairs(recovery_triggers) do
> + if trig_tbl.version > trig_oldest_version then
> + for space, trig in pairs(trig_tbl.tbl) do
> + box.space[space]:before_replace(trig)
> + log.info("Set recovery trigger on space '%s' to comply with "..
> + "version %s format", space, trig_tbl.version)
> + end
> + end
> + end
> +end
> +
> +local function set_recovery_triggers(version)
> + trig_oldest_version = version
> + box.ctl.on_schema_init(on_init_set_recovery_triggers)
> +end
> +
> +local function clear_recovery_triggers(version)
> + for _, trig_tbl in ipairs(recovery_triggers) do
> + if trig_tbl.version > trig_oldest_version and
> + (not version or trig_tbl.version < = version) then
> + for space, trig in pairs(trig_tbl.tbl) do
> + box.space[space]:before_replace(nil, trig)
> + log.info("Remove recovery trigger on space '%s' for version %s",
> + space, trig_tbl.version)
> + end
> + end
> + end
> +end
> +
>  local function upgrade(options)
>      options = options or {}
>      setmetatable(options, {__index = {auto = false}})
>
>      local version = get_version()
> - if version < mkversion(1, 7, 5) then
> - log.warn('can upgrade from 1.7.5 only')
> + if version < mkversion(1, 6, 8) then
> + log.warn('can upgrade from 1.6.8 only')
>          return
>      end
>
> @@ -1110,3 +1379,6 @@ end
>  box.schema.upgrade = upgrade;
>  box.internal.bootstrap = bootstrap;
>  box.internal.schema_needs_upgrade = schema_needs_upgrade;
> +box.internal.get_snapshot_version = get_snapshot_version;
> +box.internal.set_recovery_triggers = set_recovery_triggers;
> +box.internal.clear_recovery_triggers = clear_recovery_triggers;
> diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.result
> b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
> new file mode 100644
> index 000000000..aba5a56ab
> --- /dev/null
> +++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.result
> @@ -0,0 +1,400 @@
> +-- test-run result file version 2
> +test_run = require('test_run').new()
> + | ---
> + | ...
> +
> +-- Upgrade from 1.6.8.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:switch('upgrade')
> + | ---
> + | - true
> + | ...
> +
> +assert(not box.internal.schema_needs_upgrade())
> + | ---
> + | - true
> + | ...
> +box.space.distro:select{}
> + | ---
> + | - - ['debian', 'sarge', 31, 1118059200]
> + | - ['debian', 'etch', 40, 1176033600]
> + | - ['ubuntu', 'trusty', 1404, 1397736000]
> + | - ['ubuntu', 'vivid', 1504, 1429790400]
> + | - ['ubuntu', 'wily', 1510, 1445515200]
> + | - ['debian', 'wheezy', 70, 1367668800]
> + | - ['debian', 'squeeze', 60, 1296907200]
> + | - ['debian', 'lenny', 50, 1234612800]
> + | - ['debian', 'jessie', 80, 1430049600]
> + | - ['ubuntu', 'precise', 1510, 1335441600]
> + | - ['debian', 'woody', 30, 1027080000]
> + | ...
> +box.space._index:select{box.space.distro.id}
> + | ---
> + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
> 'string'], [1, 'string'], [
> + | 2, 'unsigned']]]
> + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
> + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
> + | ...
> +box.space._space:format()
> + | ---
> + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
> 'type': 'unsigned'}, {'name': 'name',
> + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
> {'name': 'field_count',
> + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
> {'name': 'format', 'type': 'array'}]
> + | ...
> +box.schema.user.info('admin')
> + | ---
> + | - - -
> read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('guest')
> + | ---
> + | - - - execute
> + | - role
> + | - public
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('someuser')
> + | ---
> + | - - - execute
> + | - function
> + | - someotherfunc
> + | - - execute
> + | - role
> + | - public
> + | - - execute
> + | - role
> + | - somerole
> + | - - read,write,drop,alter
> + | - space
> + | - temporary
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.role.info('somerole')
> + | ---
> + | - - - read,write,drop,alter
> + | - space
> + | - distro
> + | ...
> +
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server upgrade')
> + | ---
> + | - true
> + | ...
> +
> +-- Upgrade from 1.7.1.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:switch('upgrade')
> + | ---
> + | - true
> + | ...
> +
> +assert(not box.internal.schema_needs_upgrade())
> + | ---
> + | - true
> + | ...
> +box.space.distro:select{}
> + | ---
> + | - - ['debian', 'etch', 40, 1176033600]
> + | - ['debian', 'sarge', 31, 1118059200]
> + | - ['ubuntu', 'wily', 1510, 1445515200]
> + | - ['ubuntu', 'trusty', 1404, 1397736000]
> + | - ['ubuntu', 'vivid', 1504, 1429790400]
> + | - ['debian', 'wheezy', 70, 1367668800]
> + | - ['debian', 'squeeze', 60, 1296907200]
> + | - ['debian', 'lenny', 50, 1234612800]
> + | - ['debian', 'jessie', 80, 1430049600]
> + | - ['ubuntu', 'precise', 1510, 1335441600]
> + | - ['debian', 'woody', 30, 1027080000]
> + | ...
> +box.space._index:select{box.space.distro.id}
> + | ---
> + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
> 'string'], [1, 'string'], [
> + | 2, 'unsigned']]]
> + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
> + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
> + | ...
> +box.space._space:format()
> + | ---
> + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
> 'type': 'unsigned'}, {'name': 'name',
> + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
> {'name': 'field_count',
> + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
> {'name': 'format', 'type': 'array'}]
> + | ...
> +box.schema.user.info('admin')
> + | ---
> + | - - -
> read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('guest')
> + | ---
> + | - - - execute
> + | - role
> + | - public
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('someuser')
> + | ---
> + | - - - execute
> + | - function
> + | - someotherfunc
> + | - - execute
> + | - role
> + | - public
> + | - - execute
> + | - role
> + | - somerole
> + | - - read,write,drop,alter
> + | - space
> + | - temporary
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.role.info('somerole')
> + | ---
> + | - - - read,write,drop,alter
> + | - space
> + | - distro
> + | ...
> +
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server upgrade')
> + | ---
> + | - true
> + | ...
> +
> +-- Upgrade from 1.7.2.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:switch('upgrade')
> + | ---
> + | - true
> + | ...
> +
> +assert(not box.internal.schema_needs_upgrade())
> + | ---
> + | - true
> + | ...
> +box.space.distro:select{}
> + | ---
> + | - - ['debian', 'sarge', 31, 1118059200]
> + | - ['debian', 'etch', 40, 1176033600]
> + | - ['ubuntu', 'trusty', 1404, 1397736000]
> + | - ['ubuntu', 'vivid', 1504, 1429790400]
> + | - ['debian', 'lenny', 50, 1234612800]
> + | - ['debian', 'wheezy', 70, 1367668800]
> + | - ['debian', 'squeeze', 60, 1296907200]
> + | - ['ubuntu', 'wily', 1510, 1445515200]
> + | - ['debian', 'jessie', 80, 1430049600]
> + | - ['ubuntu', 'precise', 1510, 1335441600]
> + | - ['debian', 'woody', 30, 1027080000]
> + | ...
> +box.space._index:select{box.space.distro.id}
> + | ---
> + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
> 'string'], [1, 'string'], [
> + | 2, 'unsigned']]]
> + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
> + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
> + | ...
> +box.space._space:format()
> + | ---
> + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
> 'type': 'unsigned'}, {'name': 'name',
> + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
> {'name': 'field_count',
> + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
> {'name': 'format', 'type': 'array'}]
> + | ...
> +box.schema.user.info('admin')
> + | ---
> + | - - -
> read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('guest')
> + | ---
> + | - - - execute
> + | - role
> + | - public
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('someuser')
> + | ---
> + | - - - execute
> + | - function
> + | - someotherfunc
> + | - - execute
> + | - role
> + | - public
> + | - - execute
> + | - role
> + | - somerole
> + | - - read,write,drop,alter
> + | - space
> + | - temporary
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.role.info('somerole')
> + | ---
> + | - - - read,write,drop,alter
> + | - space
> + | - distro
> + | ...
> +
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server upgrade')
> + | ---
> + | - true
> + | ...
> +
> +-- Upgrade from 1.7.5.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('start server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:switch('upgrade')
> + | ---
> + | - true
> + | ...
> +
> +assert(not box.internal.schema_needs_upgrade())
> + | ---
> + | - true
> + | ...
> +box.space.distro:select{}
> + | ---
> + | - - ['debian', 'etch', 40, 1176033600]
> + | - ['debian', 'sarge', 31, 1118059200]
> + | - ['debian', 'lenny', 50, 1234612800]
> + | - ['ubuntu', 'trusty', 1404, 1397736000]
> + | - ['ubuntu', 'vivid', 1504, 1429790400]
> + | - ['debian', 'wheezy', 70, 1367668800]
> + | - ['debian', 'squeeze', 60, 1296907200]
> + | - ['ubuntu', 'wily', 1510, 1445515200]
> + | - ['debian', 'jessie', 80, 1430049600]
> + | - ['ubuntu', 'precise', 1510, 1335441600]
> + | - ['debian', 'woody', 30, 1027080000]
> + | ...
> +box.space._index:select{box.space.distro.id}
> + | ---
> + | - - [512, 0, 'primary', 'hash', {'unique': true}, [[0,
> 'string'], [1, 'string'], [
> + | 2, 'unsigned']]]
> + | - [512, 1, 'codename', 'hash', {'unique': true}, [[1, 'string']]]
> + | - [512, 2, 'time', 'tree', {'unique': false}, [[3, 'unsigned']]]
> + | ...
> +box.space._space:format()
> + | ---
> + | - [{'name': 'id', 'type': 'unsigned'}, {'name': 'owner',
> 'type': 'unsigned'}, {'name': 'name',
> + | 'type': 'string'}, {'name': 'engine', 'type': 'string'},
> {'name': 'field_count',
> + | 'type': 'unsigned'}, {'name': 'flags', 'type': 'map'},
> {'name': 'format', 'type': 'array'}]
> + | ...
> +box.schema.user.info('admin')
> + | ---
> + | - - -
> read,write,execute,session,usage,create,drop,alter,reference,trigger,insert,update,delete
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('guest')
> + | ---
> + | - - - execute
> + | - role
> + | - public
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.user.info('someuser')
> + | ---
> + | - - - execute
> + | - function
> + | - someotherfunc
> + | - - execute
> + | - role
> + | - public
> + | - - execute
> + | - role
> + | - somerole
> + | - - read,write,drop,alter
> + | - space
> + | - temporary
> + | - - session,usage
> + | - universe
> + | -
> + | ...
> +box.schema.role.info('somerole')
> + | ---
> + | - - - read,write,drop,alter
> + | - space
> + | - distro
> + | ...
> +
> +test_run:switch('default')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('stop server upgrade')
> + | ---
> + | - true
> + | ...
> +test_run:cmd('delete server upgrade')
> + | ---
> + | - true
> + | ...
> diff --git a/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
> b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
> new file mode 100644
> index 000000000..9096bcb7a
> --- /dev/null
> +++ b/test/xlog/gh-5894-pre-1.7.7-upgrade.test.lua
> @@ -0,0 +1,77 @@
> +test_run = require('test_run').new()
> +
> +-- Upgrade from 1.6.8.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade"')
> +test_run:cmd('start server upgrade')
> +test_run:switch('upgrade')
> +
> +assert(not box.internal.schema_needs_upgrade())
> +box.space.distro:select{}
> +box.space._index:select{box.space.distro.id}
> +box.space._space:format()
> +box.schema.user.info('admin')
> +box.schema.user.info('guest')
> +box.schema.user.info('someuser')
> +box.schema.role.info('somerole')
> +
> +test_run:switch('default')
> +test_run:cmd('stop server upgrade')
> +test_run:cmd('delete server upgrade')
> +
> +-- Upgrade from 1.7.1.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade"')
> +test_run:cmd('start server upgrade')
> +test_run:switch('upgrade')
> +
> +assert(not box.internal.schema_needs_upgrade())
> +box.space.distro:select{}
> +box.space._index:select{box.space.distro.id}
> +box.space._space:format()
> +box.schema.user.info('admin')
> +box.schema.user.info('guest')
> +box.schema.user.info('someuser')
> +box.schema.role.info('somerole')
> +
> +test_run:switch('default')
> +test_run:cmd('stop server upgrade')
> +test_run:cmd('delete server upgrade')
> +
> +-- Upgrade from 1.7.2.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade"')
> +test_run:cmd('start server upgrade')
> +test_run:switch('upgrade')
> +
> +assert(not box.internal.schema_needs_upgrade())
> +box.space.distro:select{}
> +box.space._index:select{box.space.distro.id}
> +box.space._space:format()
> +box.schema.user.info('admin')
> +box.schema.user.info('guest')
> +box.schema.user.info('someuser')
> +box.schema.role.info('somerole')
> +
> +test_run:switch('default')
> +test_run:cmd('stop server upgrade')
> +test_run:cmd('delete server upgrade')
> +
> +-- Upgrade from 1.7.5.
> +test_run:cmd('create server upgrade with script="xlog/upgrade.lua", \
> + workdir="xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade"')
> +test_run:cmd('start server upgrade')
> +test_run:switch('upgrade')
> +
> +assert(not box.internal.schema_needs_upgrade())
> +box.space.distro:select{}
> +box.space._index:select{box.space.distro.id}
> +box.space._space:format()
> +box.schema.user.info('admin')
> +box.schema.user.info('guest')
> +box.schema.user.info('someuser')
> +box.schema.role.info('somerole')
> +
> +test_run:switch('default')
> +test_run:cmd('stop server upgrade')
> +test_run:cmd('delete server upgrade')
> diff --git
> a/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
> b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
> new file mode 120000
> index 000000000..2f2a84962
> --- /dev/null
> +++ b/test/xlog/upgrade/1.6.8/gh-5894-pre-1.7.7-upgrade/fill.lua
> @@ -0,0 +1 @@
> +../../fill.lua
> \ No newline at end of file
> diff --git
> a/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
> b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
> new file mode 120000
> index 000000000..2f2a84962
> --- /dev/null
> +++ b/test/xlog/upgrade/1.7.1/gh-5894-pre-1.7.7-upgrade/fill.lua
> @@ -0,0 +1 @@
> +../../fill.lua
> \ No newline at end of file
> diff --git
> a/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
> b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
> new file mode 120000
> index 000000000..2f2a84962
> --- /dev/null
> +++ b/test/xlog/upgrade/1.7.2/gh-5894-pre-1.7.7-upgrade/fill.lua
> @@ -0,0 +1 @@
> +../../fill.lua
> \ No newline at end of file
> diff --git
> a/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
> b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
> new file mode 120000
> index 000000000..2f2a84962
> --- /dev/null
> +++ b/test/xlog/upgrade/1.7.5/gh-5894-pre-1.7.7-upgrade/fill.lua
> @@ -0,0 +1 @@
> +../../fill.lua
> \ No newline at end of file
> diff --git a/test/xlog/upgrade/fill.lua b/test/xlog/upgrade/fill.lua
> index 0ef1a8bb9..310c1ca72 100644
> --- a/test/xlog/upgrade/fill.lua
> +++ b/test/xlog/upgrade/fill.lua
> @@ -56,4 +56,8 @@ end
>  box.schema.func.create('someotherfunc')
>  box.schema.user.grant('someuser', 'execute', 'function',
> 'someotherfunc')
>  box.schema.user.grant('someuser', 'read,write', 'space', 'temporary')
> +
> +box.schema.upgrade()
> +box.snapshot()
> +
>  os.exit(0)
> --
> 2.30.1 (Apple Git-130)
>
> --
> Oleg Babin
>
>-- 
>Serge Petrenko

[-- Attachment #2: Type: text/html, Size: 46052 bytes --]

Re: [Tarantool-patches] [PATCH v2 0/2] allow upgrading from version 1.6

[Kirill Yukhin via Tarantool-patches]

Hello,

On 13 авг 02:30, Serge Petrenko via Tarantool-patches wrote:
> Changes in v2:
>   - Review fixes as per reviews from Sergos, Vladimir
>   - Fix triggers being run on xlogs resulting from
>     box.schema.ugrade()
>   - Introduce table.equals in a separate commit.
>     Use it to define whether some schema changes were
>     already applied.
> 
> https://github.com/tarantool/tarantool/issues/5894
> https://github.com/tarantool/tarantool/tree/sp/gh-5894-1.6-upgrade

I've checked your patch set into master.

--
Regards, Kirill Yukhin

Re: [Tarantool-patches] [PATCH v2 0/2] allow upgrading from version 1.6

[Kirill Yukhin via Tarantool-patches]

On 17 авг 07:28, Kirill Yukhin via Tarantool-patches wrote:
> Hello,
> 
> On 13 авг 02:30, Serge Petrenko via Tarantool-patches wrote:
> > Changes in v2:
> >   - Review fixes as per reviews from Sergos, Vladimir
> >   - Fix triggers being run on xlogs resulting from
> >     box.schema.ugrade()
> >   - Introduce table.equals in a separate commit.
> >     Use it to define whether some schema changes were
> >     already applied.
> > 
> > https://github.com/tarantool/tarantool/issues/5894
> > https://github.com/tarantool/tarantool/tree/sp/gh-5894-1.6-upgrade
> 
> I've checked your patch set into master.

Backported to 2.7 and 2.8.

--
Regards, Kirill Yukhin