Skip to content

Commit 5abe1c1

Browse files
committed
Support custom sharding key in select
NOTE: Prior to this patch CRUD assumes an index is unique. It was true for the primary key, but it is not guaranteed for a sharding key. Patch adds a tests with select() for non-unique index that failed due to assumption regarding uniq index in crud/select/plan.lua. Seems we can remove this condition and fix tests that relies on total_tuple_count == 1. See also related discussion in [1]. 1. #181 (comment) Part of #166 Reviewed-by: Oleg Babin <[email protected]> Reviewed-by: Alexander Turenko <[email protected]>
1 parent 4b3482a commit 5abe1c1

File tree

6 files changed

+189
-9
lines changed

6 files changed

+189
-9
lines changed

crud/select/compat/select.lua

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ local sharding = require('crud.common.sharding')
77
local dev_checks = require('crud.common.dev_checks')
88
local common = require('crud.select.compat.common')
99
local schema = require('crud.common.schema')
10+
local sharding_key_module = require('crud.common.sharding_key')
1011

1112
local compare_conditions = require('crud.compare.conditions')
1213
local select_plan = require('crud.select.plan')
@@ -50,12 +51,17 @@ local function build_select_iterator(space_name, user_conditions, opts)
5051
return nil, SelectError:new("Space %q doesn't exist", space_name), true
5152
end
5253
local space_format = space:format()
54+
local sharding_key_as_index_obj, err = sharding_key_module.fetch_on_router(space_name)
55+
if err ~= nil then
56+
return nil, err
57+
end
5358

5459
-- plan select
5560
local plan, err = select_plan.new(space, conditions, {
5661
first = opts.first,
5762
after_tuple = opts.after,
5863
field_names = opts.field_names,
64+
sharding_key_as_index_obj = sharding_key_as_index_obj,
5965
})
6066

6167
if err ~= nil then

crud/select/compat/select_old.lua

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ local utils = require('crud.common.utils')
88
local sharding = require('crud.common.sharding')
99
local dev_checks = require('crud.common.dev_checks')
1010
local schema = require('crud.common.schema')
11+
local sharding_key_module = require('crud.common.sharding_key')
1112

1213
local compare_conditions = require('crud.compare.conditions')
1314
local select_plan = require('crud.select.plan')
@@ -102,13 +103,18 @@ local function build_select_iterator(space_name, user_conditions, opts)
102103
return nil, SelectError:new("Space %q doesn't exist", space_name), true
103104
end
104105
local space_format = space:format()
106+
local sharding_key_as_index_obj, err = sharding_key_module.fetch_on_router(space_name)
107+
if err ~= nil then
108+
return nil, err
109+
end
105110

106111
-- plan select
107112
local plan, err = select_plan.new(space, conditions, {
108113
first = opts.first,
109114
after_tuple = opts.after,
110115
field_names = opts.field_names,
111116
force_map_call = opts.force_map_call,
117+
sharding_key_as_index_obj = sharding_key_as_index_obj,
112118
})
113119

114120
if err ~= nil then

crud/select/plan.lua

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ function select_plan.new(space, conditions, opts)
126126
after_tuple = '?table|cdata',
127127
field_names = '?table',
128128
force_map_call = '?boolean',
129+
sharding_key_as_index_obj = '?table',
129130
})
130131

131132
conditions = conditions ~= nil and conditions or {}
@@ -226,19 +227,14 @@ function select_plan.new(space, conditions, opts)
226227
end
227228
end
228229

229-
local sharding_index = primary_index -- XXX: only sharding by primary key is supported
230+
local sharding_index = opts.sharding_key_as_index_obj or primary_index
230231

231232
-- get sharding key value
232233
local sharding_key
233234
if scan_value ~= nil and (scan_iter == box.index.EQ or scan_iter == box.index.REQ) then
234235
sharding_key = extract_sharding_key_from_scan_value(scan_value, scan_index, sharding_index)
235236
end
236237

