Fix crud.select() + after behavior problems

CHANGELOG.md

@@ -10,8 +10,84 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 ### Added
 
 ### Changed
+* Optimize `crud.select()` without conditions and with `after`.
 
 ### Fixed
+* `crud.select()` if a condition is '<=' and it's value < `after`.
+
+  Before this patch:
+
+  tarantool> crud.select('developers', {{'<=', 'id', 3}}, {first = 10, after = rows[5]}).rows
+  ---
+  - - [2, 401, 'Sergey', 'Allred', 21]
+    - [1, 477, 'Alexey', 'Adams', 20]
+  ...
+
+  After this patch:
+
+  tarantool> crud.select('developers', {{'<=', 'id', 3}}, {first = 10, after = rows[5]}).rows
+  ---
+  - - [3, 2804, 'Pavel', 'Adams', 27]
+    - [2, 401, 'Sergey', 'Allred', 21]
+    - [1, 477, 'Alexey', 'Adams', 20]
+  ...
+
+* `crud.select()` filtration by a first condition if the condition is '>'
+  or '>=' and it's value > `after`.
+
+  Before this patch:
+
+  tarantool> crud.select('developers', {{'>=', 'id', 5}}, {first = 10, after = rows[2]}).rows
+  ---
+  - - [3, 2804, 'Pavel', 'Adams', 27]
+    - [4, 1161, 'Mikhail', 'Liston', 51]
+    - [5, 1172, 'Dmitry', 'Jacobi', 16]
+    - [6, 1064, 'Alexey', 'Sidorov', 31]
+  ...
+
+  After this patch:
+
+  tarantool> crud.select('developers', {{'>=', 'id', 5}}, {first = 10, after = rows[2]}).rows
+  ---
+    - [5, 1172, 'Dmitry', 'Jacobi', 16]
+    - [6, 1064, 'Alexey', 'Sidorov', 31]
+  ...
+
+* `crud.select()` results order with negative `first`.
+* `crud.select()` if a condition is '=' or '==' with negative `first`.
+
+  Suppose we have a non-unique secondary index by the field `age` field and a
+  space:
+
+  tarantool> crud.select('developers', nil, {first = 10})
+  ---
+  - metadata: [{'name': 'id', 'type': 'unsigned'}, {'name': 'bucket_id', 'type': 'unsigned'},
+      {'name': 'name', 'type': 'string'}, {'name': 'surname', 'type': 'string'}, {'name': 'age',
+        'type': 'number'}]
+    rows:
+    - [1, 477, 'Alexey', 'Adams', 20]
+    - [2, 401, 'Sergey', 'Allred', 27]
+    - [3, 2804, 'Pavel', 'Adams', 27]
+    - [4, 1161, 'Mikhail', 'Liston', 27]
+    - [5, 1172, 'Dmitry', 'Jacobi', 27]
+    - [6, 1064, 'Alexey', 'Sidorov', 31]
+  - null
+  ...
+
+  Before this patch:
+
+  tarantool> crud.select('developers', {{'=', 'age', 27}}, {first = -10, after = rows[4]}).rows
+  ---
+  - []
+  ...
+
+  After this patch:
+
+  tarantool> crud.select('developers', {{'=', 'age', 27}}, {first = -10, after = rows[4]}).rows
+  ---
+  - - [2, 401, 'Sergey', 'Allred', 27]
+    - [3, 2804, 'Pavel', 'Adams', 27]
+  ...
 
 ## [0.11.2] - 23-05-22
 

README.md

@@ -440,8 +440,8 @@ where:
 * `conditions` (`?table`) - array of [select conditions](#select-conditions)
 * `opts`:
   * `first` (`?number`) - the maximum count of the objects to return.
-     If negative value is specified, the last objects are returned
-     (`after` option is required in this case).
+     If negative value is specified, the objects behind `after` are returned
+     (`after` option is required in this case). [See pagination examples](doc/select.md#pagination).
   * `after` (`?table`) - tuple after which objects should be selected
   * `batch_size` (`?number`) - number of tuples to process per one request to storage
   * `bucket_id` (`?number|cdata`) - bucket ID

crud/common/utils.lua

@@ -455,7 +455,7 @@ function utils.invert_tarantool_iter(iter)
 end
 
 function utils.reverse_inplace(t)
-    for i = 1,#t - 1 do
+    for i = 1,math.floor(#t / 2) do
         t[i], t[#t - i + 1] = t[#t - i + 1], t[i]
     end
     return t

crud/compare/plan.lua

@@ -231,50 +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.LE then
-            scan_iter = box.index.LT
-        elseif scan_iter == box.index.EQ then
-            scan_iter = box.index.GE
-        elseif scan_iter == box.index.REQ then
-            scan_iter = box.index.LT
-        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

@@ -42,17 +42,25 @@ end
 local generate_value
 
 if has_keydef then
-    generate_value = function(after_tuple, scan_value, index_parts)
+    generate_value = function(after_tuple, scan_value, index_parts, tarantool_iter)
         local key_def = keydef_lib.new(index_parts)
-        if key_def:compare_with_key(after_tuple, scan_value) < 0 then
+        if #scan_value == 0 and after_tuple ~= nil then
+            return key_def:extract_key(after_tuple)
+        end
+        local cmp_operator = select_comparators.get_cmp_operator(tarantool_iter)
+        local cmp = key_def:compare_with_key(after_tuple, scan_value)
+        if (cmp_operator == '<' and cmp < 0) or (cmp_operator == '>' and cmp > 0) then
             return key_def:extract_key(after_tuple)
         end
     end
 else
     generate_value = function(after_tuple, scan_value, index_parts, tarantool_iter)
+        local after_tuple_key = utils.extract_key(after_tuple, index_parts)
+        if #scan_value == 0 and after_tuple ~= nil then
+            return after_tuple_key
+        end
         local cmp_operator = select_comparators.get_cmp_operator(tarantool_iter)
         local scan_comparator = select_comparators.gen_tuples_comparator(cmp_operator, index_parts)
-        local after_tuple_key = utils.extract_key(after_tuple, index_parts)
         if scan_comparator(after_tuple_key, scan_value) then
             return after_tuple_key
         end
@@ -77,9 +85,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