Skip to content

Commit

Permalink
select: don't replace EQ/REQ by GE/LE in planner with after
Browse files Browse the repository at this point in the history
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
  • Loading branch information
oleg-jukovec committed Jun 10, 2022
1 parent d51bbb9 commit 0d1a094
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 34 deletions.
45 changes: 15 additions & 30 deletions crud/compare/plan.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 35 additions & 3 deletions crud/select/executor.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion test/unit/select_plan_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 0d1a094

Please sign in to comment.