Skip to content

crud.update() may use stale schema for metadata generation #236

@Totktonada

Description

@Totktonada

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.

Metadata

Metadata

Assignees

Labels

1spbugSomething isn't workingteamE

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions