fix key_def cache invalidation

When "fields" feature was introduced we change caching strategy
and started to keep "string" key for index + fields.
This approach led to the cases when nobody could invalidate
key_def cache and if user changes index schema and changes e.g.
fields order in his/her index it won't reflect sort order. Old
value stuck in cache.

This patch fixes such behaviour. We don't consider fields in
key_def cache - yes we should recreate key_def each time. But at
least it's safe.

Author: olegrok

CHANGELOG.md

@@ -23,6 +23,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
     by calling `package.search`, built-in `merger` and `keydef` modules are used
     if present in `package.loaded`. It allows to avoid ignoring errors on checking
     modules existing via `pcall(require, '<module_name>')`.
+* Fixed some cases when module ignored schema updates.
 
 ### Added
 

crud/compare/keydef.lua

@@ -1,5 +1,3 @@
-local msgpack = require('msgpack')
-
 local comparators = require('crud.compare.comparators')
 local collations = require('crud.common.collations')
 
@@ -31,10 +29,15 @@ setmetatable(keydef_cache, {__mode = 'k'})
 local function new(space, field_names, index_id)
     -- Get requested and primary index metainfo.
     local index = space.index[index_id]
-    local key = msgpack.encode({index_id, field_names})
 
-    if keydef_cache[key] ~= nil then
-        return keydef_cache[key]
+    -- We use "index" as key here (not some string or something else)
+    -- since cache should be invalidated on schema update.
+    -- It will be done automatically because fetch_schema
+    -- rewrites "index" table in space object.
+    -- Later lua garbage collector will drop old
+    -- value from "keydef_cache" table. Since it's a weak table with "k" mode.
+    if field_names == nil and keydef_cache[index] ~= nil then
+        return keydef_cache[index]
     end
 
     -- Create a key def
@@ -52,7 +55,9 @@ local function new(space, field_names, index_id)
         keydef = keydef:merge(keydef_lib.new(normalize_parts(updated_parts)))
     end
 
-    keydef_cache[key] = keydef
+    if field_names == nil then
+        keydef_cache[index] = keydef
+    end
 
     return keydef
 end

test/entrypoint/srv_update_schema.lua

@@ -18,6 +18,7 @@ package.preload['customers-storage'] = function()
                         {name = 'id', type = 'unsigned'},
                         {name = 'bucket_id', type = 'unsigned'},
                         {name = 'value', type = 'string'},
+                        {name = 'number', type = 'integer', is_nullable = true},
                     },
                     if_not_exists = true,
                     engine = engine,
@@ -64,6 +65,20 @@ package.preload['customers-storage'] = function()
                     unique = false,
                 })
             end)
+
+            rawset(_G, 'create_number_value_index', function()
+                box.space.customers:create_index('number_value_index', {
+                    parts = { {field = 'number'}, {field = 'value'} },
+                    if_not_exists = true,
+                    unique = false,
+                })
+            end)
+
+            rawset(_G, 'alter_number_value_index', function()
+                box.space.customers.index.number_value_index:alter({
+                    parts = { {field = 'value'}, {field = 'number'} },
+                })
+            end)
         end,
     }
 end

test/integration/updated_shema_test.lua

@@ -4,6 +4,8 @@ local t = require('luatest')
 
 local helpers = require('test.helper')
 
+local fiber = require('fiber')
+local crud = require('crud')
 local crud_utils = require('crud.common.utils')
 
 local pgroup = helpers.pgroup.new('updated_schema', {
@@ -742,3 +744,53 @@ pgroup:add('test_borders_value_index_added', function(g)
         t.assert_equals(err, nil)
     end
 end)
+
+pgroup:add('test_alter_index_parts', function(g)
+    -- create space w/ bucket_id index
+    helpers.call_on_servers(g.cluster, {'s1-master', 's2-master'}, function(server)
+        server.net_box:call('create_space')
+        server.net_box:call('create_bucket_id_index')
+        server.net_box:call('create_number_value_index')
+    end)
+
+    for i = 0, 9 do
+        -- Insert {0, 9}, {1, 8}, ..., {9, 0} paris in index
+        local _, err = g.cluster.main_server.net_box:call(
+                'crud.replace', {'customers', {i, nil, tostring(i), 9 - i}})
+        t.assert_equals(err, nil)
+    end
+
+    -- Check sort order before alter
+    local result, err = g.cluster.main_server.net_box:call(
+        'crud.select', {'customers', {{'>=', 'number_value_index', {0, "0"}}}}
+    )
+    t.assert_equals(err, nil)
+    t.assert_equals(#result.rows, 10)
+
+    local objects = crud.unflatten_rows(result.rows, result.metadata)
+    for i = 0, 9 do
+        t.assert_equals(objects[i + 1].number, i)
+        t.assert_equals(objects[i + 1].value, tostring(9 - i))
+    end
+
+    -- Alter index (lead to index rebuild - change parts order)
+    helpers.call_on_servers(g.cluster, {'s1-master', 's2-master'}, function(server)
+        server.net_box:call('alter_number_value_index')
+    end)
+
+    -- Wait for index rebuild and schema update
+    fiber.sleep(1)
+
+    -- Sort order should be new
+    local result, err = g.cluster.main_server.net_box:call(
+        'crud.select', {'customers', {{'>=', 'number_value_index', {"0", 0}}}}
+    )
+    t.assert_equals(err, nil)
+    t.assert_equals(#result.rows, 10)
+
+    local objects = crud.unflatten_rows(result.rows, result.metadata)
+    for i = 0, 9 do
+        t.assert_equals(objects[i + 1].number, 9 - i)
+        t.assert_equals(objects[i + 1].value, tostring(i))
+    end
+end)