scan: fix filter erroneous early exit

.gitignore

@@ -14,3 +14,12 @@ build/Makefile
 
 # Vim Swap files.
 .*.s[a-w][a-z]
+
+# tt files.
+tt.yaml
+bin/
+distfiles/
+include/
+instances.enabled/
+modules/
+templates/

CHANGELOG.md

@@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Fixed
 * Compatibility with vshard configuration if UUIDs are omitted (#407).
 * Compatibility with automatic master discovery in vshard (#409).
+* Secondary conditions for index operands with operations `>=`, `<=`, `>`, `<`
+  no longer cause missing part of the actual result for scan operations
+  (`crud.select`, `crud.pairs`, `crud.count`, `readview:select`,
+  `readview:pairs`) (#418).
 
 ## [1.4.2] - 25-12-23
 

crud/compare/filters.lua

@@ -17,12 +17,8 @@ local filters = {}
     - opposite condition `'id' < 100` becomes false
     - in such case we can exit from iteration
 ]]
-local function is_early_exit_possible(index, tarantool_iter, condition)
-    if index == nil then
-        return false
-    end
-
-    if index.name ~= condition.operand then
+local function is_early_exit_possible(scan_index, tarantool_iter, condition)
+    if scan_index.name ~= condition.operand then
         return false
     end
 
@@ -77,8 +73,8 @@ local function get_values_opts(index)
     return index_field_opts
 end
 
-local function parse(space, conditions, opts)
-    dev_checks('table', '?table', {
+local function parse(space, scan_index, conditions, opts)
+    dev_checks('table', 'table', '?table', {
         scan_condition_num = '?number',
         tarantool_iter = 'number',
     })
@@ -137,7 +133,11 @@ local function parse(space, conditions, opts)
                 operator = condition.operator,
                 values = condition.values,
                 types = fields_types,
-                early_exit_is_possible = is_early_exit_possible(index, opts.tarantool_iter, condition),
+                early_exit_is_possible = is_early_exit_possible(
+                    scan_index,
+                    opts.tarantool_iter,
+                    condition
+                ),
                 values_opts = values_opts,
             })
         end
@@ -645,13 +645,13 @@ local function compile(filter_code)
     return func
 end
 
-function filters.gen_func(space, conditions, opts)
-    dev_checks('table', '?table', {
+function filters.gen_func(space, scan_index, conditions, opts)
+    dev_checks('table', 'table', '?table', {
         tarantool_iter = 'number',
         scan_condition_num = '?number',
     })
 
-    local filter_conditions, err = parse(space, conditions, {
+    local filter_conditions, err = parse(space, scan_index, conditions, {
         scan_condition_num = opts.scan_condition_num,
         tarantool_iter = opts.tarantool_iter,
     })

crud/count.lua

@@ -52,7 +52,7 @@ local function count_on_storage(space_name, index_id, conditions, opts)
 
     local value = opts.scan_value
 
-    local filter_func, err = filters.gen_func(space, conditions, {
+    local filter_func, err = filters.gen_func(space, index, conditions, {
         tarantool_iter = opts.tarantool_iter,
         scan_condition_num = opts.scan_condition_num,
     })

crud/readview.lua

@@ -160,7 +160,7 @@ local function select_readview_on_storage(space_name, index_id, conditions, opts
         return nil, err
     end
 
-    local filter_func, err = select_filters.gen_func(space, conditions, {
+    local filter_func, err = select_filters.gen_func(space, index, conditions, {
         tarantool_iter = opts.tarantool_iter,
         scan_condition_num = opts.scan_condition_num,
     })

crud/select.lua

@@ -78,7 +78,7 @@ local function select_on_storage(space_name, index_id, conditions, opts)
         return nil, err
     end
 
-    local filter_func, err = select_filters.gen_func(space, conditions, {
+    local filter_func, err = select_filters.gen_func(space, index, conditions, {
         tarantool_iter = opts.tarantool_iter,
         scan_condition_num = opts.scan_condition_num,
     })

test/entrypoint/srv_select/storage_init.lua

@@ -192,4 +192,39 @@ return function()
             if_not_exists = true,
         })
     end
+
+    local logins_space = box.schema.space.create('logins', {
+        format = {
+            {name = 'id', type = 'unsigned'},
+            {name = 'bucket_id', type = 'unsigned'},
+            {name = 'city', type = 'string'},
+            {name = 'name', type = 'string'},
+            {name = 'last_login', type = 'number'},
+        },
+        if_not_exists = true,
+        engine = engine,
+    })
+
+    logins_space:create_index('id_index', {
+        parts = { 'id' },
+        if_not_exists = true,
+    })
+
+    logins_space:create_index('bucket_id', {
+        parts = { 'bucket_id' },
+        unique = false,
+        if_not_exists = true,
+    })
+
+    logins_space:create_index('city', {
+        parts = { 'city' },
+        unique = false,
+        if_not_exists = true,
+    })
+
+    logins_space:create_index('last_login', {
+        parts = { 'last_login' },
+        unique = false,
+        if_not_exists = true,
+    })
 end

test/helper.lua

@@ -944,4 +944,18 @@ function helpers.assert_str_contains_pattern_with_replicaset_id(str, pattern)
     t.assert(found, ("string %q does not contain pattern %q"):format(str, pattern))
 end
 
+function helpers.prepare_ordered_data(g, space, expected_objects, bucket_id, order_condition)
+    helpers.insert_objects(g, space, expected_objects)
+
+    local resp, err = g.cluster.main_server:call('crud.select', {
+        space,
+        {order_condition},
+        {bucket_id = bucket_id, mode = 'write'},
+    })
+    t.assert_equals(err, nil)
+
+    local objects = crud.unflatten_rows(resp.rows, resp.metadata)
+    t.assert_equals(objects, expected_objects)
+end
+
 return helpers

test/integration/pairs_readview_test.lua

@@ -3,6 +3,7 @@ local t = require('luatest')
 local crud_utils = require('crud.common.utils')
 
 local helpers = require('test.helper')
+local read_scenario = require('test.integration.read_scenario')
 
 local pgroup = t.group('pairs_readview', helpers.backend_matrix({
     {engine = 'memtx'},
@@ -880,3 +881,28 @@ pgroup.test_pairs_no_map_reduce = function(g)
     local diff_2 = map_reduces_after_2 - map_reduces_after_1
     t.assert_equals(diff_2, 0, 'Select request was not a map reduce')
 end
+
+pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g)
+    local read = function(cg, space, conditions, opts)
+        opts = table.deepcopy(opts) or {}
+        opts.use_tomap = true
+
+        return cg.cluster.main_server:exec(function(space, conditions, opts)
+            local crud = require('crud')
+
+            local rv, err = crud.readview()
+            t.assert_equals(err, nil)
+
+            local status, resp = pcall(function()
+                return rv:pairs(space, conditions, opts):totable()
+            end)
+            t.assert(status, resp)
+
+            rv:close()
+
+            return resp, nil
+        end, {space, conditions, opts})
+    end
+
+    read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read)
+end

test/integration/pairs_test.lua

@@ -3,6 +3,7 @@ local t = require('luatest')
 local crud_utils = require('crud.common.utils')
 
 local helpers = require('test.helper')
+local read_scenario = require('test.integration.read_scenario')
 
 local pgroup = t.group('pairs', helpers.backend_matrix({
     {engine = 'memtx'},
@@ -891,3 +892,24 @@ pgroup.test_pairs_no_map_reduce = function(g)
     local diff_2 = map_reduces_after_2 - map_reduces_after_1
     t.assert_equals(diff_2, 0, 'Select request was not a map reduce')
 end
+
+pgroup.test_gh_418_pairs_with_secondary_noneq_index_condition = function(g)
+    local read = function(cg, space, conditions, opts)
+        opts = table.deepcopy(opts) or {}
+        opts.mode = 'write'
+        opts.use_tomap = true
+
+        return cg.cluster.main_server:exec(function(space, conditions, opts)
+            local crud = require('crud')
+
+            local status, resp = pcall(function()
+                return crud.pairs(space, conditions, opts):totable()
+            end)
+            t.assert(status, resp)
+
+            return resp, nil
+        end, {space, conditions, opts})
+    end
+
+    read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read)
+end

test/integration/read_scenario.lua

@@ -0,0 +1,62 @@
+-- crud.select/crud.pairs/readview:select/readview:pairs
+-- have a lot of common scenarios, which are mostly tested with
+-- four nearly identical copypasted test functions now.
+-- This approach is expected to improve it at least for new test cases.
+-- Scenarios here are for `srv_select` entrypoint.
+
+local t = require('luatest')
+
+local helpers = require('test.helper')
+
+local function gh_418_read_with_secondary_noneq_index_condition(cg, read)
+    -- Pin bucket_id to reproduce issue on a single storage.
+    local PINNED_BUCKET_NO = 1
+
+    local expected_objects = {
+        {
+            id = 1,
+            bucket_id = PINNED_BUCKET_NO,
+            city = 'Tatsumi Port Island',
+            name = 'Yukari',
+            last_login = 42,
+        },
+        {
+            id = 2,
+            bucket_id = PINNED_BUCKET_NO,
+            city = 'Tatsumi Port Island',
+            name = 'Junpei',
+            last_login = 52,
+        },
+        {
+            id = 3,
+            bucket_id = PINNED_BUCKET_NO,
+            city = 'Tatsumi Port Island',
+            name = 'Mitsuru',
+            last_login = 42,
+        },
+    }
+
+    helpers.prepare_ordered_data(cg,
+        'logins',
+        expected_objects,
+        PINNED_BUCKET_NO,
+        {'=', 'city', 'Tatsumi Port Island'}
+    )
+
+    -- Issue https://github.com/tarantool/crud/issues/418 is as follows:
+    -- storage iterator exits early on the second tuple because
+    -- iterator had erroneously expected tuples to be sorted by `last_login`
+    -- index while iterating on `city` index. Before the issue had beed fixed,
+    -- user had received only one record instead of two.
+    local objects = read(cg,
+        'logins',
+        {{'=', 'city', 'Tatsumi Port Island'}, {'<=', 'last_login', 42}},
+        {bucket_id = PINNED_BUCKET_NO}
+    )
+
+    t.assert_equals(objects, {expected_objects[1], expected_objects[3]})
+end
+
+return {
+    gh_418_read_with_secondary_noneq_index_condition = gh_418_read_with_secondary_noneq_index_condition,
+}

test/integration/select_readview_test.lua

@@ -7,6 +7,7 @@ local crud_utils = require('crud.common.utils')
 
 
 local helpers = require('test.helper')
+local read_scenario = require('test.integration.read_scenario')
 
 local pgroup = t.group('select_readview', helpers.backend_matrix({
     {engine = 'memtx'},
@@ -2484,3 +2485,22 @@ pgroup.test_select_closed_readview = function(g)
 
     t.assert_str_contains(err.str, 'Read view is closed')
 end
+
+pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g)
+    local read = function(cg, space, conditions, opts)
+        return cg.cluster.main_server:exec(function(space, conditions, opts)
+            local crud = require('crud')
+            local rv, err = crud.readview()
+            t.assert_equals(err, nil)
+
+            local resp, err = rv:select(space, conditions, opts)
+            t.assert_equals(err, nil)
+
+            rv:close()
+
+            return crud.unflatten_rows(resp.rows, resp.metadata)
+        end, {space, conditions, opts})
+    end
+
+    read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read)
+end

test/integration/select_test.lua

@@ -4,6 +4,7 @@ local crud = require('crud')
 local crud_utils = require('crud.common.utils')
 
 local helpers = require('test.helper')
+local read_scenario = require('test.integration.read_scenario')
 
 local pgroup = t.group('select', helpers.backend_matrix({
     {engine = 'memtx'},
@@ -32,6 +33,7 @@ pgroup.before_each(function(g)
     helpers.truncate_space_on_cluster(g.cluster, 'customers')
     helpers.truncate_space_on_cluster(g.cluster, 'developers')
     helpers.truncate_space_on_cluster(g.cluster, 'cars')
+    helpers.truncate_space_on_cluster(g.cluster, 'logins')
 end)
 
 
@@ -2265,3 +2267,17 @@ pgroup.test_pairs_yield_every_0 = function(g)
         })
     end)
 end
+
+pgroup.test_gh_418_select_with_secondary_noneq_index_condition = function(g)
+    local read = function(cg, space, conditions, opts)
+        opts = table.deepcopy(opts) or {}
+        opts.mode = 'write'
+
+        local resp, err = cg.cluster.main_server:call('crud.select', {space, conditions, opts})
+        t.assert_equals(err, nil)
+
+        return crud.unflatten_rows(resp.rows, resp.metadata)
+    end
+
+    read_scenario.gh_418_read_with_secondary_noneq_index_condition(g, read)
+end