Closed
Description
We can change existing test to reveal the problem.
diff --git a/test/integration/simple_operations_test.lua b/test/integration/simple_operations_test.lua
index 8c33daa..45041ae 100644
--- a/test/integration/simple_operations_test.lua
+++ b/test/integration/simple_operations_test.lua
@@ -396,14 +396,14 @@ end
pgroup.test_intermediate_nullable_fields_update = function(g)
local result, err = g.cluster.main_server.net_box:call(
- 'crud.insert', {'developers', {1, box.NULL}})
+ 'crud.insert', {'developers', {3, box.NULL}})
t.assert_equals(err, nil)
local objects = crud.unflatten_rows(result.rows, result.metadata)
t.assert_equals(objects, {
{
- id = 1,
- bucket_id = 477
+ id = 3,
+ bucket_id = 2804
}
})
@@ -414,19 +414,22 @@ pgroup.test_intermediate_nullable_fields_update = function(g)
end)
local result, err = g.cluster.main_server.net_box:call('crud.update',
- {'developers', 1, {{'=', 'extra_3', { a = { b = {} } } }}})
+ {'developers', 3, {{'=', 'extra_3', { a = { b = {} } } }}})
t.assert_equals(err, nil)
objects = crud.unflatten_rows(result.rows, result.metadata)
t.assert_equals(objects, {
{
- id = 1,
- bucket_id = 477,
+ id = 3,
+ bucket_id = 2804,
extra_1 = box.NULL,
extra_2 = box.NULL,
extra_3 = {a = {b = {}}},
}
})
+ -- Temporary disabled
+ --[[
+
-- Test uses jsonpaths so it should be run for version 2.3+
-- where jsonpaths are supported (https://github.com/tarantool/tarantool/issues/1261).
-- However since 2.8 Tarantool could update intermediate nullable fields
@@ -486,6 +489,8 @@ pgroup.test_intermediate_nullable_fields_update = function(g)
extra_12 = 'extra_value_12'
}
})
+
+ ]]-- Temporary disabled
end
pgroup.test_object_with_nullable_fields = function(g)
This patch does not change meaning of the test, just changes storage replicaset to work with.
After this, the test fails:
$ luatest -v -p test_intermediate_nullable_fields_update
<...>
simple_operations.engine:"memtx".test_intermediate_nullable_fields_update ... (0.058s) fail
...ol-meta/crud/test/integration/simple_operations_test.lua:420: expected:
{
{
bucket_id = 2804,
extra_1 = cdata<void *>: NULL,
extra_2 = cdata<void *>: NULL,
extra_3 = {a = {b = {}}},
id = 3,
},
}
actual:
{{bucket_id = 2804, id = 3}}
<...>
I looked at the problem a bit and my draft of the fix is the following:
diff --git a/crud/update.lua b/crud/update.lua
index 587068c..2942e76 100644
--- a/crud/update.lua
+++ b/crud/update.lua
@@ -103,6 +103,18 @@ local function call_update_on_router(space_name, key, user_operations, opts)
end
local bucket_id = sharding.key_get_bucket_id(sharding_key, opts.bucket_id)
+
+ -- We should use a space object from the same connection as
+ -- we use for the request itself. Otherwise we can use a stale
+ -- format to generate rows metadata.
+ --
+ -- XXX: This fix is fast written and may have problems. IOW, I
+ -- didn't think much about it.
+ space, err = utils.get_space(space_name, {vshard.router.route(bucket_id)})
+ if err ~= nil then
+ return nil, UpdateError:new("Failed to get space %q: %s", space_name, err), true
+ end
+
local call_opts = {
mode = 'write',
timeout = opts.timeout,
Of course, we should look, whether it is fully correct: we use a space format from a 'wrong' connection before this code and, maybe, it should be revisited too.
I would also verify other operations for problems of this kind.