Skip to content

Commit e7aaa35

Browse files
authored
Fix case with invalid pagination (#138)
This quite strange that it didn't shoot before. However, we introduced a merger usage so behaviour was changed a bit. So pagination over primary key were broken and this patch fixes it and introduces test for it.
1 parent cf48e25 commit e7aaa35

File tree

6 files changed

+77
-28
lines changed

6 files changed

+77
-28
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
1212
* Fixed not finding field in tuple on `crud.update` if
1313
there are `is_nullable` fields in front of it that were added
1414
when the schema was changed for Tarantool version <= 2.2.
15+
16+
* Pagination over multipart primary key.
1517

1618
### Added
1719

crud/select/comparators.lua

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -141,26 +141,16 @@ end
141141

142142
function comparators.gen_func(cmp_operator, key_parts)
143143
local cmp_func = array_cmp_funcs_by_operators[cmp_operator]
144-
if cmp_func == nil then
145-
return nil, ComparatorsError:new('Unsupported operator %q', cmp_operator)
146-
end
147-
148-
local func, err = gen_array_cmp_func(cmp_func, key_parts)
149-
if err ~= nil then
150-
return nil, ComparatorsError:new('Failed to generate comparator function %q', cmp_operator)
151-
end
144+
ComparatorsError:assert(cmp_func ~= nil, 'Unsupported operator %q', cmp_operator)
152145

153-
return func
146+
return gen_array_cmp_func(cmp_func, key_parts)
154147
end
155148

156149
function comparators.gen_tuples_comparator(cmp_operator, key_parts, field_names, space_format)
157150
local updated_key_parts = comparators.update_key_parts_by_field_names(
158151
space_format, field_names, key_parts
159152
)
160-
local keys_comparator, err = comparators.gen_func(cmp_operator, updated_key_parts)
161-
if err ~= nil then
162-
return nil, ComparatorsError:new("Failed to generate comparator function: %s", err)
163-
end
153+
local keys_comparator = comparators.gen_func(cmp_operator, updated_key_parts)
164154

165155
return function(lhs, rhs)
166156
local lhs_key = utils.extract_key(lhs, updated_key_parts)

crud/select/compat/select_old.lua

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -135,12 +135,9 @@ local function build_select_iterator(space_name, user_conditions, opts)
135135
-- generator of tuples comparator needs field_names and space_format
136136
-- to update fieldno in each part in cmp_key_parts because storage result contains
137137
-- fields in order specified by field_names
138-
local tuples_comparator, err = select_comparators.gen_tuples_comparator(
138+
local tuples_comparator = select_comparators.gen_tuples_comparator(
139139
cmp_operator, cmp_key_parts, plan.field_names, space_format
140140
)
141-
if err ~= nil then
142-
return nil, SelectError:new("Failed to generate comparator function: %s", err)
143-
end
144141

145142
-- filter space format by plan.field_names (user defined fields + primary key + scan key)
146143
-- to pass it user as metadata

crud/select/executor.lua

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
local errors = require('errors')
2-
local log = require('log')
32

43
local dev_checks = require('crud.common.dev_checks')
54
local select_comparators = require('crud.select.comparators')
65

76
local utils = require('crud.common.utils')
87

9-
local ScrollToAfterError = errors.new_class('ScrollToAfterError')
108
local ExecuteSelectError = errors.new_class('ExecuteSelectError')
119

1210
local executor = {}
@@ -17,10 +15,7 @@ local function scroll_to_after_tuple(gen, space, scan_index, tarantool_iter, aft
1715
local scroll_key_parts = utils.merge_primary_key_parts(scan_index.parts, primary_index.parts)
1816

1917
local cmp_operator = select_comparators.get_cmp_operator(tarantool_iter)
20-
local scroll_comparator, err = select_comparators.gen_tuples_comparator(cmp_operator, scroll_key_parts)
21-
if err ~= nil then
22-
return nil, ScrollToAfterError:new("Failed to generate comparator to scroll: %s", err)
23-
end
18+
local scroll_comparator = select_comparators.gen_tuples_comparator(cmp_operator, scroll_key_parts)
2419

2520
while true do
2621
local tuple
@@ -59,11 +54,9 @@ function executor.execute(space, index, filter_func, opts)
5954
value = opts.after_tuple
6055
else
6156
local cmp_operator = select_comparators.get_cmp_operator(opts.tarantool_iter)
62-
local scan_comparator, err = select_comparators.gen_tuples_comparator(cmp_operator, index.parts)
63-
if err ~= nil then
64-
log.warn("Failed to generate comparator for scan value: %s", err)
65-
elseif scan_comparator(opts.after_tuple, opts.scan_value) then
66-
local after_tuple_key = utils.extract_key(opts.after_tuple, index.parts)
57+
local scan_comparator = select_comparators.gen_tuples_comparator(cmp_operator, index.parts)
58+
local after_tuple_key = utils.extract_key(opts.after_tuple, index.parts)
59+
if scan_comparator(after_tuple_key, opts.scan_value) then
6760
value = after_tuple_key
6861
end
6962
end

test/entrypoint/srv_select.lua

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,28 @@ package.preload['customers-storage'] = function()
104104
if_not_exists = true,
105105
})
106106

107+
local book_translation = box.schema.space.create('book_translation', {
108+
format = {
109+
{ name = 'id', type = 'unsigned' },
110+
{ name = 'bucket_id', type = 'unsigned' },
111+
{ name = 'language', type = 'string' },
112+
{ name = 'edition', type = 'integer' },
113+
{ name = 'translator', type = 'string' },
114+
{ name = 'comments', type = 'string', is_nullable = true },
115+
},
116+
if_not_exists = true,
117+
})
118+
119+
book_translation:create_index('id', {
120+
parts = { 'id', 'language', 'edition' },
121+
if_not_exists = true,
122+
})
123+
124+
book_translation:create_index('bucket_id', {
125+
parts = { 'bucket_id' },
126+
unique = false,
127+
if_not_exists = true,
128+
})
107129
end,
108130
}
109131
end