237-
if sharding_key ~= nil and opts.force_map_call ~= true then
238-
total_tuples_count = 1
239-
scan_iter = box.index.REQ
240-
end
241-
242238
local plan = {
243239
conditions = conditions,
244240
space_name = space_name,

test/entrypoint/srv_ddl.lua

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,14 @@ package.preload['customers-storage'] = function()
3737
{path = 'name', is_nullable = false, type = 'string'},
3838
},
3939
}
40+
local primary_index_id = {
41+
name = 'id',
42+
type = 'TREE',
43+
unique = true,
44+
parts = {
45+
{path = 'id', is_nullable = false, type = 'unsigned'},
46+
},
47+
}
4048
local bucket_id_index = {
4149
name = 'bucket_id',
4250
type = 'TREE',
@@ -45,15 +53,54 @@ package.preload['customers-storage'] = function()
4553
{path = 'bucket_id', is_nullable = false, type = 'unsigned'},
4654
}
4755
}
56+
local name_index = {
57+
name = 'name',
58+
type = 'TREE',
59+
unique = true,
60+
parts = {
61+
{path = 'name', is_nullable = false, type = 'string'},
62+
},
63+
}
64+
local secondary_index = {
65+
name = 'secondary',
66+
type = 'TREE',
67+
unique = false,
68+
parts = {
69+
{path = 'id', is_nullable = false, type = 'unsigned'},
70+
{path = 'name', is_nullable = false, type = 'string'},
71+
},
72+
}
4873

4974
local customers_name_key_schema = table.deepcopy(customers_schema)
5075
customers_name_key_schema.sharding_key = {'name'}
5176
table.insert(customers_name_key_schema.indexes, primary_index)
5277
table.insert(customers_name_key_schema.indexes, bucket_id_index)
5378

79+
local customers_name_key_uniq_index_schema = table.deepcopy(customers_schema)
80+
customers_name_key_uniq_index_schema.sharding_key = {'name'}
81+
table.insert(customers_name_key_uniq_index_schema.indexes, primary_index)
82+
table.insert(customers_name_key_uniq_index_schema.indexes, bucket_id_index)
83+
table.insert(customers_name_key_uniq_index_schema.indexes, name_index)
84+
85+
local customers_name_key_non_uniq_index_schema = table.deepcopy(customers_schema)
86+
customers_name_key_non_uniq_index_schema.sharding_key = {'name'}
87+
name_index.unique = false
88+
table.insert(customers_name_key_non_uniq_index_schema.indexes, primary_index)
89+
table.insert(customers_name_key_non_uniq_index_schema.indexes, bucket_id_index)
90+
table.insert(customers_name_key_non_uniq_index_schema.indexes, name_index)
91+
92+
local customers_secondary_idx_name_key_schema = table.deepcopy(customers_schema)
93+
customers_secondary_idx_name_key_schema.sharding_key = {'name'}
94+
table.insert(customers_secondary_idx_name_key_schema.indexes, primary_index_id)
95+
table.insert(customers_secondary_idx_name_key_schema.indexes, secondary_index)
96+
table.insert(customers_secondary_idx_name_key_schema.indexes, bucket_id_index)
97+
5498
local schema = {
5599
spaces = {
56100
customers_name_key = customers_name_key_schema,
101+
customers_name_key_uniq_index = customers_name_key_uniq_index_schema,
102+
customers_name_key_non_uniq_index = customers_name_key_non_uniq_index_schema,
103+
customers_secondary_idx_name_key = customers_secondary_idx_name_key_schema,
57104
}
58105
}
59106

test/integration/ddl_sharding_key_test.lua

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ local crud = require('crud')
33
local t = require('luatest')
44

55
local helpers = require('test.helper')
6+
local storage_stat = require('test.helpers.storage_stat')
67

78
local ok = pcall(require, 'ddl')
89
if not ok then
@@ -46,6 +47,9 @@ pgroup.after_all(function(g) helpers.stop_cluster(g.cluster) end)
4647

