select: don't replace EQ/REQ by GE/LE in planner with after

Before the patch we copy an original EQ/REQ condition to post-filter
and change the original to GE/LE. The problem is that EQ/REQ can't be
a stop condition. As a result EQ/REQ + after unexpectedly became a
full scan.

The patch removes this behavior, the query becomes more expected and
in base executes as regular EQ/REQ.

We had an idea to replace EQ with GE+LE. This would help to skip less
tuples in the executor inside scroll_to_after_tuple. But unfortunately,
due to a bug[1], this will break the current behavior. So we need to
fix the bug first.

In additional, the 'potentially long select'[2] warning is confused by
the GE/LE in the query plan.

This patch only changes the internal behavior and does not affect a
client code.

1. #301
2. #277

Author: oleg-jukovec

crud/compare/plan.lua

@@ -231,48 +231,33 @@ function plan.new(space, conditions, opts)
         return nil, err
     end
 
-    -- Since we use pagination we should continue iteration since after tuple.
-    -- Such iterator could be run only with index:pairs(key(after_tuple), 'GT/LT').
-    -- To preserve original condition we should manually inject it in `filter_conditions`
-    -- See function `parse` in crud/select/filters.lua file for details.
-    if scan_after_tuple ~= nil then
-        if scan_iter == box.index.REQ or scan_iter == box.index.EQ then
-            table.insert(conditions, conditions[scan_condition_num])
-        end
-    end
-
+    local is_equal_iter = scan_iter == box.index.EQ or scan_iter == box.index.REQ
     if opts.first ~= nil then
         total_tuples_count = math.abs(opts.first)
 
         if opts.first < 0 then
             scan_iter = utils.invert_tarantool_iter(scan_iter)
 
-            -- scan condition becomes border consition
-            scan_condition_num = nil
-
-            if scan_after_tuple ~= nil then
-                if has_keydef then
-                    local key_def = keydef_lib.new(scan_index.parts)
-                    scan_value = key_def:extract_key(scan_after_tuple)
+            -- it makes no sence for EQ/REQ
+            if not is_equal_iter then
+                -- scan condition becomes border condition
+                scan_condition_num = nil
+
+                if scan_after_tuple ~= nil then
+                    -- after becomes a new scan value
+                    if has_keydef then
+                        local key_def = keydef_lib.new(scan_index.parts)
+                        scan_value = key_def:extract_key(scan_after_tuple)
+                    else
+                        scan_value = utils.extract_key(scan_after_tuple, scan_index.parts)
+                    end
                 else
-                    scan_value = utils.extract_key(scan_after_tuple, scan_index.parts)
+                    scan_value = nil
                 end
-            else
-                scan_value = nil
             end
         end
     end
 
-    -- Moreover, for correct pagination we change iterator
-    -- to continue iterating in direct and reverse order.
-    if scan_after_tuple ~= nil then
-        if scan_iter == box.index.EQ then
-            scan_iter = box.index.GE
-        elseif scan_iter == box.index.REQ then
-            scan_iter = box.index.LE
-        end
-    end
-
     local sharding_key = nil
     if opts.bucket_id == nil then
         local sharding_index = opts.sharding_key_as_index_obj or primary_index

crud/select/executor.lua

@@ -79,9 +79,41 @@ function executor.execute(space, index, filter_func, opts)
 
     local value = opts.scan_value
     if opts.after_tuple ~= nil then
-        local new_value = generate_value(opts.after_tuple, opts.scan_value, index.parts, opts.tarantool_iter)
-        if new_value ~= nil then
-            value = new_value
+        local iter = opts.tarantool_iter
+        if iter == box.index.EQ or iter == box.index.REQ then
+            -- we need to make sure that the keys are equal
+            -- the code is correct even if value is a partial key
+            local parts = {}
+            for i, _ in ipairs(value) do
+                -- the code required for tarantool 1.10.6 at least
+                table.insert(parts, index.parts[i])
+            end
+
+            local is_eq = iter == box.index.EQ
+            local is_after_bigger
+            if has_keydef then
+                local key_def = keydef_lib.new(parts)
+                local cmp = key_def:compare_with_key(opts.after_tuple, value)
+                is_after_bigger = (is_eq and cmp > 0) or (not is_eq and cmp < 0)
+            else
+                local comparator
+                if is_eq then
+                    comparator = select_comparators.gen_func('<=', parts)
+                else
+                    comparator = select_comparators.gen_func('>=', parts)
+                end
+                local after_key = utils.extract_key(opts.after_tuple, parts)
+                is_after_bigger = not comparator(after_key, value)
+            end
+            if is_after_bigger then
+                -- it makes no sence to continue
+                return resp
+            end
+        else
+            local new_value = generate_value(opts.after_tuple, value, index.parts, iter)
+            if new_value ~= nil then
+                value = new_value
+            end
         end
     end
 

test/unit/select_plan_test.lua

@@ -276,7 +276,7 @@ g.test_first = function()
     t.assert_equals(plan.scan_value, {90}) -- after_tuple id
     t.assert_equals(plan.after_tuple, after_tuple)
     t.assert_equals(plan.scan_condition_num, 1)
-    t.assert_equals(plan.tarantool_iter, box.index.GE) -- inverted iterator
+    t.assert_equals(plan.tarantool_iter, box.index.EQ)
     t.assert_equals(plan.total_tuples_count, 10)
     t.assert_equals(plan.sharding_key, nil)
     t.assert_equals(plan.field_names, nil)