test/integration/select_test.lua

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,51 @@ pgroup:add('test_composite_index', function(g)
610610
t.assert_equals(err, nil)
611611
local objects = crud.unflatten_rows(result.rows, result.metadata)
612612
t.assert_equals(objects, helpers.get_objects_by_idxs(customers, {2, 1})) -- in full_name order
613+
614+
-- first 1
615+
local result, err = g.cluster.main_server.net_box:call('crud.select', {'customers', conditions, {first = 1}})
616+
t.assert_equals(err, nil)
617+
local objects = crud.unflatten_rows(result.rows, result.metadata)
618+
t.assert_equals(objects, helpers.get_objects_by_idxs(customers, {2})) -- in full_name order
619+
620+
-- first 1 with full specified key
621+
local result, err = g.cluster.main_server.net_box:call('crud.select',
622+
{'customers', {{'==', 'full_name', {'Elizabeth', 'Johnson'}}}, {first = 1}})
623+
t.assert_equals(err, nil)
624+
local objects = crud.unflatten_rows(result.rows, result.metadata)
625+
t.assert_equals(objects, helpers.get_objects_by_idxs(customers, {2})) -- in full_name order
626+
end)
627+
628+
pgroup:add('test_composite_primary_index', function(g)
629+
local book_translation = helpers.insert_objects(g, 'book_translation', {
630+
{
631+
id = 5,
632+
language = 'Ukrainian',
633+
edition = 55,
634+
translator = 'Mitro Dmitrienko',
635+
comments = 'Translation 55',
636+
}
637+
})
638+
t.assert_equals(#book_translation, 1)
639+
640+
local conditions = {{'=', 'id', {5, 'Ukrainian', 55}}}
641+
642+
local result, err = g.cluster.main_server.net_box:call('crud.select', {'book_translation', conditions})
643+
t.assert_equals(err, nil)
644+
t.assert_equals(#result.rows, 1)
645+
646+
local result, err = g.cluster.main_server.net_box:call('crud.select', {'book_translation', conditions, {first = 2}})
647+
t.assert_equals(err, nil)
648+
t.assert_equals(#result.rows, 1)
649+
650+
local result, err = g.cluster.main_server.net_box:call('crud.select', {'book_translation', conditions, {first = 1}})
651+
t.assert_equals(err, nil)
652+
t.assert_equals(#result.rows, 1)
653+
654+
local result, err = g.cluster.main_server.net_box:call('crud.select',
655+
{'book_translation', conditions, {first = 1, after = result.rows[1]}})
656+
t.assert_equals(err, nil)
657+
t.assert_equals(#result.rows, 0)
613658
end)
614659

615660
pgroup:add('test_select_with_batch_size_1', function(g)

0 commit comments

Comments
 (0)