4748
pgroup.before_each(function(g)
4849
helpers.truncate_space_on_cluster(g.cluster, 'customers_name_key')
50+
helpers.truncate_space_on_cluster(g.cluster, 'customers_name_key_uniq_index')
51+
helpers.truncate_space_on_cluster(g.cluster, 'customers_name_key_non_uniq_index')
52+
helpers.truncate_space_on_cluster(g.cluster, 'customers_secondary_idx_name_key')
4953
end)
5054

5155
pgroup.test_insert_object = function(g)
@@ -246,3 +250,127 @@ pgroup.test_upsert = function(g)
246250
local result = conn_s2.space['customers_name_key']:get({1, 'John'})
247251
t.assert_equals(result, nil)
248252
end
253+
254+
-- The main purpose of testcase is to verify that CRUD will calculate bucket_id
255+
-- using secondary sharding key (name) correctly and will get tuple on storage
256+
-- in replicaset s2.
257+
-- bucket_id was calculated using function below:
258+
-- function(key)
259+
-- return require('vshard.hash').strcrc32(key) % 3000 + 1
260+
-- end
261+
-- where 3000 is a default number of buckets used in vshard.
262+
pgroup.test_select = function(g)
263+
-- bucket_id is 234, storage is s-2
264+
local tuple = {8, 234, 'Ptolemy', 20}
265+
266+
-- Put tuple to s2 replicaset.
267+
local conn_s2 = g.cluster:server('s2-master').net_box
268+
local result = conn_s2.space['customers_name_key']:insert(tuple)
269+
t.assert_not_equals(result, nil)
270+
271+
local conditions = {{'==', 'name', 'Ptolemy'}}
272+
local result, err = g.cluster.main_server.net_box:call('crud.select', {
273+
'customers_name_key', conditions,
274+
})
275+
276+
t.assert_equals(err, nil)
277+
t.assert_equals(#result.rows, 1)
278+
t.assert_equals(result.rows[1], tuple)
279+
end
280+
281+
-- TODO: After enabling support of sharding keys that are not equal to primary
282+
-- keys, we should handle it differently: it is not enough to look just on scan
283+
-- value, we should traverse all conditions. Now missed cases lead to
284+
-- map-reduce. Will be resolved in #213.
285+
pgroup.test_select_wont_lead_map_reduce = function(g)
286+
local space_name = 'customers_name_key_uniq_index'
287+
288+
local conn_s1 = g.cluster:server('s1-master').net_box
289+
local conn_s2 = g.cluster:server('s2-master').net_box
290+
291+
-- bucket_id is 477, storage is s-2
292+
local result = conn_s2.space[space_name]:insert({1, 477, 'Viktor Pelevin', 58})
293+
t.assert_not_equals(result, nil)
294+
-- bucket_id is 401, storage is s-1
295+
local result = conn_s1.space[space_name]:insert({2, 401, 'Isaac Asimov', 72})
296+
t.assert_not_equals(result, nil)
297+
-- bucket_id is 2804, storage is s-2
298+
local result = conn_s2.space[space_name]:insert({3, 2804, 'Aleksandr Solzhenitsyn', 89})
299+
t.assert_not_equals(result, nil)
300+
-- bucket_id is 1161, storage is s-2
301+
local result = conn_s2.space[space_name]:insert({4, 1161, 'James Joyce', 59})
302+
t.assert_not_equals(result, nil)
303+
304+
local stat_a = storage_stat.collect(g.cluster)
305+
306+
-- Select a tuple with name 'Viktor Pelevin'.
307+
local result, err = g.cluster.main_server.net_box:call('crud.select', {
308+
space_name, {{'==', 'name', 'Viktor Pelevin'}}
309+
})
310+
t.assert_equals(err, nil)
311+
t.assert_not_equals(result, nil)
312+
t.assert_equals(#result.rows, 1)
313+
314+
local stat_b = storage_stat.collect(g.cluster)
315+
316+
-- Check a number of select() requests made by CRUD on cluster's storages
317+
-- after calling select() on a router. Make sure only a single storage has
318+
-- a single select() request. Otherwise we lead map-reduce.
319+
t.assert_equals(storage_stat.diff(stat_b, stat_a), {
320+
['s-1'] = {
321+
select_requests = 0,
322+
},
323+
['s-2'] = {
324+
select_requests = 1,
325+
},
326+
})
327+
end
328+
329+
pgroup.test_select_secondary_idx = function(g)
330+
local tuple = {2, box.NULL, 'Ivan', 20}
331+
332+
-- insert tuple
333+
local result, err = g.cluster.main_server.net_box:call('crud.insert', {
334+
'customers_secondary_idx_name_key', tuple
335+
})
336+
337+
t.assert_equals(err, nil)
338+
t.assert_not_equals(result, nil)
339+
t.assert_equals(#result.rows, 1)
340+
341+
local conditions = {{'==', 'name', 'Ivan'}}
342+
343+
local result, err = g.cluster.main_server.net_box:call('crud.select', {
344+
'customers_secondary_idx_name_key', conditions,
345+
})
346+
347+
t.assert_equals(err, nil)
348+
t.assert_equals(#result.rows, 1)
349+
t.assert_equals(result.rows[1], {2, 1366, 'Ivan', 20})
350+
end
351+
352+
pgroup.test_select_non_unique_index = function(g)
353+
local space_name = 'customers_name_key_non_uniq_index'
354+
local customers = helpers.insert_objects(g, space_name, {
355+
{id = 1, name = 'Viktor Pelevin', age = 58},
356+
{id = 2, name = 'Isaac Asimov', age = 72},
357+
{id = 3, name = 'Aleksandr Solzhenitsyn', age = 89},
358+
{id = 4, name = 'James Joyce', age = 59},
359+
{id = 5, name = 'Oscar Wilde', age = 46},
360+
-- First tuple with name 'Ivan Bunin'.
361+
{id = 6, name = 'Ivan Bunin', age = 83},
362+
{id = 7, name = 'Ivan Turgenev', age = 64},
363+
{id = 8, name = 'Alexander Ostrovsky', age = 63},
364+
{id = 9, name = 'Anton Chekhov', age = 44},
365+
-- Second tuple with name 'Ivan Bunin'.
366+
{id = 10, name = 'Ivan Bunin', age = 83},
367+
})
368+
t.assert_equals(#customers, 10)
369+
370+
local result, err = g.cluster.main_server.net_box:call('crud.select', {
371+
space_name, {{'==', 'name', 'Ivan Bunin'}}
372+
})
373+
t.assert_equals(err, nil)
374+
t.assert_not_equals(result, nil)
375+
t.assert_equals(#result.rows, 2)
376+
end

test/unit/select_plan_test.lua

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ g.test_is_scan_by_full_sharding_key_eq = function()
159159

160160
t.assert_equals(err, nil)
161161

162-
t.assert_equals(plan.total_tuples_count, 1)
163162
t.assert_equals(plan.sharding_key, {15})
164163

165164
-- id is a part of scan index
@@ -173,7 +172,6 @@ g.test_is_scan_by_full_sharding_key_eq = function()
173172

174173
t.assert_equals(plan.index_id, 3) -- index name_id is used
175174
t.assert_equals(plan.scan_value, {'Ivan', 11})
176-
t.assert_equals(plan.total_tuples_count, 1)
177175
t.assert_equals(plan.sharding_key, {11})
178176

179177
-- other index is first
@@ -221,7 +219,6 @@ g.test_is_scan_by_full_sharding_key_eq = function()
221219

222220
t.assert_equals(err, nil)
223221

224-
t.assert_equals(plan.total_tuples_count, 1)
225222
t.assert_equals(plan.sharding_key, {1, 0})
226223
end
227224

0 commit comments

Comments
 (0)