Fixed select crash when dropping indexes. (#108)

Author: mRrvz

CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 * Support for UUID field types and UUID values
 
+### Fixed
+
+* Fixed select crash when dropping indexes.
+
 ## [0.4.0] - 2020-12-02
 
 ### Fixed

crud/select/plan.lua

@@ -15,19 +15,28 @@ local function index_is_allowed(index)
 end
 
 local function get_index_for_condition(space_indexes, space_format, condition)
-    for i= 0, #space_indexes do
+    -- If we use # (not table.maxn), we may lose indexes, when user drop some indexes.
+    -- E.g: we have table with indexes id {1, 2, 3, nil, nil, 6}.
+    -- If we use #{1, 2, 3, nil, nil, 6} (== 3) we will lose index with id = 6.
+    -- See details: https://github.com/tarantool/crud/issues/103
+    local max_index = table.maxn(space_indexes)
+    for i = 0, max_index do
         local index = space_indexes[i]
-        if index.name == condition.operand and index_is_allowed(index) then
-            return index
+        if index ~= nil then
+            if index.name == condition.operand and index_is_allowed(index) then
+                return index
+            end
         end
     end
 
-    for i = 0, #space_indexes do
+    for i = 0, max_index do
         local index = space_indexes[i]
-        local first_part_fieldno = index.parts[1].fieldno
-        local first_part_name = space_format[first_part_fieldno].name
-        if first_part_name == condition.operand and index_is_allowed(index) then
-            return index
+        if index ~= nil then
+            local first_part_fieldno = index.parts[1].fieldno
+            local first_part_name = space_format[first_part_fieldno].name
+            if first_part_name == condition.operand and index_is_allowed(index) then
+                return index
+            end
         end
     end
 end
@@ -39,9 +48,16 @@ local function validate_conditions(conditions, space_indexes, space_format)
     end
 
     local index_names = {}
-    for i = 0, #space_indexes do
+
+    -- If we use # (not table.maxn), we may lose indexes, when user drop some indexes.
+    -- E.g: we have table with indexes id {1, 2, 3, nil, nil, 6}.
+    -- If we use #{1, 2, 3, nil, nil, 6} (== 3) we will lose index with id = 6.
+    -- See details: https://github.com/tarantool/crud/issues/103
+    for i = 0, table.maxn(space_indexes) do
         local index = space_indexes[i]
-        index_names[index.name] = true
+        if index ~= nil then
+            index_names[index.name] = true
+        end
     end
 
     for _, condition in ipairs(conditions) do

test/unit/select_dropped_indexes_test.lua

@@ -0,0 +1,109 @@
+local select_plan = require('crud.select.plan')
+
+local select_conditions = require('crud.select.conditions')
+local cond_funcs = select_conditions.funcs
+
+local t = require('luatest')
+local g = t.group('select_dropped_indexes')
+
+local helpers = require('test.helper')
+
+g.before_all = function()
+    helpers.box_cfg()
+
+    local customers = box.schema.space.create('customers', {
+        format = {
+            {'id', 'unsigned'},
+            {'bucket_id', 'unsigned'},
+            {'name', 'string'},
+            {'age', 'unsigned'},
+            {'number_of_pets', 'unsigned'},
+            {'cars', 'array'},
+        },
+        if_not_exists = true,
+    })
+
+    customers:create_index('index1', {
+        type = 'TREE',
+        parts = {'id'},
+        if_not_exists = true,
+    })
+
+    customers:create_index('index2', {
+        parts = {'bucket_id'},
+        unique = false,
+        if_not_exists = true,
+    })
+
+    customers:create_index('index3', {
+        type = 'TREE',
+        parts = {'age'},
+        unique = false,
+        if_not_exists = true,
+    })
+
+    customers:create_index('index4_dropped', {
+        type = 'HASH',
+        parts = {'name'},
+        if_not_exists = true,
+    })
+
+    customers:create_index('index5_dropped', {
+        type = 'HASH',
+        parts = {'age'},
+        if_not_exists = true,
+    })
+
+    customers:create_index('index6', {
+        type = 'TREE',
+        parts = {'name'},
+        unique = false,
+        if_not_exists = true,
+    })
+
+    customers.index.index4_dropped:drop()
+    customers.index.index5_dropped:drop()
+
+    -- We need this check to make sure that test actually covers a problem.
+    t.assert(#box.space.customers.index ~= table.maxn(box.space.customers.index))
+end
+
+g.after_all = function()
+    box.space.customers:drop()
+end
+
+g.test_before_dropped_index_field = function()
+    local conditions = { cond_funcs.eq('index3', 20) }
+    local plan, err = select_plan.new(box.space.customers, conditions)
+
+    t.assert_equals(err, nil)
+    t.assert_type(plan, 'table')
+
+    t.assert_equals(plan.conditions, conditions)
+    t.assert_equals(plan.space_name, 'customers')
+    t.assert_equals(plan.index_id, 2)
+end
+
+g.test_after_dropped_index_field = function()
+    local conditions = { cond_funcs.eq('index6', 'Alexey') }
+    local plan, err = select_plan.new(box.space.customers, conditions)
+
+    t.assert_equals(err, nil)
+    t.assert_type(plan, 'table')
+
+    t.assert_equals(plan.conditions, conditions)
+    t.assert_equals(plan.space_name, 'customers')
+    t.assert_equals(plan.index_id, 5)
+end
+
+g.test_non_indexed_field = function()
+    local conditions = { cond_funcs.eq('number_of_pets', 2) }
+    local plan, err = select_plan.new(box.space.customers, conditions)
+
+    t.assert_equals(err, nil)
+    t.assert_type(plan, 'table')
+
+    t.assert_equals(plan.conditions, conditions)
+    t.assert_equals(plan.space_name, 'customers')
+    t.assert_equals(plan.index_id, 0) -- PK
+end